[PATCH v3] sched/eevdf: Fix vprot across reweighting

[PATCH v3] sched/eevdf: Fix vprot across reweighting

[Kayra Cizmeci]

Currently in reweighting, while a tasks vruntime changes dead protection
could go alive, or alive protection could decrease, increase and go dead.
Fix it.

Fixes: 80390ead2080 ("sched/fair: Separate se->vlag from se->vprot")
Fixes: bcd74b2ffdd0 ("sched/fair: Only set slice protection at pick time")
Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---

Changes v2->v3:
* Rebase and move the enqueue_task_fair() block to if (!curr) block.


Not tested at all :-(. But I think
This should'a cover the shced_change_begin() path same,
since curr == NULL, so we always enter if (!curr) block.

 kernel/sched/fair.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0feec172fd37..4593d0122d66 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4892,12 +4892,15 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
 	update_load_set(&se->h_load, weight);
 
 	if (on_rq) {
-		if (rel_vprot)
-			se->vprot += avruntime;
 		se->deadline += avruntime;
 		se->rel_deadline = 0;
 		se->vruntime = avruntime - se->vlag;
 
+		if (rel_vprot)
+			se->vprot += avruntime;
+		else if (curr)
+			se->vprot = se->vruntime;
+
 		if (!curr)
 			__enqueue_entity(cfs_rq, se);
 		cfs_rq->h_nr_queued++;
@@ -8155,8 +8158,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	int task_new = !(flags & ENQUEUE_WAKEUP);
 	struct sched_entity *se = &p->se;
 	struct cfs_rq *cfs_rq = &rq->cfs;
-	unsigned long weight;
-	bool curr;
+	unsigned long weight, old_weight;
+	bool curr, live, running;
+	s64 dist;
 
 	if (task_is_throttled(p) && enqueue_throttled_task(p))
 		return;
@@ -8200,8 +8204,24 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	weight = enqueue_hierarchy(p, flags);
 
 	if (!curr) {
+		running = task_current_donor(rq, p);
+		live = running && protect_slice(se);
+
+		if (live) {
+			old_weight = se->h_load.weight;
+			dist = se->vprot - se->vruntime;
+		}
 		reweight_eevdf(cfs_rq, se, weight, false);
 		place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
+
+		if (live) {
+			if (weight != old_weight)
+				dist = div64_long(dist * old_weight, weight);
+			se->vprot = se->vruntime + dist;
+		} else if (running) {
+			cancel_protect_slice(se);
+		}
+
 		__enqueue_entity(cfs_rq, se);
 	}
 
-- 
2.53.0

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Vincent Guittot]

On Fri, 16 Oct 2026 at 10:46, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
>
> Currently in reweighting, while a tasks vruntime changes dead protection
> could go alive, or alive protection could decrease, increase and go dead.
> Fix it.
>
> Fixes: 80390ead2080 ("sched/fair: Separate se->vlag from se->vprot")
> Fixes: bcd74b2ffdd0 ("sched/fair: Only set slice protection at pick time")
> Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
> ---
>
> Changes v2->v3:
> * Rebase and move the enqueue_task_fair() block to if (!curr) block.
>
>
> Not tested at all :-(. But I think
> This should'a cover the shced_change_begin() path same,
> since curr == NULL, so we always enter if (!curr) block.
>
>  kernel/sched/fair.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0feec172fd37..4593d0122d66 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4892,12 +4892,15 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>         update_load_set(&se->h_load, weight);
>
>         if (on_rq) {
> -               if (rel_vprot)
> -                       se->vprot += avruntime;
>                 se->deadline += avruntime;
>                 se->rel_deadline = 0;
>                 se->vruntime = avruntime - se->vlag;
>
> +               if (rel_vprot)
> +                       se->vprot += avruntime;
> +               else if (curr)
> +                       se->vprot = se->vruntime;
> +

This part looks good

>                 if (!curr)
>                         __enqueue_entity(cfs_rq, se);
>                 cfs_rq->h_nr_queued++;
> @@ -8155,8 +8158,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>         int task_new = !(flags & ENQUEUE_WAKEUP);
>         struct sched_entity *se = &p->se;
>         struct cfs_rq *cfs_rq = &rq->cfs;
> -       unsigned long weight;
> -       bool curr;
> +       unsigned long weight, old_weight;
> +       bool curr, live, running;
> +       s64 dist;
>
>         if (task_is_throttled(p) && enqueue_throttled_task(p))
>                 return;
> @@ -8200,8 +8204,24 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>         weight = enqueue_hierarchy(p, flags);
>
>         if (!curr) {
> +               running = task_current_donor(rq, p);
> +               live = running && protect_slice(se);

IMO, "protected" is a better name than "live".

 se != curr so why is this needed?

> +
> +               if (live) {
> +                       old_weight = se->h_load.weight;
> +                       dist = se->vprot - se->vruntime;
> +               }
>                 reweight_eevdf(cfs_rq, se, weight, false);
>                 place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> +
> +               if (live) {
> +                       if (weight != old_weight)
> +                               dist = div64_long(dist * old_weight, weight);
> +                       se->vprot = se->vruntime + dist;
> +               } else if (running) {
> +                       cancel_protect_slice(se);
> +               }
> +
>                 __enqueue_entity(cfs_rq, se);
>         }
>
> --
> 2.53.0
>

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Kayra Cizmeci]

On Fri, Oct 16, 2026 at 11:15:31AM +0200, Vincent Guittot wrote:
> On Fri, 16 Oct 2026 at 10:46, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> >
> > Currently in reweighting, while a tasks vruntime changes dead protection
> > could go alive, or alive protection could decrease, increase and go dead.
> > Fix it.
> >
> > Fixes: 80390ead2080 ("sched/fair: Separate se->vlag from se->vprot")
> > Fixes: bcd74b2ffdd0 ("sched/fair: Only set slice protection at pick time")
> > Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
> > Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
> > ---
> >
> > Changes v2->v3:
> > * Rebase and move the enqueue_task_fair() block to if (!curr) block.
> >
> >
> > Not tested at all :-(. But I think
> > This should'a cover the shced_change_begin() path same,
> > since curr == NULL, so we always enter if (!curr) block.
> >
> >  kernel/sched/fair.c | 28 ++++++++++++++++++++++++----
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 0feec172fd37..4593d0122d66 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4892,12 +4892,15 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> >         update_load_set(&se->h_load, weight);
> >
> >         if (on_rq) {
> > -               if (rel_vprot)
> > -                       se->vprot += avruntime;
> >                 se->deadline += avruntime;
> >                 se->rel_deadline = 0;
> >                 se->vruntime = avruntime - se->vlag;
> >
> > +               if (rel_vprot)
> > +                       se->vprot += avruntime;
> > +               else if (curr)
> > +                       se->vprot = se->vruntime;
> > +
> 
> This part looks good

:-)
> 
> >                 if (!curr)
> >                         __enqueue_entity(cfs_rq, se);
> >                 cfs_rq->h_nr_queued++;
> > @@ -8155,8 +8158,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >         int task_new = !(flags & ENQUEUE_WAKEUP);
> >         struct sched_entity *se = &p->se;
> >         struct cfs_rq *cfs_rq = &rq->cfs;
> > -       unsigned long weight;
> > -       bool curr;
> > +       unsigned long weight, old_weight;
> > +       bool curr, live, running;
> > +       s64 dist;
> >
> >         if (task_is_throttled(p) && enqueue_throttled_task(p))
> >                 return;
> > @@ -8200,8 +8204,24 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >         weight = enqueue_hierarchy(p, flags);
> >
> >         if (!curr) {
> > +               running = task_current_donor(rq, p);
> > +               live = running && protect_slice(se);
> 
> IMO, "protected" is a better name than "live".

OK. Will rename it.

> 
>  se != curr so why is this needed?

sched_change_begin() sets .running as task_current_donor(), then if
.running is true calls put_prev_task(). put_prev_task_fair() sets
cfs_rq->curr to NULL. While the task still is the donor. Then we enter
sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
the donor is still p. There is set_next_task() call afterwards, but it
does not sets protection since its called with SNT_NORMAL.

> 
> > +
> > +               if (live) {
> > +                       old_weight = se->h_load.weight;
> > +                       dist = se->vprot - se->vruntime;
> > +               }
> >                 reweight_eevdf(cfs_rq, se, weight, false);
> >                 place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> > +
> > +               if (live) {
> > +                       if (weight != old_weight)
> > +                               dist = div64_long(dist * old_weight, weight);
> > +                       se->vprot = se->vruntime + dist;
> > +               } else if (running) {
> > +                       cancel_protect_slice(se);
> > +               }
> > +
> >                 __enqueue_entity(cfs_rq, se);
> >         }
> >
> > --
> > 2.53.0
> >

Thanks for the review!

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Vincent Guittot]

Le vendredi 16 oct. 2026 à 12:45:45 (+0300), Kayra Cizmeci a écrit :
> On Fri, Oct 16, 2026 at 11:15:31AM +0200, Vincent Guittot wrote:
> > On Fri, 16 Oct 2026 at 10:46, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> > >
> > > Currently in reweighting, while a tasks vruntime changes dead protection
> > > could go alive, or alive protection could decrease, increase and go dead.
> > > Fix it.
> > >
> > > Fixes: 80390ead2080 ("sched/fair: Separate se->vlag from se->vprot")
> > > Fixes: bcd74b2ffdd0 ("sched/fair: Only set slice protection at pick time")
> > > Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
> > > Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
> > > ---
> > >
> > > Changes v2->v3:
> > > * Rebase and move the enqueue_task_fair() block to if (!curr) block.
> > >
> > >
> > > Not tested at all :-(. But I think
> > > This should'a cover the shced_change_begin() path same,
> > > since curr == NULL, so we always enter if (!curr) block.
> > >
> > >  kernel/sched/fair.c | 28 ++++++++++++++++++++++++----
> > >  1 file changed, 24 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index 0feec172fd37..4593d0122d66 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -4892,12 +4892,15 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > >         update_load_set(&se->h_load, weight);
> > >
> > >         if (on_rq) {
> > > -               if (rel_vprot)
> > > -                       se->vprot += avruntime;
> > >                 se->deadline += avruntime;
> > >                 se->rel_deadline = 0;
> > >                 se->vruntime = avruntime - se->vlag;
> > >
> > > +               if (rel_vprot)
> > > +                       se->vprot += avruntime;
> > > +               else if (curr)
> > > +                       se->vprot = se->vruntime;
> > > +
> > 
> > This part looks good
> 
> :-)
> > 
> > >                 if (!curr)
> > >                         __enqueue_entity(cfs_rq, se);
> > >                 cfs_rq->h_nr_queued++;
> > > @@ -8155,8 +8158,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > >         int task_new = !(flags & ENQUEUE_WAKEUP);
> > >         struct sched_entity *se = &p->se;
> > >         struct cfs_rq *cfs_rq = &rq->cfs;
> > > -       unsigned long weight;
> > > -       bool curr;
> > > +       unsigned long weight, old_weight;
> > > +       bool curr, live, running;
> > > +       s64 dist;
> > >
> > >         if (task_is_throttled(p) && enqueue_throttled_task(p))
> > >                 return;
> > > @@ -8200,8 +8204,24 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > >         weight = enqueue_hierarchy(p, flags);
> > >
> > >         if (!curr) {
> > > +               running = task_current_donor(rq, p);
> > > +               live = running && protect_slice(se);
> > 
> > IMO, "protected" is a better name than "live".
> 
> OK. Will rename it.
> 
> > 
> >  se != curr so why is this needed?
> 
> sched_change_begin() sets .running as task_current_donor(), then if
> .running is true calls put_prev_task(). put_prev_task_fair() sets
> cfs_rq->curr to NULL. While the task still is the donor. Then we enter
> sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
> the donor is still p. There is set_next_task() call afterwards, but it
> does not sets protection since its called with SNT_NORMAL.

okay, but we don't want to spread this all over the code.
reweight_eevdf/scale_entity is  doing this already.

The below should do the stuff while keeping the computation in reweight_eevdf:

---
 kernel/sched/fair.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0feec172fd37..c212c6aa9691 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4863,10 +4863,11 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
 }
 
 static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
-			   unsigned long weight, bool on_rq)
+			   unsigned long weight, bool update_vprot)
 {
 	bool curr = cfs_rq->curr == se;
 	bool rel_vprot = false;
+	bool on_rq = se->on_rq;
 	u64 avruntime = 0;
 
 	if (se->h_load.weight == weight)
@@ -4878,22 +4879,25 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
 		se->deadline -= avruntime;
 		se->rel_deadline = 1;
 		if (curr && protect_slice(se)) {
-			se->vprot -= avruntime;
 			rel_vprot = true;
 		}
 
 		cfs_rq->h_nr_queued--;
 		if (!curr)
 			__dequeue_entity(cfs_rq, se);
+	} else if (update_vprot) {
+		avruntime = se->vruntime;
+		rel_vprot = true;
 	}
 
+	if (rel_vprot)
+		se->vprot -= avruntime;
+
 	rescale_entity(se, weight, rel_vprot);
 
 	update_load_set(&se->h_load, weight);
 
 	if (on_rq) {
-		if (rel_vprot)
-			se->vprot += avruntime;
 		se->deadline += avruntime;
 		se->rel_deadline = 0;
 		se->vruntime = avruntime - se->vlag;
@@ -4902,6 +4906,12 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
 			__enqueue_entity(cfs_rq, se);
 		cfs_rq->h_nr_queued++;
 	}
+
+	if (rel_vprot)
+		se->vprot += avruntime;
+	else if (curr || update_vprot)
+		se->vprot = se->vruntime;
+
 }
 
 static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
