Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933836AbbBDN2Z (ORCPT ); Wed, 4 Feb 2015 08:28:25 -0500 Received: from mail-yh0-f48.google.com ([209.85.213.48]:51408 "EHLO mail-yh0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933725AbbBDN2V (ORCPT ); Wed, 4 Feb 2015 08:28:21 -0500 MIME-Version: 1.0 In-Reply-To: <1418920263-21358-1-git-send-email-pang.xunlei@linaro.org> References: <1418920263-21358-1-git-send-email-pang.xunlei@linaro.org> Date: Wed, 4 Feb 2015 21:28:21 +0800 Message-ID: Subject: Re: [PATCH] sched/deadline: Add update_rq_clock() in yield_task_dl() From: Xunlei Pang To: Kirill Tkhai Cc: Peter Zijlstra , Juri Lelli , Xunlei Pang , lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2542 Lines: 79 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 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 > --- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/