Date: Mon, 27 Mar 2017 19:02:53 +0200
From: luca abeni <luca.abeni@santannapisa.it>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@arm.com>,
        Claudio Scordino <claudio@evidence.eu.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Tommaso Cucinotta <tommaso.cucinotta@sssup.it>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Joel Fernandes <joelaf@google.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [RFC v5 8/9] sched/deadline: base GRUB reclaiming on the
 inactive utilization
Message-ID: <20170327190253.00025dbb@sweethome>
In-Reply-To: <20170327155335.yfmpulff5o5pyzro@hirez.programming.kicks-ass.net>
References: <1490327582-4376-1-git-send-email-luca.abeni@santannapisa.it>
        <1490327582-4376-9-git-send-email-luca.abeni@santannapisa.it>
        <20170327142633.nubm5saddpitylot@hirez.programming.kicks-ass.net>
        <20170327165651.2d09b00d@luca>
        <20170327155335.yfmpulff5o5pyzro@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2156
Lines: 66

On Mon, 27 Mar 2017 17:53:35 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Mar 27, 2017 at 04:56:51PM +0200, Luca Abeni wrote:
> 
> > > > +u64 grub_reclaim(u64 delta, struct rq *rq, u64 u)
> > > >  {
> > > > +	u64 u_act;
> > > > +
> > > > +	if (rq->dl.this_bw - rq->dl.running_bw > (1 << 20) - u)
> > > > +		u_act = u;
> > > > +	else
> > > > +		u_act = (1 << 20) - rq->dl.this_bw +
> > > > rq->dl.running_bw; +
> > > > +	return (delta * u_act) >> 20;    
> > > 
> > > But that's not what is done here I think, something like this
> > > instead:
> > > 
> > > 	Uinact = Utot - Uact
> > > 
> > > 		-t_u dt ; Uinact > (1 - t_u)
> > > 	dq = {
> > > 		-(1 - Uinact) dt
> > > 
> > > 
> > > And nowhere do we have an explanation for that.  
> > 
> > Sorry about this confusion... The accounting should be
> > 	dq = -(1 - Uinact)dt
> > but if (1 - Uinact) is too large (larger than the task's
> > utilization) then we use the task's utilization instead (otherwise,
> > we end up reclaiming other runqueues' time). I realized that this
> > check was needed after writing the comments, and I forgot to update
> > the comments when I fixed the code :(
> >   
> > > Now, I suspect we can write that like: dq = -max{ t_u, (1 -
> > > Uinact) } dt, which would suggest this is a sanity check on Utot,
> > > which I suspect can be over 1. Is this what is happening?  
> > 
> > Right... I'll fix the code and comments according to your
> > suggestion.  
> 
> But doesn't that suggest there is now another corner case where we
> 'always' select t_u because of Utot overload?
> 
> My intuition suggests we'd reclaim insufficient time in that case, but
> I've not thought much about it.

Well, setting U_act = u_i (task utilization) means that task i is
reclaiming the whole CPU time (then, the next patch will make sure that
deadline tasks cannot consume 100% of the CPU time on a single CPU).


> I feel we want a few words explaining the trade-offs made here and the
> corner cases explored.
> 
> Does that make sense?

I think it is a good idea; maybe at the OSPM summit we can work on
finding the correct wording for these comments?


			Thanks,
				Luca