@@ -4965,7 +4975,7 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
 	for_each_sched_entity(se)
 		weight = __calc_prop_weight(cfs_rq_of(se), se, weight);
 
-	reweight_eevdf(&rq->cfs, &p->se, weight, p->se.on_rq);
+	reweight_eevdf(&rq->cfs, &p->se, weight, false);
 }
 
 static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
@@ -8200,7 +8210,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	weight = enqueue_hierarchy(p, flags);
 
 	if (!curr) {
-		reweight_eevdf(cfs_rq, se, weight, false);
+		reweight_eevdf(cfs_rq, se, weight, task_current_donor(rq, p));
 		place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
 		__enqueue_entity(cfs_rq, se);
 	}
@@ -15335,7 +15345,7 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
 		}
 
 		se = &curr->se;
-		reweight_eevdf(cfs_rq, se, weight, se->on_rq);
+		reweight_eevdf(cfs_rq, se, weight, false);
 	}
 
 	if (queued)
@@ -15542,7 +15552,7 @@ static void set_next_task_fair(struct rq *rq, struct task_struct *p, enum snt_e
 	cfs_rq->curr = se;
 
 	if (on_rq) {
-		reweight_eevdf(cfs_rq, se, weight, se->on_rq);
+		reweight_eevdf(cfs_rq, se, weight, false);
 		if (first)
 			set_protect_slice(cfs_rq, se);
 	}
-- 
2.53.0


> 
> > 
> > > +
> > > +               if (live) {
> > > +                       old_weight = se->h_load.weight;
> > > +                       dist = se->vprot - se->vruntime;
> > > +               }
> > >                 reweight_eevdf(cfs_rq, se, weight, false);
> > >                 place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> > > +
> > > +               if (live) {
> > > +                       if (weight != old_weight)
> > > +                               dist = div64_long(dist * old_weight, weight);
> > > +                       se->vprot = se->vruntime + dist;
> > > +               } else if (running) {
> > > +                       cancel_protect_slice(se);
> > > +               }
> > > +
> > >                 __enqueue_entity(cfs_rq, se);
> > >         }
> > >
> > > --
> > > 2.53.0
> > >
> 
> Thanks for the review!

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Kayra Cizmeci]

