Re: [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS

[Andrea Righi <arighi@nvidia.com>]

On Fri, Jul 31, 2026 at 10:51:39AM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 7/30/2026 7:28 PM, Andrea Righi wrote:
> > diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> > index b215b0ead9a60..8fb8474d0a0ec 100644
> > --- a/kernel/sched/syscalls.c
> > +++ b/kernel/sched/syscalls.c
> > @@ -645,12 +645,19 @@ int __sched_setscheduler(struct task_struct *p,
> >  		goto recheck;
> >  	}
> >  
> > +	/* KEEP_PARAMS only makes sense if the scheduling policy is unchanged */
> > +	if ((attr->sched_flags & SCHED_FLAG_KEEP_PARAMS) && policy != p->policy) {
> > +		retval = -EINVAL;
> > +		goto unlock;
> > +	}
> > +
> >  	/*
> >  	 * If setscheduling to SCHED_DEADLINE (or changing the parameters
> >  	 * of a SCHED_DEADLINE task) we need to check if enough bandwidth
> >  	 * is available.
> >  	 */
> > -	if ((dl_policy(policy) || dl_task(p)) && sched_dl_overflow(p, policy, attr)) {
> > +	if (!(attr->sched_flags & SCHED_FLAG_KEEP_PARAMS) &&
> > +	    (dl_policy(policy) || dl_task(p)) && sched_dl_overflow(p, policy, attr)) {
> >  		retval = -EBUSY;
> >  		goto unlock;
> >  	}
> 
> On an unrelated side note, similar concern exists for p->reset_on_fork and
> that it can be changed by a parallel sched_setscheduler() that finished
> before and the one that is lagging can continue with a stale copy.
> 
> reset_on_fork is computed outside the rq_lock for KEEP_POLICY case and
> p->reset_on_fork will be set to that if nothing else changes (same policy,
> same attributes, no uclamp changes) in the early unlock case.

This looks like another race in the KEEP_POLICY path: p->sched_reset_on_fork is
sampled before taking the rq lock, while the locked recheck only detects changes
to p->policy, so a concurrent same-policy update can be overwritten by the stale
snapshot. We should probably address this in a separate fix.

> 
> Peter, is that a concern?
> 
> > @@ -675,7 +682,7 @@ int __sched_setscheduler(struct task_struct *p,
> >  	prev_class = p->sched_class;
> >  	next_class = __setscheduler_class(policy, newprio);
> >  
> > -	if (prev_class != next_class)
> > +	if (!(attr->sched_flags & SCHED_FLAG_KEEP_PARAMS) && prev_class != next_class)
> >  		queue_flags |= DEQUEUE_CLASS;
> 
> Can this happen if we've already ensured policy is unchanged under
> rq_lock for KEEP_PARAMS? The
> 
>     newprio = __normal_prio(policy, ...);
> 
> above would have fixed it under the rq_lock right based on policy
> right?

For the traditional DL/RT/fair mapping, yes: with the policy unchanged and after
accounting for PI in newprio, the resulting class should match p->sched_class.

However, __setscheduler_class() also depends on task_should_scx(policy). During
sched_ext enable/disable, the global state consulted by task_should_scx() is
updated before all tasks have been migrated. Therefore, __setscheduler_class()
can temporarily return ext_sched_class while p->sched_class is still
fair_sched_class, or vice versa, even though policy == p->policy.

Since KEEP_PARAMS deliberately skips the p->sched_class = next_class assignment,
it must also suppress DEQUEUE_CLASS. Otherwise, class-transition callbacks could
run while p->sched_class remains unchanged.

> 
> >  
> >  	scoped_guard (sched_change, p, queue_flags) {
> 
> -- 
> Thanks and Regards,
> Prateek
> 

Thanks,
-Andrea