Re: [PATCH sched_ext/for-7.4] sched_ext: Keep proxy donors local through core-sched retries

[Tejun Heo <tj@kernel.org>]

Hello, Andrea.

The following is a Claude-generated review.

Everything below is from reading the code and nothing has been
reproduced.

On Thu, Oct 15, 2026 at 12:19:35PM +0200, Andrea Righi wrote:
> Fixes: 7ab553756463 ("sched_ext: Keep proxy donors with slice left on the local DSQ")

This commit doesn't exist. The parent landed in sched_ext/for-7.4 as
67d0d8d018f3.

> +	if (rq->donor == rq->idle) {
> +		struct task_struct *first;
> +
> +		first = list_first_entry_or_null(&rq->scx.local_dsq.list,
> +						 struct task_struct, scx.dsq_list.node);
> +		if (first && (first->scx.dsq_flags & SCX_TASK_DSQ_PROXY_IDLE) &&
> +		    sched_cpu_cookie_match(rq, first))
> +			return true;
> +	}

Does this need the mark? With rq->donor == rq->idle and next_class at
ext, the next pick takes the local DSQ head no matter how it got there.
Non-proxy IMMED tasks hit the same spurious reenq today. For example,
two IMMED wakeups land on an idle CPU. The first insert sees the rq open
and rq_owned_post_enq() raises next_class to ext. The second makes nr 2
and schedules a reenq, which task_woken_scx() runs. reenq_local() then
sees the rq closed and sends the first task back to BPF even though it's
at the head.

Testing just the head's cookie while the donor is idle would cover both,
and SCX_TASK_DSQ_PROXY_IDLE with its set and clear could go away.

Also, first_local_task() is the same lookup if moved up. As the block
only runs with next_class at ext, maybe it belongs below the "@rq is
either picking, in transition to, or running an SCX task." comment?

> +/*
> + * A proxy retry can bypass pick_task_scx() and leave PROXY_PENDING set.
> + * A cookie mismatch means core scheduling forced this CPU idle, not that
> + * proxy resolution put a selected donor through idle.
> + */
>  static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>  			      struct task_struct *next)
>  {
>  	struct scx_sched *sch = scx_task_sched(p);
>  	bool proxy_put = p->is_blocked && next == rq->idle &&
> -			 (rq->scx.flags & SCX_RQ_PROXY_PENDING);
> +			 (rq->scx.flags & SCX_RQ_PROXY_PENDING) &&
> +			 sched_cpu_cookie_match(rq, p);

A forced idle can also happen with a matching cookie. If the core-wide
max is untagged, core_cookie is 0 and pick_next_task() forces a
mismatching rq idle without trying sched_core_find(), so an untagged
donor still passes sched_cpu_cookie_match(). For example, with CFS
bandwidth control:

1. A tagged fair task H on this rq is throttled. A sibling's core-wide
   pick selects the untagged blocked donor D for this rq under an
   untagged max.

2. H is unthrottled. The unthrottle re-enqueues H with
   enqueue_task_fair(), which doesn't bump core_task_seq, so this rq
   still takes the core_pick fastpath and set_next_task_scx() sets
   SCX_RQ_PROXY_PENDING for D.

3. find_proxy_task() abandons the pick without a put and __schedule()
   goes to pick_again.

4. H wins pick_task() on this rq, so pick_task_scx() doesn't run. The
   sibling's untagged task is still the max, core_cookie is 0 and this
   rq is forced idle.

5. put_prev_task_scx() sees the stale flag and a matching cookie and
   parks D as a proxy put.

Maybe clear SCX_RQ_PROXY_PENDING where find_proxy_task() abandons the
pick instead? None of the existing hooks fit the pick_again site, so
it'd need a small new one, but the flag then can't go stale and the
cookie test isn't needed.

And maybe retitle? The reproduced bug is an SCX wakeup during the proxy
idle rather than a core-sched retry, and "retries" reads like
SCX_RQ_PROXY_RETRY.

Thanks.

-- 
tejun