Re: [PATCH v2 sched_ext/for-7.3 17/36] sched_ext: RCU-protect the sub-sched tree's children/sibling lists

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete inheritance of bypass state in scx_link_sched() leaves the new sub-scheduler in a broken bypass mode.
- [High] RCU-001: Remove Before Reclaim Ordering Violation in scheduler disable paths.
--

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

sched_ext: RCU-protect the sub-sched tree's children/sibling lists

This commit switches the children and sibling list operations to RCU and
expands the descendant walker to accept rcu_read_lock. It also adds logic
to inherit the bypass state when linking a new sub-scheduler under a
bypassing ancestor.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 8ee980f822aac..9231ccc775e01 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -5487,7 +5488,10 @@ s32 scx_link_sched(struct scx_sched *sch)
>  				break;
>  			}
>  
> -			list_add_tail(&sch->sibling, &parent->children);
> +			/* inherit the ancestor bypass state */
> +			WRITE_ONCE(sch->bypass_depth, READ_ONCE(parent->bypass_depth));
> +
> +			list_add_tail_rcu(&sch->sibling, &parent->children);
>  		}
>  #endif	/* CONFIG_EXT_SUB_SCHED */
>  

[Severity: High]
Does inheriting the bypass_depth here without setting the per-CPU fast-path
flags leave the new sub-scheduler in a broken bypass mode?

If sch->bypass_depth is inherited as 1, and scx_bypass(sch, true) is
later called in scx_sub_enable_workfn(), inc_bypass_depth() checks
if (sch->bypass_depth != 1). It will return false and jump to the unlock
label, skipping the SCX_SCHED_PCPU_BYPASSING flag initialization across
all CPUs.

Could this lead to BPF operations being invoked when they should be bypassed
and cause potential hangs or deadlocks?

[ ... ]

> @@ -5514,7 +5518,7 @@ void scx_unlink_sched(struct scx_sched *sch)
>  		if (scx_parent(sch)) {
>  			rhashtable_remove_fast(&scx_sched_hash, &sch->hash_node,
>  					       scx_sched_hash_params);
> -			list_del_init(&sch->sibling);
> +			list_del_rcu(&sch->sibling);
>  		}
>  #endif	/* CONFIG_EXT_SUB_SCHED */
>  		list_del_rcu(&sch->all);

[Severity: High]
Does the teardown path violate the remove-before-reclaim RCU invariant now
that this list is RCU-protected?

In both scx_sub_disable() and scx_disable(), the grace period functions
(synchronize_rcu_expedited() or synchronize_rcu()) are called before
scx_unlink_sched(sch):

scx_sub_disable() {
    synchronize_rcu_expedited();
    scx_disable_bypass_dsp(sch);

    scx_unlink_sched(sch);
}

Because the scheduler remains linked in parent->children and scx_sched_all
during and after the grace period, lockless RCU readers using
scx_for_each_descendant_pre() can discover the dying scheduler after the
grace period has finished but before it is unlinked.

Could this result in readers accessing an exited BPF scheduler context?

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