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

[K Prateek Nayak <kprateek.nayak@amd.com>]

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?

> 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".

>  	}
> -	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?

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);


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?

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

-- 
Thanks and Regards,
Prateek