On Fri, Oct 16, 2026 at 12:27:16PM +0200, Vincent Guittot wrote:
> Le vendredi 16 oct. 2026 à 12:45:45 (+0300), Kayra Cizmeci a écrit :


> > >  se != curr so why is this needed?
> > 
> > sched_change_begin() sets .running as task_current_donor(), then if
> > .running is true calls put_prev_task(). put_prev_task_fair() sets
> > cfs_rq->curr to NULL. While the task still is the donor. Then we enter
> > sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
> > the donor is still p. There is set_next_task() call afterwards, but it
> > does not sets protection since its called with SNT_NORMAL.
> 
> okay, but we don't want to spread this all over the code.
> reweight_eevdf/scale_entity is  doing this already.
> 
> The below should do the stuff while keeping the computation in reweight_eevdf:
> 
> ---
>  kernel/sched/fair.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0feec172fd37..c212c6aa9691 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4863,10 +4863,11 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
>  }
>  
>  static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> -			   unsigned long weight, bool on_rq)
> +			   unsigned long weight, bool update_vprot)
>  {
>  	bool curr = cfs_rq->curr == se;
>  	bool rel_vprot = false;
> +	bool on_rq = se->on_rq;
>  	u64 avruntime = 0;
>  
>  	if (se->h_load.weight == weight)
> @@ -4878,22 +4879,25 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  		se->deadline -= avruntime;
>  		se->rel_deadline = 1;
>  		if (curr && protect_slice(se)) {
> -			se->vprot -= avruntime;
>  			rel_vprot = true;
>  		}
>  
>  		cfs_rq->h_nr_queued--;
>  		if (!curr)
>  			__dequeue_entity(cfs_rq, se);
> +	} else if (update_vprot) {
> +		avruntime = se->vruntime;
> +		rel_vprot = true;
>  	}
>  
> +	if (rel_vprot)
> +		se->vprot -= avruntime;
> +
>  	rescale_entity(se, weight, rel_vprot);
>  
>  	update_load_set(&se->h_load, weight);
>  
>  	if (on_rq) {
> -		if (rel_vprot)
> -			se->vprot += avruntime;
>  		se->deadline += avruntime;
>  		se->rel_deadline = 0;
>  		se->vruntime = avruntime - se->vlag;
> @@ -4902,6 +4906,12 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  			__enqueue_entity(cfs_rq, se);
>  		cfs_rq->h_nr_queued++;
>  	}
> +
> +	if (rel_vprot)
> +		se->vprot += avruntime;
> +	else if (curr || update_vprot)
> +		se->vprot = se->vruntime;
> +
>  }
>  
>  static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
> @@ -4965,7 +4975,7 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
>  	for_each_sched_entity(se)
>  		weight = __calc_prop_weight(cfs_rq_of(se), se, weight);
>  
> -	reweight_eevdf(&rq->cfs, &p->se, weight, p->se.on_rq);
> +	reweight_eevdf(&rq->cfs, &p->se, weight, false);
>  }
>  
>  static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
> @@ -8200,7 +8210,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	weight = enqueue_hierarchy(p, flags);
>  
>  	if (!curr) {
> -		reweight_eevdf(cfs_rq, se, weight, false);
> +		reweight_eevdf(cfs_rq, se, weight, task_current_donor(rq, p));
>  		place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
>  		__enqueue_entity(cfs_rq, se);
>  	}

