Re: [PATCH 02/18] sched/core: Dequeue waking proxy donors before reset
From: K Prateek Nayak
Date: Tue Sep 01 2026 - 01:25:09 EST
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@xxxxxxxxxx>
> ---
> 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