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

[Kayra Cizmeci <kayracizmeci@gmail.com>]

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