On the old code on_rq was always false. But because enqueue_hierarchy(),
se->on_rq is always true. So this just goes to the other path in
reweight_eevdf().

Also, vruntime can move in place_entity() too.

Thanks!
Kayra

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Vincent Guittot]

On Fri, 16 Oct 2026 at 12:51, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
>
> On Fri, Oct 16, 2026 at 12:27:16PM +0200, Vincent Guittot wrote:
> > Le vendredi 16 oct. 2026 à 12:45:45 (+0300), Kayra Cizmeci a écrit :
>
>
> > > >  se != curr so why is this needed?
> > >
> > > sched_change_begin() sets .running as task_current_donor(), then if
> > > .running is true calls put_prev_task(). put_prev_task_fair() sets
> > > cfs_rq->curr to NULL. While the task still is the donor. Then we enter
> > > sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
> > > the donor is still p. There is set_next_task() call afterwards, but it
> > > does not sets protection since its called with SNT_NORMAL.
> >
> > okay, but we don't want to spread this all over the code.
> > reweight_eevdf/scale_entity is  doing this already.
> >
> > The below should do the stuff while keeping the computation in reweight_eevdf:
> >
> > ---
> >  kernel/sched/fair.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 0feec172fd37..c212c6aa9691 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4863,10 +4863,11 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
> >  }
> >
> >  static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > -                        unsigned long weight, bool on_rq)
> > +                        unsigned long weight, bool update_vprot)
> >  {
> >       bool curr = cfs_rq->curr == se;
> >       bool rel_vprot = false;
> > +     bool on_rq = se->on_rq;
> >       u64 avruntime = 0;
> >
> >       if (se->h_load.weight == weight)
> > @@ -4878,22 +4879,25 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> >               se->deadline -= avruntime;
> >               se->rel_deadline = 1;
> >               if (curr && protect_slice(se)) {
> > -                     se->vprot -= avruntime;
> >                       rel_vprot = true;
> >               }
> >
> >               cfs_rq->h_nr_queued--;
> >               if (!curr)
> >                       __dequeue_entity(cfs_rq, se);
> > +     } else if (update_vprot) {
> > +             avruntime = se->vruntime;
> > +             rel_vprot = true;
> >       }
> >
> > +     if (rel_vprot)
> > +             se->vprot -= avruntime;
> > +
> >       rescale_entity(se, weight, rel_vprot);
> >
> >       update_load_set(&se->h_load, weight);
> >
> >       if (on_rq) {
> > -             if (rel_vprot)
> > -                     se->vprot += avruntime;
> >               se->deadline += avruntime;
> >               se->rel_deadline = 0;
> >               se->vruntime = avruntime - se->vlag;
> > @@ -4902,6 +4906,12 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> >                       __enqueue_entity(cfs_rq, se);
> >               cfs_rq->h_nr_queued++;
> >       }
> > +
> > +     if (rel_vprot)
> > +             se->vprot += avruntime;
> > +     else if (curr || update_vprot)
> > +             se->vprot = se->vruntime;
> > +
> >  }
> >
> >  static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > @@ -4965,7 +4975,7 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
> >       for_each_sched_entity(se)
> >               weight = __calc_prop_weight(cfs_rq_of(se), se, weight);
> >
> > -     reweight_eevdf(&rq->cfs, &p->se, weight, p->se.on_rq);
> > +     reweight_eevdf(&rq->cfs, &p->se, weight, false);
> >  }
> >
> >  static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
> > @@ -8200,7 +8210,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >       weight = enqueue_hierarchy(p, flags);
> >
> >       if (!curr) {
> > -             reweight_eevdf(cfs_rq, se, weight, false);
> > +             reweight_eevdf(cfs_rq, se, weight, task_current_donor(rq, p));
> >               place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> >               __enqueue_entity(cfs_rq, se);
> >       }
>
> On the old code on_rq was always false. But because enqueue_hierarchy(),
> se->on_rq is always true. So this just goes to the other path in
> reweight_eevdf().

