Re: [PATCH 2/6 v2] sched/eevdf: Take into account current's lag when updating slice protection

[Vincent Guittot]

On Tue, 16 Jun 2026 at 11:30, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Mon, Jun 15, 2026 at 06:24:16PM +0200, Vincent Guittot wrote:
> > Take into account the lag of current task when setting the slice protection
> > in order to ensure that the absolute value of lags will remain in the
> > range [0 : slice+tick]
> > A task that already has a negative lag will see its protection reduced
> > whereas a task with positive lag will keep a full slice protection.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 83bce5a04f3d..b8d5d9bcc014 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -1083,6 +1083,7 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
> >   */
> >  static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >  {
> > +     u64 vruntime = min_vruntime(se->vruntime, avg_vruntime(cfs_rq));
> >       u64 slice = normalized_sysctl_sched_base_slice;
> >       u64 vprot = se->deadline;
> >
> > @@ -1090,8 +1091,8 @@ static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity
> >               slice = cfs_rq_min_slice(cfs_rq);
> >
> >       slice = min(slice, se->slice);
> > -     if (slice != se->slice)
> > -             vprot = min_vruntime(vprot, se->vruntime + calc_delta_fair(slice, se));
> > +     if (vruntime != se->vruntime || slice != se->slice)
> > +             vprot = min_vruntime(vprot, vruntime + calc_delta_fair(slice, se));
> >
> >       se->vprot = vprot;
> >  }
>
> As already noted by Prateek, this doesn't seem to make much sense, since
> we just got selected by schedule(), we *must* be left of avg_vruntime(),
> otherwise we'd not be eligible and all that.
>
> > @@ -1099,8 +1100,9 @@ static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity
> >  static inline void update_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >  {
> >       u64 slice = cfs_rq_min_slice(cfs_rq);
> > +     u64 vruntime = min_vruntime(se->vruntime, avg_vruntime(cfs_rq));
> >
> > -     se->vprot = min_vruntime(se->vprot, se->vruntime + calc_delta_fair(slice, se));
> > +     se->vprot = min_vruntime(se->vprot, vruntime + calc_delta_fair(slice, se));
> >  }
> >
> >  static inline bool protect_slice(struct sched_entity *se)
>
> So:
>
>  - set_protect_slice() is called at: set_next_task(.first = true),
>    eg, only when the task gets scheduled().
>
>  - set_protect_slice() takes se->deadline as the baseline, and (when
>    RUN_TO_PARITY) computes a shorter vprot [ min_slice vs slice ].
>
>  - update_protect_slice() is called upon (failed) wakeup preemption,
>    new tasks have been added and as such the goal is to re-compute the
>    min_slice and possibly reduce vprot.
>
> Right?

Yes

>
>
> And while update_protect_slice() would ideally use the original
> se->vruntime (as per set_next_task(.first = true) to compute any new
> (shorter) vprot, per its use of min_vruntime() it can not in fact end up
> with a vprot that is longer than the initial.
>
> Now, your change is to use min(se->vruntime, avg_vruntime()) to increase
> the chance of actually computing a shorter vprot. Still very much wrong,
> but possibly less wrong.
>
> Rather than taking avg_vruntime(), would it make sense to do something
> like:

avg_vruntime can move back if the newly enqueued task has a positive
lag, accounting for this prevents the lag from increasing further and
possibly exceeding the limit.

As an example,
TA starts to run with a 16ms slice and vprot is set to its deadline
TB wakes up just after with a lag of 15.9ms and a slice of 16ms
TB doesn't preempt TA because of the run to parity
TA will run for 16ms and TB's lag will go above slice + tick
Furthermore, if TC with a 8ms slice wakes up just after TB, it will
not preempt TA because TB has an earlier deadline. Using the time
already consumed by TA would not help further because it would let TA
with almost 8ms of protection.

>
>         slice = cfs_rq_min_slice(cfs_rq);
>         slice -= se->sum_exec_runtime - se->prev_sum_exec_runtime;
>
>         vprot = se->vruntime;
>         if (slice < 0)
>                 vprot -= calc_delta_fair(-slice, se);
>         else
>                 vprot += calc_delta_fair(slice, se);
>
>         se->vprot = min_vruntime(se->vprot, vprot);
>
> That is, reduce the slice with the time already ran.
>
> Now, this will go sideways in the unlikely case of renice, and possibly
> sched_change pattern (it seems we update prev_sum_exec_runtime for
> !first), but overall it might be a better approximation, no?



>