Re: [PATCH sched_ext/for-7.3 28/32] sched_ext: Route ops.update_idle() to sub-schedulers and re-notify owed scheds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double delivery of `ops.update_idle(..., true)` to BPF schedulers during an idle transition if a forced re-notify is pending.
--

commit f8054dfa3b3f832d674e303615ccbb3864f6e4c8
Author: Tejun Heo <tj@kernel.org>

sched_ext: Route ops.update_idle() to sub-schedulers and re-notify owed scheds

This commit routes ops.update_idle() to all sub-schedulers that hold base
capabilities on a CPU, rather than just the root scheduler. It also adds a
re-notify mechanism to flush suppressed idle transitions when a scheduler
leaves bypass mode or gains baseline access.

> diff --git a/kernel/sched/ext/idle.c b/kernel/sched/ext/idle.c
> index 8e8c6201b7dfd..04b320f89b6f2 100644
> --- a/kernel/sched/ext/idle.c
> +++ b/kernel/sched/ext/idle.c

[ ... ]

> @@ -730,6 +731,46 @@ static void update_builtin_idle(int cpu, bool idle)
>  	}
>  }
>  
> +/*
> + * Notify schedulers of an idle transition on @cpu's cid, delivering to every
> + * sched that holds %SCX_CAP_BASE on the cid (the root holds every cap). A real
> + * transition (@do_notify) reaches all holders. A forced one (@root_renotify for
> + * the root, a sub-sched's idle_renotify marker for a sub) reaches only the owed
> + * scheds.
> + */
> +static void scx_idle_notify(struct rq *rq, bool idle, bool do_notify, bool root_renotify)
> +{
> +	s32 cpu = cpu_of(rq);
> +	s32 cid = scx_cpu_arg(cpu);
> +	struct scx_sched *pos;
> +
> +	lockdep_assert_rq_held(rq);
> +
> +	pos = scx_next_descendant_pre(NULL, scx_root);
> +	while (pos) {
> +		bool forced = false;
> +
> +		if (unlikely(scx_missing_caps(pos, cpu, SCX_CAP_BASE))) {
> +			pos = scx_skip_subtree_pre(pos, scx_root);
> +			continue;
> +		}
> +
> +		if (pos == scx_root) {
> +			forced = root_renotify;
> +		}
> +#ifdef CONFIG_EXT_SUB_SCHED
> +		else if (per_cpu_ptr(pos->pcpu, cpu)->idle_renotify) {
> +			per_cpu_ptr(pos->pcpu, cpu)->idle_renotify = false;
> +			forced = true;
> +		}
> +#endif
> +		if ((do_notify || forced) && SCX_HAS_OP(pos, update_idle) &&
> +		    !scx_bypassing(pos, cpu))
> +			SCX_CALL_OP(pos, update_idle, rq, cid, idle);
> +		pos = scx_next_descendant_pre(pos, scx_root);
> +	}
> +}

[ ... ]

> @@ -772,20 +812,26 @@ void __scx_update_idle(struct rq *rq, bool idle, bool do_notify)
>  			update_builtin_idle(cpu, idle);
>  
>  	/*
> -	 * Trigger ops.update_idle() only when transitioning from a task to
> -	 * the idle thread and vice versa.
> -	 *
> -	 * Idle transitions are indicated by do_notify being set to true,
> -	 * managed by put_prev_task_idle()/set_next_task_idle().
> +	 * ops.update_idle() fires on real idle transitions, indicated by
> +	 * @do_notify and managed by put_prev_task_idle()/set_next_task_idle().
> +	 * An idle pick also fires it to flush a forced notify owed to a sched
> +	 * that missed transitions while bypassed or on a cid it just gained.
> +	 * unbypass_renotify_idle() and scx_process_sync_ecaps() arm the per-rq
> +	 * gates, and scx_idle_notify() targets the owed scheds.
>  	 *
> -	 * This must come after builtin idle update so that BPF schedulers can
> -	 * create interlocking between ops.update_idle() and ops.enqueue() -
> +	 * This must come after the builtin idle update so that BPF schedulers
> +	 * can create interlocking between ops.update_idle() and ops.enqueue() -
>  	 * either enqueue() sees the idle bit or update_idle() sees the task
>  	 * that enqueue() queued.
>  	 */
> -	if (SCX_HAS_OP(sch, update_idle) && do_notify &&
> -	    !scx_bypassing(sch, cpu_of(rq)))
> -		SCX_CALL_OP(sch, update_idle, rq, scx_cpu_arg(cpu_of(rq)), idle);
> +	if (do_notify ||
> +	    (idle && (rq->scx.flags &
> +		      (SCX_RQ_SUB_IDLE_RENOTIFY | SCX_RQ_ROOT_IDLE_RENOTIFY)))) {
> +		bool root_renotify = rq->scx.flags & SCX_RQ_ROOT_IDLE_RENOTIFY;
> +
> +		rq->scx.flags &= ~(SCX_RQ_SUB_IDLE_RENOTIFY | SCX_RQ_ROOT_IDLE_RENOTIFY);
> +		scx_idle_notify(rq, idle, do_notify, root_renotify);
> +	}
>  }

[Severity: High]
Does this code introduce a regression where ops.update_idle(..., true) is
delivered twice consecutively to BPF schedulers?

If a BPF scheduler sets idle_renotify (e.g. via scx_bypass() or
scx_process_sync_ecaps()) while a CPU is active, and the CPU subsequently
goes idle:

1. __pick_next_task() calls pick_task_idle()
2. pick_task_idle() calls __scx_update_idle() with idle=true and do_notify=false
3. __scx_update_idle() sees the renotify flags, clears them, and calls scx_idle_notify()
4. scx_idle_notify() delivers update_idle(..., true) to the owed scheduler because forced is true.

Immediately after, __pick_next_task() calls put_prev_set_next_task(), which
calls set_next_task_idle():

5. set_next_task_idle() calls __scx_update_idle() with idle=true and do_notify=true
6. __scx_update_idle() sees do_notify=true and calls scx_idle_notify()
7. scx_idle_notify() delivers update_idle(..., true) to all schedulers, including the owed scheduler again.

Could this double delivery violate edge-trigger semantics and cause BPF
schedulers maintaining idle CPU lists to push the CPU multiple times?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703080159.2314350-1-tj@kernel.org?part=28