From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751063AbdBNWtg (ORCPT ); Tue, 14 Feb 2017 17:49:36 -0500 Received: from mail.santannapisa.it ([193.205.80.99]:62933 "EHLO mail.santannapisa.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750731AbdBNWtf (ORCPT ); Tue, 14 Feb 2017 17:49:35 -0500 Date: Tue, 14 Feb 2017 23:49:26 +0100 From: luca abeni To: "Steven Rostedt (VMware)" Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Daniel Bristot de Oliveira , Juri Lelli , Tommaso Cucinotta , Mike Galbraith , Romulo Silva de Oliveira Subject: Re: [PATCH 3/2] sched/deadline: Use deadline instead of period when calculating overflow Message-ID: <20170214234926.6b415428@sweethome> In-Reply-To: <20170214142848.4e62a91f@gandalf.local.home> References: <20170214142848.4e62a91f@gandalf.local.home> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Steven, On Tue, 14 Feb 2017 14:28:48 -0500 "Steven Rostedt (VMware)" wrote: > 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 I agree that there is an inconsistency here (again, using equations from the "period=deadline" case with a relative deadline different from period). I am not sure about the correct fix (wouldn't "runtime / (deadline - t) > dl_runtime / dl_deadline" allow the task to use a fraction of CPU time equal to dl_runtime / dl_deadline?) The current code is clearly wrong (as shown by Daniel), but I do not understand how the current check can allow the task to consume more than dl_runtime / dl_period... I need some more time to think about this issue. Luca > > 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) > --- > Index: linux-trace.git/kernel/sched/deadline.c > =================================================================== > --- linux-trace.git.orig/kernel/sched/deadline.c > +++ linux-trace.git/kernel/sched/deadline.c > @@ -445,13 +445,13 @@ static void replenish_dl_entity(struct s > * > * 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 sc > * 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); >