Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup

[Vincent Guittot]

On Thu, 22 Sept 2022 at 18:50, Dietmar Eggemann
<dietmar.eggemann@xxxxxxx> wrote:
>
> On 22/09/2022 09:12, Vincent Guittot wrote:
> > On Thu, 22 Sept 2022 at 00:41, Dietmar Eggemann
> > <dietmar.eggemann@xxxxxxx> wrote:
> >>
> >> On 20/09/2022 17:49, Vincent Guittot wrote:
> >>> On Tue, 20 Sept 2022 at 15:18, Dietmar Eggemann
> >>> <dietmar.eggemann@xxxxxxx> wrote:
> >>>>
> >>>> On 19/09/2022 17:39, Vincent Guittot wrote:
> >>>>> On Mon, 19 Sept 2022 at 12:05, Dietmar Eggemann
> >>>>> <dietmar.eggemann@xxxxxxx> wrote:
> >>>>>>
> >>>>>> On 16/09/2022 10:03, Vincent Guittot wrote:
>
> [...]
>
> > I thought you were speaking about priority 0 vs [1..19] as you made a
> > difference in your previous comment below
> >
> >>
> >> (1) p = 10 curr =  19 -> wakeup_latency_gran() returns 12ms
> >>
> >> (2) p = 10 curr = -10 -> wakeup_latency_gran() returns 24ms
> >>
> >> In (1) only p's own latency counts whereas in (2) we take the diff,
> >
> > Yes because  curr is latency sensitive in (2) whereas it's not in (1)
> >
> >>
> >> In (A) we 'punish' p even though it competes against curr which has an
> >> even lower latency requirement than p,
> >
> > What is (A) ?  Assuming you meant (1), having a positive nice latency
>
> Sorry, yes I meant (1).
>
> > means that you don't have latency requirement but you are tolerant to
> > scheduling delay so we don't 'punish' p. P will preempt curr is we are
> > above the tolerance
>
> wakeup_preempt_entity() {
>
>     vdiff = curr->vruntime - se->vruntime
>
>     vdiff -= wakeup_latency_gran(curr, se)   <-- (3)
>
>     if (vdiff <= 0)
>         return -1;
>
>     ...
> }
>
> Wouldn't it be more suitable to return 0 from wakeup_latency_gran() if
> both have latency_nice >=0 in this case instead of se->latency_offset?

No because that defeats all the purpose of being tolerant to
scheduling delay and not trying to preempt the current as usual at
wakeup. In this case there would be not diff with not setting the
latency nice value.

>
> By `punish` I mean that vdiff (3) gets smaller in case we return (the
> positive) `se->latency_offset` even `latency nice of curr` > `latency
> nice of p`.
>
> [...]
>
> >> With p = -19 and curr = -19 we would take the diff, so 0ms.
> >>
> >> With p = 19 and curr = 19, if we would use `latency_offset -=
> >> curr->latency_offset` wakeup_latency_gran() would return 973/1024*24ms -
> >> 973/1024*24ms = 0ms and nothing will shift.
> >>
> >> OTHA, in case (1) wakeup_latency_gran() would return 512/1024*24ms -
> >> 973/1024*24ms = - 10.80ms. So p would gain an advantage here instead of
> >> a penalty.
> >
> > And that's all the point. A priority >= 0 means that you don't care
> > about scheduling delays so there is no reason to be more aggressive
> > with a task that is also latency tolerant. We only have to ensure that
> > the delay stays in the acceptable range
>
> OK, I understand you model here but I'm still not convinced. Might be
> interesting to hear what others think.