Re: [PATCH v2 4/5] sched/eevdf: Keep expired protection expired across reweighting

[Kayra Cizmeci]

Sorry for being a bit late :-(

> @@ -4941,6 +4941,9 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  		se->deadline += avruntime;
>  		se->rel_deadline = 0;
>  		se->vruntime = avruntime - se->vlag;
> +		/* Reweighting must not revive expired slice protection. */
> +		if (curr && !rel_vprot)
> +			se->vprot = se->vruntime;
>  
>  		if (!curr)
 			__enqueue_entity(cfs_rq, se);
> @@ -8204,7 +8207,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	struct sched_entity *se = &p->se;
>  	struct cfs_rq *cfs_rq = &rq->cfs;
>  	unsigned long weight;
> -	bool curr;
> +	bool curr, expired = false;
>  
>  	if (task_is_throttled(p) && enqueue_throttled_task(p))
>  		return;
> @@ -8237,6 +8240,8 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	 * XXX comment on the curr thing
>  	 */
>  	curr = (cfs_rq->curr == se);
> +	if (!curr && task_current_donor(rq, p))
> +		expired = !protect_slice(se);
>   	if (curr)
>  		place_entity(cfs_rq, se, flags);
> 

> @@ -8248,6 +8253,8 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	if (!curr) {
>  		reweight_eevdf(cfs_rq, se, weight, false);
>  		place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> +		if (expired)
> +			se->vprot = se->vruntime;
>  		__enqueue_entity(cfs_rq, se);
>  	}
>  
> -- 

There are a few thing here:

This (!curr) branch has been removed from tip sched/core, here: https://patch.msgid.link/20260911155449.1249726-1-kayracizmeci@xxxxxxxxx
So, heads-up.

Also, when we have alive (or live, as y'all call it, I think it's better this way tho :>) 
protection, and (!expired) reweight_eevdf() doesn't enter the on_rq path and
alive protection is not reweighted. Not tested tho. And I may be getting something wrong.
This is not about this patch tho.

Your commit says "Commit ff38424030f9 ("sched/eevdf: Update
se->vprot in reweight_entity()") subsequently handled live protection,
but left the expired case unchanged." too. So it's gotta change if I'm not missing something.

If I'm not tho that can mean a patch. :-). 

But IDK if that's intentional or not :-(. I think it's not but I'm not sure.

Anyway,

Thanks,

Regards,

Kayra :>