All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS
@ 2026-07-30 13:58 Andrea Righi
  2026-07-31  5:21 ` K Prateek Nayak
  0 siblings, 1 reply; 3+ messages in thread
From: Andrea Righi @ 2026-07-30 13:58 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Tejun Heo, Patrick Bellasi,
	linux-kernel

SCHED_FLAG_KEEP_PARAMS prevents __sched_setscheduler() from applying the
requested scheduler parameters, policy and class. However, a different
requested policy can still trigger deadline bandwidth accounting and
scheduling class callbacks.

Reject policy changes with SCHED_FLAG_KEEP_PARAMS while holding the
task's rq lock. Also skip deadline bandwidth accounting and class
transition callbacks when the guarded scheduler update cannot apply the
corresponding changes.

This prevents deadline bandwidth from being accounted without changing
the task and avoids class callbacks when p->sched_class remains
unchanged.

Fixes: a509a7cd7974 ("sched/uclamp: Extend sched_setattr() to support utilization clamping")
Fixes: 637b0682821b ("sched: Fold sched_class::switch{ing,ed}_{to,from}() into the change pattern")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
Changes in v2:
 - Reject policy changes with SCHED_FLAG_KEEP_PARAMS (Peter Zijlstra)
 - Perform the policy check under the rq lock to avoid racing with a concurrent
   policy change
 - Link to v1: https://lore.kernel.org/all/20260730055011.2267333-1-arighi@nvidia.com/

 kernel/sched/syscalls.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

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;
 	}
@@ -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;
 
 	scoped_guard (sched_change, p, queue_flags) {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS
  2026-07-30 13:58 [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS Andrea Righi
@ 2026-07-31  5:21 ` K Prateek Nayak
  2026-07-31 12:16   ` Andrea Righi
  0 siblings, 1 reply; 3+ messages in thread
From: K Prateek Nayak @ 2026-07-31  5:21 UTC (permalink / raw)
  To: Andrea Righi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Tejun Heo, Patrick Bellasi, linux-kernel

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.

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?

>  
>  	scoped_guard (sched_change, p, queue_flags) {

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS
  2026-07-31  5:21 ` K Prateek Nayak
@ 2026-07-31 12:16   ` Andrea Righi
  0 siblings, 0 replies; 3+ messages in thread
From: Andrea Righi @ 2026-07-31 12:16 UTC (permalink / raw)
  To: K Prateek Nayak
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Tejun Heo, Patrick Bellasi, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-31 12:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 13:58 [PATCH v2] sched: Reject policy changes with SCHED_FLAG_KEEP_PARAMS Andrea Righi
2026-07-31  5:21 ` K Prateek Nayak
2026-07-31 12:16   ` Andrea Righi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.