Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext

[Andrea Righi <arighi@nvidia.com>]

On Thu, Nov 12, 2026 at 05:41:49PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 05:32:40PM +0100, Andrea Righi wrote:
> 
> > > 
> > > ---
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index 1a088d4576f5..1c1e669473a6 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -11252,8 +11252,13 @@ void sched_change_end(struct sched_change_ctx *ctx)
> > >  	 */
> > >  	WARN_ON_ONCE(p->sched_class != ctx->class && !(ctx->flags & ENQUEUE_CLASS));
> > >  
> > > -	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to)
> > > -		p->sched_class->switching_to(rq, p);
> > > +	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to) {
> > > +		bool block = p->sched_class->switching_to(rq, p);
> > > +		if (block && p->is_blocked && ctx->queued) {
> > > +			block_task(rq, p, READ_ONCE(p->__state));
> > > +			ctx->queued = false;
> > > +		}
> > 
> > So at this point sched_change_begin() has already dequeued p from the
> > old class, but p hasn't been enqueued in the new class yet, block_task()
> > would dequeue it again, through the new class, and repeat the generic
> > dequeue accounting. I think we should complete the block without another
> > dequeue here.
> > 
> > Also, if p is rq->donor, clearing ctx->queued isn't enough: we need to
> > reset the donor and clear ctx->running, otherwise sched_change_end()
> > would call set_next_task(rq, p) for the task we just blocked.
> 
> Right, I figured there was something missing. Can you take this patch
> and make it work?; you are more proficient with running this ext stuff
> than me.

Ack, I can take it from here and test all the nasty ext edge cases. :)

> 
> > > +	}
> > >  
> > >  	if (ctx->queued)
> > >  		enqueue_task(rq, p, ctx->flags);
> > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > > index f74ffb3b1aca..4440e32236b3 100644
> > > --- a/kernel/sched/ext/ext.c
> > > +++ b/kernel/sched/ext/ext.c
> 
> > > @@ -8947,10 +8934,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
> > >  
> > >  		if (old_class != new_class)
> > >  			queue_flags |= DEQUEUE_CLASS;
> > > -		if (new_class == &ext_sched_class) {
> > > -			scx_prepare_task_sched_change(p);
> > > +		if (new_class == &ext_sched_class)
> > >  			queue_flags |= DEQUEUE_NOCLOCK;
> > > -		}
> > 
> > scx_prepare_task_sched_change() also calls update_rq_clock(). With
> > that call removed, keeping DEQUEUE_NOCLOCK makes sched_change_begin()
> > skip the clock update even though we haven't done it here.
> > 
> > Maybe we should drop this flag too and let sched_change_begin() update
> > the clock.
> 
> Sure.
> 
> > > diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> > > index b5d92d89629b..e73f3474d685 100644
> > > --- a/kernel/sched/ext/sub.c
> > > +++ b/kernel/sched/ext/sub.c
> > > @@ -1647,7 +1647,6 @@ static void scx_rehome_task(struct scx_sched *to, struct task_struct *p)
> > >  	lockdep_assert_held(&p->pi_lock);
> > >  	lockdep_assert_rq_held(task_rq(p));
> > >  
> > > -	scx_prepare_task_sched_change(p);
> > >  	scoped_guard (sched_change, p,
> > >  		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
> > 
> > With sub-schedulers these paths can move a task between BPF schedulers
> > without changing p->sched_class, so switching_to() won't run. We still
> > need to end the old proxy session before changing ownership, even if the
> > new BPF scheduler also supports proxy-exec.
> > 
> > Can we keep scx_prepare_task_sched_change() for these ownership changes
> > and use switching_to() for the actual class transitions?
> 
> ARGH, are you *sure* you want to support crazy like that? Just have the
> thing mandate consistent proxy support.
> 
> Enforce the primary scheduler's take when loading sub-schedulers. If it
> does support proxy, dis-allow loading sub scheds that don't and
> vice-versa.
> 
> Alternatively, make the support the logical and of all sub-scheduler
> support. Ergo if one sub sched doesn't want to support proxy, nobody
> gets it.

Yeah, fair point, mixed proxy support across sub-schedulers makes this
unnecessarily complicated for zero benefit. It seems more reasonable to
make proxy exec opt-in hierarchical.

I'll look into this along with the switching_to() changes.

Thanks,
-Andrea