Re: [PATCH v5 4/7] sched/fair: Take into account latency priority at wakeup

[Vincent Guittot <vincent.guittot@linaro.org>]

On Sat, 22 Oct 2022 at 17:09, Chen Yu <yu.c.chen@intel.com> wrote:
>
> Hi Vincent,
> On 2022-09-25 at 16:39:05 +0200, Vincent Guittot wrote:
> [...]
> > +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
> > +{
> > +     long latency_offset = se->latency_offset;
> > +
> > +     /*
> > +      * A negative latency offset means that the sched_entity has latency
> > +      * requirement that needs to be evaluated versus other entity.
> > +      * Otherwise, use the latency weight to evaluate how much scheduling
> > +      * delay is acceptable by se.
> > +      */
> > +     if ((se->latency_offset < 0) || (curr->latency_offset < 0))
> Maybe use latency_offset < 0 directly?

yes

> BTW, is it the policy that requires the user to provide a negative
> latency nice so as to tell the kernel to compare between two tasks?

Only tasks with negative latency nice value want to be more aggressive
than the default behavior to preempt at wakeup.

> Maybe I missed the scenario, I'm thinking of the reason why we used
> " || " rather than " && " above. To be more specific, why not comparing
> se and curr only when they both have high requirement on latency(negative)?

When one task wants to be scheduled in priority (negative latency
nice), we want to keep the same offset for a same delta between the
latency nice prio.


>
> The benefit of using "||" I'm thinking of is that, if se->latency_offset < 0
> and curr->latency_offset > 0, the latency_offset would be even smaller than
> se->latency_offset, which will make the preemption easier. And vice verse.
>
> thanks,
> Chenyu
> > +             latency_offset -= curr->latency_offset;
> > +
> > +     return latency_offset;
> > +}