Re: [PATCH 01/14] sched: Employ sched_change guards

[Peter Zijlstra]

On Mon, Oct 06, 2025 at 08:51:27PM +0530, Shrikanth Hegde wrote:
> 
> 
> On 9/10/25 9:14 PM, Peter Zijlstra wrote:
> > As proposed a long while ago -- and half done by scx -- wrap the
> > scheduler's 'change' pattern in a guard helper.
> > 
> [...]>   		put_task_struct(p);
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -3860,23 +3860,22 @@ extern void check_class_changed(struct r
> >   extern struct balance_callback *splice_balance_callbacks(struct rq *rq);
> >   extern void balance_callbacks(struct rq *rq, struct balance_callback *head);
> > -#ifdef CONFIG_SCHED_CLASS_EXT
> > -/*
> > - * Used by SCX in the enable/disable paths to move tasks between sched_classes
> > - * and establish invariants.
> > - */
> > -struct sched_enq_and_set_ctx {
> > +struct sched_change_ctx {
> >   	struct task_struct	*p;
> > -	int			queue_flags;
> > +	int			flags;
> >   	bool			queued;
> >   	bool			running;
> >   };
> > -void sched_deq_and_put_task(struct task_struct *p, int queue_flags,
> > -			    struct sched_enq_and_set_ctx *ctx);
> > -void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);
> > +struct sched_change_ctx *sched_change_begin(struct task_struct *p, unsigned int flags);
> > +void sched_change_end(struct sched_change_ctx *ctx);
> > -#endif /* CONFIG_SCHED_CLASS_EXT */
> > +DEFINE_CLASS(sched_change, struct sched_change_ctx *,
> > +	     sched_change_end(_T),
> > +	     sched_change_begin(p, flags),
> > +	     struct task_struct *p, unsigned int flags)
> > +
> > +DEFINE_CLASS_IS_UNCONDITIONAL(sched_change)
> >   #include "ext.h"
> could you please add a comment on matching flags on dequeue/enqueue
> here?

Would something like so be okay? This assumes at least the second patch
is applied as well.

---

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10783,6 +10783,12 @@ struct sched_change_ctx *sched_change_be
 	struct sched_change_ctx *ctx = this_cpu_ptr(&sched_change_ctx);
 	struct rq *rq = task_rq(p);
 
+	/*
+	 * Must exclusively use matched flags since this is both dequeue and
+	 * enqueue.
+	 */
+	WARN_ON_ONCE(flags & 0xFFFF0000);
+
 	lockdep_assert_rq_held(rq);
 
 	if (!(flags & DEQUEUE_NOCLOCK)) {