Re: [PATCH] sched/deadline: Remove redundant code replenishing runtime

[Byungchul Park]

On Tue, Feb 14, 2017 at 08:42:43AM +0900, Byungchul Park wrote:
> On Mon, Feb 13, 2017 at 03:24:55PM +0000, Juri Lelli wrote:
> > > > > I think we actually want to replenish and set the next deadline at this
> > > > > point of time, not the one that we get when the task will eventually wake up.
> > > > 
> > > > Hello juri,
> > > > 
> > > > But I wonder if it's meaningful to set a next deadline for a 'sleeping
> > > > task', which, rather, could be worse because its bandwidth might be
> > > > distorted at the time it's woken up.
> > > > 
> > 
> > What you mean by 'distorted'. AFAIU, we just want to replenish when
> > needed. The instant of time when the task will eventually wake up it is
> > something we cannot rely upon, and could introduce errors.
> > 
> > IIUC, your situation looks like the below
> 
> Exactly.
> 
> > 
> >    oooo|-------------------vxxx^ooo
> >        |                   |   |
> >        |                   |   |
> >     sleep/throttle         |   |
> >                      r. timer  |
> >    		           wakeup
> 
> Sorry for bothering you..
> 
> > The task gets throttled while going to sleep, when the replenishment
> > timer fires you are proposing we do nothing and we actually replenishing
> > using the wakeup rq_clock() as reference. My worry is that, by doing so,
> > we make the task potentially loose some of its bandwidth, as we will
> > have lost some time (the 3 x-es in the diagram above) when calculating
> > its next dynamic deadline.
> 
> I meant, when we decide whether it's overflowed in dl_entiry_overflow(),
> 'right' might be smaller than 'left' because 't' is the time the 3 x-es
> already passed.
> 
> Of course, here I assumed that runtime ~= 0 and deadline ~= rq_clock
> when it was throttled, if scheduler works nicely.
> 
> > > > IMHO, it's neat to set its deadline and runtime when being woken up, in
> > > > the case already passed its deadline. Am I wrong?
> > > 
> > > And I found that dl_entity_overflow() returns true and replenishes the
> > > task unconditionally in update_dl_entity() again when the task is woken
> > > up, because 'runtime / (deadline - t) > dl_runtime / dl_period' is true.
> > > 
> > 
> > Why 'unconditionally'? It will postpone and replenish if the task is
> 
> Not exactly 'unconditially' if my assumption is broken. Sorry for
> choosing a word that is not careful.
> 
> > going to overflow, if not, it will keep its runtime and deadline we set
> 
> I meant the task will be almost always considered 'overflow', as I
> explained above. So it will be overwritten again when waking up the task
> than keep what we set in timer callback.

I don't want to argue strongly, keeping current code unchanged is ok.
I just wanted to say it will be replenished twice in most cases if:

   1. The task was throttled and passed its deadline while sleeping.

Of course, it also depends on how much negative runtime it had when
throttled. Sorry for bothering you and thanks for explaining it.