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

[Peter Zijlstra <peterz@infradead.org>]

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.

> > +	}
> >  
> >  	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.