Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754877AbdCGIWs (ORCPT ); Tue, 7 Mar 2017 03:22:48 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33131 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753171AbdCGIWm (ORCPT ); Tue, 7 Mar 2017 03:22:42 -0500 MIME-Version: 1.0 In-Reply-To: <02135a27f1ae3fe5fd032568a5a2f370e190e8d7.1488392936.git.bristot@redhat.com> References: <02135a27f1ae3fe5fd032568a5a2f370e190e8d7.1488392936.git.bristot@redhat.com> From: Wanpeng Li Date: Tue, 7 Mar 2017 15:53:42 +0800 Message-ID: Subject: Re: [PATCH V4 3/3] sched/deadline: Use deadline instead of period when calculating overflow To: Daniel Bristot de Oliveira Cc: "linux-kernel@vger.kernel.org" , Ingo Molnar , Peter Zijlstra , Juri Lelli , Tommaso Cucinotta , Luca Abeni , Steven Rostedt , Mike Galbraith , Romulo Silva de Oliveira 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: 4158 Lines: 97 2017-03-02 22:10 GMT+08:00 Daniel Bristot de Oliveira : > From: "Steven Rostedt (VMware)" > > I was testing Daniel's changes with his test case, and tweaked it a > little. Instead of having the runtime equal to the deadline, I > increased the deadline ten fold. > > Daniel's test case had: > > attr.sched_runtime = 2 * 1000 * 1000; /* 2 ms */ > attr.sched_deadline = 2 * 1000 * 1000; /* 2 ms */ > attr.sched_period = 2 * 1000 * 1000 * 1000; /* 2 s */ > > To make it more interesting, I changed it to: > > attr.sched_runtime = 2 * 1000 * 1000; /* 2 ms */ > attr.sched_deadline = 20 * 1000 * 1000; /* 20 ms */ > attr.sched_period = 2 * 1000 * 1000 * 1000; /* 2 s */ > > The results were rather surprising. The behavior that Daniel's patch > was fixing came back. The task started using much more than .1% of the > CPU. More like 20%. > > Looking into this I found that it was due to the dl_entity_overflow() > constantly returning true. That's because it uses the relative period > against relative runtime vs the absolute deadline against absolute > runtime. > > runtime / (deadline - t) > dl_runtime / dl_period > > There's even a comment mentioning this, and saying that when relative > deadline equals relative period, that the equation is the same as using > deadline instead of period. That comment is backwards! What we really > want is: > > runtime / (deadline - t) > dl_runtime / dl_deadline > > We care about if the runtime can make its deadline, not its period. And > then we can say "when the deadline equals the period, the equation is > the same as using dl_period instead of dl_deadline". > > After correcting this, now when the task gets enqueued, it can throttle > correctly, and Daniel's fix to the throttling of sleeping deadline > tasks works even when the runtime and deadline are not the same. > > Signed-off-by: Steven Rostedt (VMware) > Reviewed-by: Daniel Bristot de Oliveira > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Juri Lelli > Cc: Tommaso Cucinotta > Cc: Luca Abeni > Cc: Steven Rostedt > Cc: Mike Galbraith > Cc: Romulo Silva de Oliveira > Cc: Daniel Bristot de Oliveira > Cc: linux-kernel@vger.kernel.org > --- Reviewed-by: Wanpeng Li > kernel/sched/deadline.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index b669f7f..f7403e5 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -445,13 +445,13 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se, > * > * This function returns true if: > * > - * runtime / (deadline - t) > dl_runtime / dl_period , > + * runtime / (deadline - t) > dl_runtime / dl_deadline , > * > * IOW we can't recycle current parameters. > * > - * Notice that the bandwidth check is done against the period. For > + * Notice that the bandwidth check is done against the deadline. For > * task with deadline equal to period this is the same of using > - * dl_deadline instead of dl_period in the equation above. > + * dl_period instead of dl_deadline in the equation above. > */ > static bool dl_entity_overflow(struct sched_dl_entity *dl_se, > struct sched_dl_entity *pi_se, u64 t) > @@ -476,7 +476,7 @@ static bool dl_entity_overflow(struct sched_dl_entity *dl_se, > * of anything below microseconds resolution is actually fiction > * (but still we want to give the user that illusion >;). > */ > - left = (pi_se->dl_period >> DL_SCALE) * (dl_se->runtime >> DL_SCALE); > + left = (pi_se->dl_deadline >> DL_SCALE) * (dl_se->runtime >> DL_SCALE); > right = ((dl_se->deadline - t) >> DL_SCALE) * > (pi_se->dl_runtime >> DL_SCALE); > > -- > 2.9.3 >