Messages in this thread Patch in this message | | | Date | Wed, 4 Feb 2015 21:28:21 +0800 | Subject | Re: [PATCH] sched/deadline: Add update_rq_clock() in yield_task_dl() | From | Xunlei Pang <> |
| |
Hi Kirill,
I've also sent out this similar patch to yours before, and I agree with you on this point :-)
But with the latest modification which peter made, ( see: cebde6d681aa45f96111cfcffc1544cf2a0454ff ) I think it would be better to add a skip operation after update_curr_dl(rq) like below:
--- kernel/sched/deadline.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index e5db8c6..5648e62 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -915,7 +915,11 @@ static void yield_task_dl(struct rq *rq) rq->curr->dl.dl_yielded = 1; p->dl.runtime = 0; } + + update_rq_clock(rq); update_curr_dl(rq); + /* Will go to schedule(), to avoid another clock update. */ + rq_clock_skip_update(rq, true); }
#ifdef CONFIG_SMP -- 1.9.1 On 19 December 2014 at 00:31, Xunlei Pang <pang.xunlei@linaro.org> wrote: > yield_task_dl() calls update_curr_dl() which calls start_dl_timer() > to throttle current task. But yield_task_dl() doesn't update the rq > clock which will cause start_dl_timer() to set the wrong dl_timer > which may be much later than current deadline time. > > For instance, in systems with 100HZ tick, if there're few dl-tasks, > then rq clock will mainly depend on the tick to update its value. > If we choose the CONFIG_NO_HZ_FULL_ALL feature, things may get even > worse. > > Moreover, there're some statistics in update_curr_dl() also relying > on rq clock, like rq_clock_task(). > > Thus, in yield_task_dl(), we should add update_rq_clock() before > update_curr_dl(). > > Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org> > --- > kernel/sched/deadline.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index e5db8c6..5648e62 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -915,7 +915,11 @@ static void yield_task_dl(struct rq *rq) > rq->curr->dl.dl_yielded = 1; > p->dl.runtime = 0; > } > + > + update_rq_clock(rq); > update_curr_dl(rq); > + /* Will go to schedule(), to avoid another clock update. */ > + rq->skip_clock_update = 1; > } > > #ifdef CONFIG_SMP > -- > 1.9.1 >
| |