Re: [PATCH 2/6] sched_ext: Free a sub-scheduler a grace period after discarding its ecaps syncs

[Andrea Righi <arighi@nvidia.com>]

On Sat, Oct 17, 2026 at 03:59:48PM -1000, Tejun Heo wrote:
> The free path removes a sub-scheduler's ecaps sync node from each cpu's list
> and then cycles the rq lock until the node reads off-list, as a sync batch
> can hold the node privately. Cycling the lock is correct only because the
> sched is bypassing and a bypassing sched's sync calls no op, so the batch
> touches the pcpu within one rq lock hold.
> 
> Tying the free path to what the sync does for a bypassing sched is unusual
> and brittle, and it gets in the way of changing the sync. Delivering ecaps
> to a bypassing sched or its parent would break the wait, as a delivery drops
> the rq lock.
> 
> Wait for the batches with RCU instead, the usual way to wait for lockless
> users before a free. Discard the nodes after the first grace period as
> before, and free the sched after a second one. The first grace period waits
> for the paths that found the sched before it was unlinked, such as the
> sub-cap kfuncs and the cpu hotplug hooks. Until they finish, they can queue
> its syncs, and a cpu can take a queued node into a sync batch that the first
> grace period doesn't wait for. After it, nothing queues the syncs, and the
> discards remove the nodes still queued.
> 
> A batch that took a node keeps irqs off from llist_del_all() to its last
> pcpu write, even across the rq lock drops of a delivery. The second grace
> period waits for it whether or not it called an op. The exit and disable
> irq_works, the helper kthread and the bypass load balancing timer are
> stopped ahead of the per-cpu checks as before, so they move into
> scx_sched_quiesce_rcu_work() with the checks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---

...

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 7edad2ddaa54..1923bdc33d6b 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -1318,8 +1318,10 @@ static void clear_all_caps(struct scx_sched *sch)
>  }
>  
>  /*
> - * @pcpu's sched was unhashed before the grace period, so nothing re-queues its
> - * sync node. Remove the node from @rq's pending list so the pcpu can be freed.
> + * Remove @pcpu's ecaps sync node from @cpu's pending list. The first grace
> + * period has passed, so nothing queues the node again. The second grace period
> + * waits for a sync batch that took the node earlier, see
> + * scx_sched_quiesce_rcu_work().
>   */
>  void scx_discard_ecaps_to_sync(s32 cpu, struct scx_sched_pcpu *pcpu)
>  {
> @@ -1327,43 +1329,28 @@ void scx_discard_ecaps_to_sync(s32 cpu, struct scx_sched_pcpu *pcpu)
>  	struct llist_node *head = NULL, *tail = NULL;
>  	struct llist_node *pos, *tmp;
>  
> +	guard(rq_lock_irqsave)(rq);
> +
> +	if (!llist_on_list(&pcpu->ecaps_to_sync_node))
> +		return;
> +

This is called for every possible CPU from the free path of every sched,
including a root without sub-scheds, whose nodes are never queued. With the lock
moved ahead of the test, every scheduler unload now takes all the rq locks with
irqs off, which on a large machine can be a lot of lock traffic.

The first grace period has passed by now, so nothing queues the node again, and
a node taken by an in-flight batch still reads as on-list. So, can we keep the
lockless test in front?

	if (!llist_on_list(&pcpu->ecaps_to_sync_node))
		return;

	guard(rq_lock_irqsave)(rq);

And the WARN_ON_ONCE() on SCX_SCHED_PCPU_ECAPS_REPORT added in 3/6 could go
above the lockless test.

Thanks,
-Andrea

>  	/*
>  	 * llist can't unlink a single node. Take all queued nodes, drop @pcpu's
>  	 * and resplice the rest. Nodes in the taken batch read as on-list
>  	 * throughout, so queue_sync_ecaps() stays correct.
>  	 */
> -	if (llist_on_list(&pcpu->ecaps_to_sync_node)) {
> -		scoped_guard (rq_lock_irqsave, rq) {
> -			llist_for_each_safe(pos, tmp, llist_del_all(&rq->scx.ecaps_to_sync)) {
> -				if (pos == &pcpu->ecaps_to_sync_node) {
> -					init_llist_node(pos);
> -				} else {
> -					pos->next = head;
> -					head = pos;
> -					if (!tail)
> -						tail = pos;
> -				}
> -			}
> -			if (head)
> -				llist_add_batch(head, tail, &rq->scx.ecaps_to_sync);
> -		}
> -	}
> -
> -	/*
> -	 * An in-flight scx_process_sync_ecaps() batch may still hold the node
> -	 * privately across dispatch-induced rq unlocks, reading as on-list.
> -	 *
> -	 * Because a bypassing sched gets no op call, init_llist_node() and all
> -	 * @pcpu accesses share one contiguous lock hold, off-list under the rq
> -	 * lock means @pcpu won't be accessed again.
> -	 */
> -	while (true) {
> -		scoped_guard (rq_lock_irqsave, rq) {
> -			if (!llist_on_list(&pcpu->ecaps_to_sync_node))
> -				return;
> +	llist_for_each_safe(pos, tmp, llist_del_all(&rq->scx.ecaps_to_sync)) {
> +		if (pos == &pcpu->ecaps_to_sync_node) {
> +			init_llist_node(pos);
> +		} else {
> +			pos->next = head;
> +			head = pos;
> +			if (!tail)
> +				tail = pos;
>  		}
> -		cpu_relax();
>  	}
> +	if (head)
> +		llist_add_batch(head, tail, &rq->scx.ecaps_to_sync);
>  }
>  
>  /**
> -- 
> 2.56.0
>