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

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Kayra,

On 10/3/2026 5:16 PM, Kayra Cizmeci wrote:
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 7b91cae4a59c..bd9e2d119910 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -584,6 +584,7 @@ struct sched_entity {
>  	unsigned char			on_rq;
>  	unsigned char			sched_delayed;
>  	unsigned char			rel_deadline;
> +	unsigned char			rel_vprot;
>  	unsigned char			custom_slice;
>  					/* hole */
>  
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 71d3c948e14a..f1241f572cdc 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4638,6 +4638,7 @@ static void __sched_fork(u64 clone_flags, struct task_struct *p)
>  	p->se.vruntime			= 0;
>  	p->se.vlag			= 0;
>  	p->se.rel_deadline		= 0;
> +	p->se.rel_vprot			= 0;
>  	INIT_LIST_HEAD(&p->se.group_node);
>  
>  	/* A delayed task cannot be in clone(). */
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b42a0ca2a69e..0bcd3a77aba4 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4614,7 +4614,7 @@ dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  }
>  
>  static void
> -rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
> +rescale_entity(struct sched_entity *se, unsigned long weight)
>  {
>  	long old_weight = se->h_load.weight;
>  
> @@ -4712,7 +4712,7 @@ rescale_entity(struct sched_entity *se, unsigned long weight, bool rel_vprot)
>  	if (se->rel_deadline)
>  		se->deadline = div64_long(se->deadline * old_weight, weight);
>  
> -	if (rel_vprot)
> +	if (se->rel_vprot)
>  		se->vprot = div64_long(se->vprot * old_weight, weight);
>  }
>  
> @@ -4720,7 +4720,6 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  			   unsigned long weight, bool on_rq)
>  {
>  	bool curr = cfs_rq->curr == se;
> -	bool rel_vprot = false;
>  	u64 avruntime = 0;
>  
>  	if (se->h_load.weight == weight)
> @@ -4733,7 +4732,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  		se->rel_deadline = 1;
>  		if (curr && protect_slice(se)) {
>  			se->vprot -= avruntime;
> -			rel_vprot = true;
> +			se->rel_vprot = 1;
>  		}
>  
>  		cfs_rq->h_nr_queued--;
> @@ -4741,17 +4740,23 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  			__dequeue_entity(cfs_rq, se);
>  	}
>  
> -	rescale_entity(se, weight, rel_vprot);
> +	rescale_entity(se, weight);
>  
>  	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 (se->rel_vprot) {
> +			se->vprot += avruntime;
> +			se->rel_vprot = 0;
> +		} else if (curr) {
> +			/* Reweighting must not revive expired slice protection. */
> +			cancel_protect_slice(se);

This is the only genuine fix I see in this entire patch where
vruntime adjustment will make the tasks protected and we
probably need to cancel it in that case but i don't get why
we need a new se->rel_vprot for that ...

> +		}
> +
>  		if (!curr)
>  			__enqueue_entity(cfs_rq, se);
>  		cfs_rq->h_nr_queued++;
> @@ -6297,6 +6302,11 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  
>  	se->vruntime = vruntime - lag;
>  
> +	if (se->rel_vprot) {
> +		se->vprot += se->vruntime;
> +		se->rel_vprot = 0;
> +	}
> +
>  	if (update_zero)
>  		update_zero_vruntime(cfs_rq, -lag);
>  
> @@ -8143,9 +8153,13 @@ static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags)
>  
>  	dequeue_hierarchy(p, flags);
>  
> -	if (sched_feat(PLACE_REL_DEADLINE) && !task_sleep) {
> -		se->deadline -= se->vruntime;
> -		se->rel_deadline = 1;
> +	if (!task_sleep) {
> +		se->vprot = protect_slice(se) ? se->vprot - se->vruntime : 0;
> +		se->rel_vprot = 1;

I'm having a super hard time understanding why we need this.

Protect slice is only required for current entities so why do we
care about it during migration? The set_next_task() during next
selection will override it anyway.

What am I missing?

> +		if (sched_feat(PLACE_REL_DEADLINE)) {
> +			se->deadline -= se->vruntime;
> +			se->rel_deadline = 1;
> +		}
>  	}
>  	if (se != cfs_rq->curr)
>  		__dequeue_entity(cfs_rq, se);

-- 
Thanks and Regards,
Prateek