Re: [PATCH 02/18] sched/core: Dequeue waking proxy donors before reset

[Andrea Righi <arighi@nvidia.com>]

Hi Prateek,

On Tue, Sep 01, 2026 at 10:54:44AM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 8/31/2026 7:12 PM, Andrea Righi wrote:
> > proxy_needs_return() resets an active donor while holding blocked_lock.
> > proxy_reset_donor() invokes scheduling-class callbacks, adding an
> > unnecessary raw-spinlock nesting. It also presents the waking donor to
> > put_prev_task() as still runnable immediately before block_task()
> > removes it from the runqueue.
> 
> Is that an issue for scx?

Yes. When a proxy-migrated donor wakes while it's still rq->donor,
proxy_needs_return() has to reset the rq's donor before clearing the task's
generic on_rq state and returning it through the full wakeup path.

proxy_reset_donor() calls put_prev_set_next_task(), which invokes
put_prev_task_scx() for an EXT donor. If this happens before the
scheduling-class dequeue, SCX_TASK_QUEUED and is_blocked are still set, so
put_prev_task_scx() re-enqueues the donor through scx_do_enqueue_task() and the
following block_task() immediately dequeues it again.

I can clarify this sched_ext-specific ordering better in the patch description.

> 
> > Split block_task() so the waking donor can first be dequeued from its
> > scheduling class. Release blocked_lock, dequeue the donor while its
> > generic on_rq state still prevents migration, replace all donor
> > references, and only then complete the generic runqueue removal. This
> > follows the normal sleep ordering and avoids transiently re-enqueuing
> > the waking donor.
> > 
> > This is a preparatory change to support proxy execution with sched_ext.
> > 
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >  kernel/sched/core.c | 30 +++++++++++++++++++++++-------
> >  1 file changed, 23 insertions(+), 7 deletions(-)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 5817d1a4cea2c..237d216382f46 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -2252,7 +2252,8 @@ void deactivate_task(struct rq *rq, struct task_struct *p, int flags)
> >  	dequeue_task(rq, p, flags);
> >  }
> >  
> > -static void block_task(struct rq *rq, struct task_struct *p, unsigned long task_state)
> > +static bool dequeue_block_task(struct rq *rq, struct task_struct *p,
> > +			       unsigned long task_state)
> >  {
> >  	int flags = DEQUEUE_NOCLOCK;
> >  
> > @@ -2273,9 +2274,15 @@ static void block_task(struct rq *rq, struct task_struct *p, unsigned long task_
> >  	 *
> >  	 * Where __schedule() and ttwu() have matching control dependencies.
> >  	 *
> > -	 * After this, schedule() must not care about p->state any more.
> > +	 * Once the caller invokes __block_task(), schedule() must not care about
> > +	 * p->state any more.
> >  	 */
> > -	if (dequeue_task(rq, p, DEQUEUE_SLEEP | flags))
> > +	return dequeue_task(rq, p, DEQUEUE_SLEEP | flags);
> > +}
> > +
> > +static void block_task(struct rq *rq, struct task_struct *p, unsigned long task_state)
> > +{
> > +	if (dequeue_block_task(rq, p, task_state))
> >  		__block_task(rq, p);
> >  }
> >  
> > @@ -3774,6 +3781,9 @@ static inline void proxy_reset_donor(struct rq *rq)
> >   */
> >  static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
> >  {
> > +	bool reset_donor = false;
> > +	bool dequeued;
> > +
> >  	/*
> >  	 * Typically per __set_task_cpu(), task_cpu(p) == p->wake_cpu.
> >  	 *
> > @@ -3797,11 +3807,17 @@ static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
> >  		if (task_current(rq, p))
> >  			return false;
> >  
> > -		/* If we're return migrating the rq->donor, switch it out for idle */
> > -		if (task_current_donor(rq, p))
> > -			proxy_reset_donor(rq);
> > +		reset_donor = task_current_donor(rq, p);
> 
> nit.
> 
> Since proxy_needs_return() holds the rq_lock, you can check this outside
> the blocked_lock safely, even after the dequeue. There is no need to
> stash "reset_donor".

Agreed, rq->donor is stable while the rq lock is held, so this can use
task_current_donor(rq, p) directly after dequeue_block_task() and remove
reset_donor.

> 
> >  	}
> > -	block_task(rq, p, TASK_WAKING);
> > +
> > +	dequeued = dequeue_block_task(rq, p, TASK_WAKING);
> > +
> > +	/* Keep on_rq set until all donor references have been replaced. */
> > +	if (reset_donor)
> > +		proxy_reset_donor(rq);
> 
> Since TASK_WAKING is guaranteed to block the task by adding
> DEQUEUE_SPECIAL, you can just move the proxy_reset_donor() bit out the
> blocked lock and keep everything else the same right?

We still need to perform the sched-class dequeue before proxy_reset_donor().

If proxy_reset_donor() runs first, put_prev_task_scx() sees the donor with
SCX_TASK_QUEUED and is_blocked still set and reenqueues it through
scx_do_enqueue_task(). Then the following block_task() dequeues it again.

> 
> I'm not sure I understand "transiently re-enqueuing the waking donor"
> bit. How is that possible if we just do:
> 
>     /* __task_rq_lock is held throughout. */
> 
>     if (task_current_donor(rq, p))
>         proxy_reset_donor(rq, p);
> 
>     block_task(rq, p, TASK_WAKING);

And the transient reenqueue happens inside proxy_reset_donor():

  proxy_reset_donor()
    put_prev_set_next_task()
      put_prev_task_scx()
        scx_do_enqueue_task()
          ops.enqueue()

__clear_task_blocked_on() clears the blocked_on relationship, but p->is_blocked
remains set until ttwu_do_wakeup(). As SCX_TASK_QUEUED is also still set,
put_prev_task_scx() takes its retained-blocked-donor path.

The subsequent block_task() would then immediately undo that enqueue.

> Is the split because dequeue_task_scx() needs a correct rq->donor
> reference or does ext requires dequeue_task_scx() to be called
> before doing put_prev_task_scx() always?

Both are aspects of the same ordering requirement here.

dequeue_task_scx() must run with rq->donor == p so it recognizes p as the
current scheduling context, emits the appropriate stopping transition and clears
SCX_TASK_QUEUED. Then put_prev_task_scx() can run as part of proxy_reset_donor()
without placing the blocked donor back into a DSQ or calling ops.enqueue()
again.

And this is specific to resetting a retained donor, it's not a general
requirement that dequeue_task_scx() always precede put_prev_task_scx().

I'll remove the unnecessary reset_donor variable and expand the comment to make
this ordering more explicit.

Thanks!
-Andrea

> 
> > +
> > +	if (dequeued)
> > +		__block_task(rq, p);
> >  	return true;
> >  }
> >  #else /* !CONFIG_SCHED_PROXY_EXEC */
> 
> -- 
> Thanks and Regards,
> Prateek
>