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

From: Vincent Guittot
Date: Mon Nov 14 2022 - 05:37:18 EST


On Sun, 13 Nov 2022 at 09:51, shrikanth suresh hegde
<sshegde@xxxxxxxxxxxxxxxxxx> wrote:
>
>
> > /*
> > * Preempt the current task with a newly woken task if needed:
> > */
> > @@ -4566,7 +4568,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > {
> > unsigned long ideal_runtime, delta_exec;
> > struct sched_entity *se;
> > - s64 delta;
> > + s64 delta, offset;
> >
> > ideal_runtime = sched_slice(cfs_rq, curr);
> > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> > @@ -4591,10 +4593,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > se = __pick_first_entity(cfs_rq);
> > delta = curr->vruntime - se->vruntime;
> >
> > - if (delta < 0)
> > + offset = wakeup_latency_gran(curr, se);
> > + if (delta < offset)
> > return;
> >
> > - if (delta > ideal_runtime)
> > + if ((delta > ideal_runtime) ||
> > + (delta > get_latency_max()))
> > resched_curr(rq_of(cfs_rq));
> > }
> >
> >
> Hi Vincent,
>
> I am not sure if i have understood this below change correctly.

I assume you wanted to say above as you removed the end of the patch

>
> wakeup_latency_gran - this function returns difference in latency nice offsets.
> Hence the more negative the value, it means se has more latency requirement
> compared to current. Hence se should preempt the current here right?

yes. the latency nice offset can add a offset in the comparison of (delta < 0)


>
> we are comparing delta to get_latency_max and ideal_runtime, both of which can
> be higher positive value, hence we will not preempt. that is not what we want
> right?

Here we are in the tick and not in the wakeup path. So the wakeup
preemption, if any, should have already happened and we now have to
make sure that if current has not been preempted at wakeup, it will
not get too much run time that will move waiting tasks out of the
sched_latency period

>