Re: [PATCH sched_ext/for-7.3 14/32] 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] RCU-001: Remove Before Reclaim Ordering violation during scheduler teardown
- [Medium] RCU publication order violation in scx_link_sched()
--

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

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

Future kfuncs need to walk descendants without scx_sched_lock. Make the
walker RCU-safe so that they can. A sub-sched's fields are initialized
before it is linked, so a walk that observes a linked node also observes its
setup.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index c0a3a1ead2833..1e38aaad43323 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

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

[Severity: Medium]
Since the list is now RCU-protected and can be traversed locklessly, does
initializing sch->bypass_depth after list_add_tail_rcu() allow a concurrent
RCU reader (like scx_for_each_descendant_pre()) to observe the new node with
uninitialized or stale bypass state?

Should the payload be initialized before it is published to the list?

>  		}
>  #endif	/* CONFIG_EXT_SUB_SCHED */
>  
> @@ -5519,7 +5523,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);

[Severity: High]
With these lists now being traversed under RCU, do the teardown paths properly
unlink the scheduler before waiting for grace periods?

Looking at scx_root_disable() and scx_sub_disable(), it appears
synchronize_rcu() (or its expedited variant) is called before
scx_unlink_sched().

Because the scheduler is still in the RCU-protected tree during the grace
period, could new RCU readers discover the scheduler after the grace period
finishes, and access it while its resources are being destroyed?

Should the node be unlinked before calling synchronize_rcu() to prevent new
readers from finding it?

>  		}
>  #endif	/* CONFIG_EXT_SUB_SCHED */
>  		list_del_rcu(&sch->all);

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