I changed the interface and the last arg is no more on_rq but force
the update of vprot

>
> Also, vruntime can move in place_entity() too.

But still, this is horrible...

Also, after more thinking, I don't think trying to keep vprot correct
in all cases is worth the effort, nor is it achievable without cost.
When one changes the prio and/or the slice of the running task, that
task might no longer be the best one to pick anyway. We should just do
our best effort like other places where we only decrease or cancel
protection to ensure it will not have an extended protection slice.

>
> Thanks!
> Kayra

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Kayra Cizmeci]

On Fri, Oct 16, 2026 at 04:06:47PM +0200, Vincent Guittot wrote:
> On Fri, 16 Oct 2026 at 12:51, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> >
> > On Fri, Oct 16, 2026 at 12:27:16PM +0200, Vincent Guittot wrote:
> > > Le vendredi 16 oct. 2026 à 12:45:45 (+0300), Kayra Cizmeci a écrit :
> >
> >
> > > > >  se != curr so why is this needed?
> > > >
> > > > sched_change_begin() sets .running as task_current_donor(), then if
> > > > .running is true calls put_prev_task(). put_prev_task_fair() sets
> > > > cfs_rq->curr to NULL. While the task still is the donor. Then we enter
> > > > sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
> > > > the donor is still p. There is set_next_task() call afterwards, but it
> > > > does not sets protection since its called with SNT_NORMAL.
> > >
> > > okay, but we don't want to spread this all over the code.
> > > reweight_eevdf/scale_entity is  doing this already.
> > >
> > > The below should do the stuff while keeping the computation in reweight_eevdf:
> > >
> > > ---
> > >  kernel/sched/fair.c | 26 ++++++++++++++++++--------
> > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index 0feec172fd37..c212c6aa9691 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -4863,10 +4863,11 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
> > >  }
> > >
> > >  static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > -                        unsigned long weight, bool on_rq)
> > > +                        unsigned long weight, bool update_vprot)
> > >  {
> > >       bool curr = cfs_rq->curr == se;
> > >       bool rel_vprot = false;
> > > +     bool on_rq = se->on_rq;
> > >       u64 avruntime = 0;
> > >
> > >       if (se->h_load.weight == weight)
> > > @@ -4878,22 +4879,25 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > >               se->deadline -= avruntime;
> > >               se->rel_deadline = 1;
> > >               if (curr && protect_slice(se)) {
> > > -                     se->vprot -= avruntime;
> > >                       rel_vprot = true;
> > >               }
> > >
> > >               cfs_rq->h_nr_queued--;
> > >               if (!curr)
> > >                       __dequeue_entity(cfs_rq, se);
> > > +     } else if (update_vprot) {
> > > +             avruntime = se->vruntime;
> > > +             rel_vprot = true;
> > >       }
> > >
> > > +     if (rel_vprot)
> > > +             se->vprot -= avruntime;
> > > +
> > >       rescale_entity(se, weight, rel_vprot);
> > >
> > >       update_load_set(&se->h_load, weight);
> > >
> > >       if (on_rq) {
> > > -             if (rel_vprot)
> > > -                     se->vprot += avruntime;
> > >               se->deadline += avruntime;
> > >               se->rel_deadline = 0;
> > >               se->vruntime = avruntime - se->vlag;
> > > @@ -4902,6 +4906,12 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > >                       __enqueue_entity(cfs_rq, se);
> > >               cfs_rq->h_nr_queued++;
> > >       }
> > > +
> > > +     if (rel_vprot)
> > > +             se->vprot += avruntime;
> > > +     else if (curr || update_vprot)
> > > +             se->vprot = se->vruntime;
> > > +
> > >  }
> > >
> > >  static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > @@ -4965,7 +4975,7 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
> > >       for_each_sched_entity(se)
> > >               weight = __calc_prop_weight(cfs_rq_of(se), se, weight);
> > >
> > > -     reweight_eevdf(&rq->cfs, &p->se, weight, p->se.on_rq);
> > > +     reweight_eevdf(&rq->cfs, &p->se, weight, false);
> > >  }
> > >
> > >  static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
> > > @@ -8200,7 +8210,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > >       weight = enqueue_hierarchy(p, flags);
> > >
> > >       if (!curr) {
> > > -             reweight_eevdf(cfs_rq, se, weight, false);
> > > +             reweight_eevdf(cfs_rq, se, weight, task_current_donor(rq, p));
> > >               place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> > >               __enqueue_entity(cfs_rq, se);
> > >       }
> >
> > On the old code on_rq was always false. But because enqueue_hierarchy(),
> > se->on_rq is always true. So this just goes to the other path in
> > reweight_eevdf().
> 
> I changed the interface and the last arg is no more on_rq but force
> the update of vprot
> 
> >
> > Also, vruntime can move in place_entity() too.
> 
> But still, this is horrible...

