Hi, Xunlei, В Ср, 04/02/2015 в 21:28 +0800, Xunlei Pang пишет: > I've also sent out this similar patch to yours before, > and I agree with you on this point :-)
I missed your patch, thanks for the pointing to it. > 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.xun...@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.xun...@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 > > Kirill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