Yeah. There is really no clean way of fixing vprot in all the ways.
Just a cleaner way than the other.

> 
> Also, after more thinking, I don't think trying to keep vprot correct
> in all cases is worth the effort, nor is it achievable without cost.
> When one changes the prio and/or the slice of the running task, that
> task might no longer be the best one to pick anyway. We should just do
> our best effort like other places where we only decrease or cancel
> protection to ensure it will not have an extended protection slice.

So something like below for !curr path?

	if (!curr) {
		reweight_eevdf(cfs_rq, se, weight, false);
		place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
		if (task_current_donor(rq, p))
			cancel_protect_slice(se);
		__enqueue_entity(cfs_rq, se);
	}

Does this work for you? And keeping the reweight_eevdf() part as is?
BTW, I didn't typed a comment right now, so if you say 'yes' I'll add a comment
above how even task_current_donor() can return true on the !curr path.

Thanks!

Kayra


> 
> >
> > Thanks!
> > Kayra

Re: [PATCH v3] sched/eevdf: Fix vprot across reweighting

[Vincent Guittot]

On Fri, 16 Oct 2026 at 16:22, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
>
> On Fri, Oct 16, 2026 at 04:06:47PM +0200, Vincent Guittot wrote:
> > On Fri, 16 Oct 2026 at 12:51, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> > >
> > > On Fri, Oct 16, 2026 at 12:27:16PM +0200, Vincent Guittot wrote:
> > > > Le vendredi 16 oct. 2026 à 12:45:45 (+0300), Kayra Cizmeci a écrit :
> > >
> > >
> > > > > >  se != curr so why is this needed?
> > > > >
> > > > > sched_change_begin() sets .running as task_current_donor(), then if
> > > > > .running is true calls put_prev_task(). put_prev_task_fair() sets
> > > > > cfs_rq->curr to NULL. While the task still is the donor. Then we enter
> > > > > sched_change_end(), we call enqueue_task() with cfs_rq->curr == NULL but
> > > > > the donor is still p. There is set_next_task() call afterwards, but it
> > > > > does not sets protection since its called with SNT_NORMAL.
> > > >
> > > > okay, but we don't want to spread this all over the code.
> > > > reweight_eevdf/scale_entity is  doing this already.
> > > >
> > > > The below should do the stuff while keeping the computation in reweight_eevdf:
> > > >
> > > > ---
> > > >  kernel/sched/fair.c | 26 ++++++++++++++++++--------
> > > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > index 0feec172fd37..c212c6aa9691 100644
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -4863,10 +4863,11 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
> > > >  }
> > > >
> > > >  static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > > -                        unsigned long weight, bool on_rq)
> > > > +                        unsigned long weight, bool update_vprot)
> > > >  {
> > > >       bool curr = cfs_rq->curr == se;
> > > >       bool rel_vprot = false;
> > > > +     bool on_rq = se->on_rq;
> > > >       u64 avruntime = 0;
> > > >
> > > >       if (se->h_load.weight == weight)
> > > > @@ -4878,22 +4879,25 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > >               se->deadline -= avruntime;
> > > >               se->rel_deadline = 1;
> > > >               if (curr && protect_slice(se)) {
> > > > -                     se->vprot -= avruntime;
> > > >                       rel_vprot = true;
> > > >               }
> > > >
> > > >               cfs_rq->h_nr_queued--;
> > > >               if (!curr)
> > > >                       __dequeue_entity(cfs_rq, se);
> > > > +     } else if (update_vprot) {
> > > > +             avruntime = se->vruntime;
> > > > +             rel_vprot = true;
> > > >       }
> > > >
> > > > +     if (rel_vprot)
> > > > +             se->vprot -= avruntime;
> > > > +
> > > >       rescale_entity(se, weight, rel_vprot);
> > > >
> > > >       update_load_set(&se->h_load, weight);
> > > >
> > > >       if (on_rq) {
> > > > -             if (rel_vprot)
> > > > -                     se->vprot += avruntime;
> > > >               se->deadline += avruntime;
> > > >               se->rel_deadline = 0;
> > > >               se->vruntime = avruntime - se->vlag;
> > > > @@ -4902,6 +4906,12 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > >                       __enqueue_entity(cfs_rq, se);
> > > >               cfs_rq->h_nr_queued++;
> > > >       }
> > > > +
> > > > +     if (rel_vprot)
> > > > +             se->vprot += avruntime;
> > > > +     else if (curr || update_vprot)
> > > > +             se->vprot = se->vruntime;
> > > > +
> > > >  }
> > > >
> > > >  static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
> > > > @@ -4965,7 +4975,7 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
> > > >       for_each_sched_entity(se)
> > > >               weight = __calc_prop_weight(cfs_rq_of(se), se, weight);
> > > >
> > > > -     reweight_eevdf(&rq->cfs, &p->se, weight, p->se.on_rq);
> > > > +     reweight_eevdf(&rq->cfs, &p->se, weight, false);
> > > >  }
> > > >
> > > >  static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
> > > > @@ -8200,7 +8210,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > > >       weight = enqueue_hierarchy(p, flags);
> > > >
> > > >       if (!curr) {
> > > > -             reweight_eevdf(cfs_rq, se, weight, false);
> > > > +             reweight_eevdf(cfs_rq, se, weight, task_current_donor(rq, p));
> > > >               place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> > > >               __enqueue_entity(cfs_rq, se);
> > > >       }
> > >
> > > On the old code on_rq was always false. But because enqueue_hierarchy(),
> > > se->on_rq is always true. So this just goes to the other path in
> > > reweight_eevdf().
> >
> > I changed the interface and the last arg is no more on_rq but force
> > the update of vprot
> >
> > >
> > > Also, vruntime can move in place_entity() too.
> >
> > But still, this is horrible...
>
> Yeah. There is really no clean way of fixing vprot in all the ways.
> Just a cleaner way than the other.
>
> >
> > Also, after more thinking, I don't think trying to keep vprot correct
> > in all cases is worth the effort, nor is it achievable without cost.
> > When one changes the prio and/or the slice of the running task, that
> > task might no longer be the best one to pick anyway. We should just do
> > our best effort like other places where we only decrease or cancel
> > protection to ensure it will not have an extended protection slice.
>
> So something like below for !curr path?
>
>         if (!curr) {
>                 reweight_eevdf(cfs_rq, se, weight, false);
>                 place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
>                 if (task_current_donor(rq, p))
>                         cancel_protect_slice(se);
>                 __enqueue_entity(cfs_rq, se);
>         }
>
> Does this work for you? And keeping the reweight_eevdf() part as is?

yes, looks good for me

> BTW, I didn't typed a comment right now, so if you say 'yes' I'll add a comment
> above how even task_current_donor() can return true on the !curr path.

Thanks
Vincent

>
> Thanks!
>
> Kayra
>
>
> >
> > >
> > > Thanks!
> > > Kayra