The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	sched-ext@lists.linux.dev, Emil Tsalapatis <emil@etsalapatis.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/4] sched_ext: Build the cid tables privately and publish them with RCU
Date: Wed, 22 Jul 2026 10:29:23 +0200	[thread overview]
Message-ID: <amB_Y-s4wcQL0_J8@gpd4> (raw)
In-Reply-To: <20260722082341.2533979-1-tj@kernel.org>

On Tue, Jul 21, 2026 at 10:23:41PM -1000, Tejun Heo wrote:
> The cid tables are visible to the cid kfuncs while being modified: the
> first enable publishes the global pointers before filling them,
> ops.init_cids() overrides rewrite them in place, and re-enables rebuild
> them in place. A racing TRACING or SYSCALL program can read unfilled
> entries, including uninitialized memory in the kmalloc'd tables, or torn
> topo updates.
> 
> Tie the tables' lifetimes to the root sched instead: each root enable
> builds a fresh set privately and publishes the per-table __rcu globals once
> the layout is final, and root disable unpublishes and RCU-frees the set. A
> non-NULL global is now always a fully built table which stays valid for the
> reader's RCU read section, and lookups stay two loads. Kfuncs treat NULL as
> no-mapping, also after the scheduler exits instead of reporting the stale
> last mapping.
> 
> The cid kfuncs are available whether the root scheduler is cid-form or
> cpu-form, the latter to allow gradual migration to cids. Every root
> therefore builds and publishes a default mapping.

Looks good. :)

Thanks,
-Andrea

> 
> Every reader must either be gated on scheduler liveness or NULL-check
> inside an RCU read section. Fix the two kfuncs that were neither:
> scx_bpf_this_cid() read the table with no RCU or preemption protection and
> scx_bpf_task_cid() relied on KF_RCU, which doesn't put a sleepable program
> in an RCU read section. The hotplug callbacks are instead serialized by
> retiring the tables inside the cpus_read_lock() section that clears
> scx_root.
> 
> v2: Document why every root builds the tables (desc + cid.c comment).
> 
> Reported-by: Andrea Righi <arighi@nvidia.com>
> Closes: https://lore.kernel.org/r/al3tLtPZZkFjMveK@gpd4
> Reviewed-by: Andrea Righi <arighi@nvidia.com>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  kernel/sched/ext/cid.c      | 260 +++++++++++++++++++++++-------------
>  kernel/sched/ext/cid.h      |  57 +++++---
>  kernel/sched/ext/ext.c      |  54 ++++++--
>  kernel/sched/ext/internal.h |   5 +-
>  kernel/sched/ext/sub.c      |  42 ++++--
>  5 files changed, 276 insertions(+), 142 deletions(-)
> 
> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index 5f990d95735f..231c8562d7c7 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
> @@ -11,20 +11,22 @@
>  #include "cid.h"
>  
>  /*
> - * cid tables.
> - *
> - * Pointers are allocated on first enable and never freed. During root enable,
> - * the default mapping is populated and then ops.init_cids() is called which can
> - * use scx_bpf_cid_override() to change the mapping. The mapping stays stable
> - * until the root is disabled.
> + * cid tables. The cid kfuncs are available whether the root scheduler is
> + * cid-form or cpu-form, the latter to allow gradual migration to cids, so every
> + * root builds a default mapping. Each root enable allocates a fresh set, builds
> + * it privately and publishes the __rcu globals below once the layout is final.
> + * Root disable unpublishes and RCU-frees the set. kfuncs may run before the
> + * tables are published and must check for NULL.
>   */
>  u32 scx_nr_cid_shards;
> -s16 *scx_cid_to_cpu_tbl;
> -s16 *scx_cpu_to_cid_tbl;
> -s32 *scx_cid_to_shard;
> -s32 *scx_shard_node;
> -struct scx_cid_shard *scx_cid_shard_ranges;
> -struct scx_cid_topo *scx_cid_topo;
> +s16 __rcu *scx_cid_to_cpu_tbl;
> +s16 __rcu *scx_cpu_to_cid_tbl;
> +s32 __rcu *scx_cid_to_shard;
> +s32 __rcu *scx_shard_node;
> +struct scx_cid_shard __rcu *scx_cid_shard_ranges;
> +struct scx_cid_topo __rcu *scx_cid_topo;
> +
> +static struct scx_cid_tables *scx_cid_tables;	/* used only during alloc/free */
>  
>  #define SCX_CID_TOPO_NEG	(struct scx_cid_topo) {				\
>  	.core_cid = -1, .core_idx = -1, .llc_cid = -1, .llc_idx = -1,		\
> @@ -73,50 +75,102 @@ static void calc_shard_layout(const struct cpumask *llc_cpus, u32 shard_size,
>  	*nr_large_p = nr_cores % nr_shards;
>  }
>  
> -/* Allocate the cid tables once on first enable; never freed. */
> -static s32 scx_cid_arrays_alloc(void)
> +static void scx_cid_tables_free(struct scx_cid_tables *tbls)
> +{
> +	if (!tbls)
> +		return;
> +	kvfree(tbls->cid_to_cpu);
> +	kvfree(tbls->cpu_to_cid);
> +	kvfree(tbls->cid_to_shard);
> +	kvfree(tbls->shard_node);
> +	kvfree(tbls->shard_ranges);
> +	kvfree(tbls->topo);
> +	kfree(tbls);
> +}
> +
> +static void scx_cid_tables_free_rcufn(struct rcu_head *rcu)
> +{
> +	scx_cid_tables_free(container_of(rcu, struct scx_cid_tables, rcu));
> +}
> +
> +static struct scx_cid_tables *scx_cid_alloc_tables(void)
>  {
>  	u32 npossible = num_possible_cpus();
> -	s16 *cid_to_cpu, *cpu_to_cid;
> -	s32 *cid_to_shard;
> -	s32 *shard_node;
> -	struct scx_cid_shard *cid_shard_ranges;
> -	struct scx_cid_topo *cid_topo;
> -
> -	if (scx_cid_to_cpu_tbl)
> -		return 0;
> -
> -	cid_to_cpu = kzalloc_objs(*scx_cid_to_cpu_tbl, npossible, GFP_KERNEL);
> -	cpu_to_cid = kzalloc_objs(*scx_cpu_to_cid_tbl, nr_cpu_ids, GFP_KERNEL);
> -	cid_to_shard = kzalloc_objs(*scx_cid_to_shard, npossible, GFP_KERNEL);
> -	shard_node = kmalloc_objs(*scx_shard_node, npossible, GFP_KERNEL);
> -	cid_shard_ranges = kzalloc_objs(*scx_cid_shard_ranges, npossible, GFP_KERNEL);
> -	cid_topo = kmalloc_objs(*scx_cid_topo, npossible, GFP_KERNEL);
> -
> -	if (!cid_to_cpu || !cpu_to_cid || !cid_to_shard || !shard_node ||
> -	    !cid_shard_ranges || !cid_topo) {
> -		kfree(cid_to_cpu);
> -		kfree(cpu_to_cid);
> -		kfree(cid_to_shard);
> -		kfree(shard_node);
> -		kfree(cid_shard_ranges);
> -		kfree(cid_topo);
> -		return -ENOMEM;
> +	struct scx_cid_tables *tbls;
> +
> +	tbls = kzalloc_obj(*tbls, GFP_KERNEL);
> +	if (!tbls)
> +		return NULL;
> +
> +	tbls->cid_to_cpu = kvcalloc(npossible, sizeof(*tbls->cid_to_cpu), GFP_KERNEL);
> +	tbls->cpu_to_cid = kvcalloc(nr_cpu_ids, sizeof(*tbls->cpu_to_cid), GFP_KERNEL);
> +	tbls->cid_to_shard = kvcalloc(npossible, sizeof(*tbls->cid_to_shard), GFP_KERNEL);
> +	tbls->shard_node = kvcalloc(npossible, sizeof(*tbls->shard_node), GFP_KERNEL);
> +	tbls->shard_ranges = kvcalloc(npossible, sizeof(*tbls->shard_ranges), GFP_KERNEL);
> +	tbls->topo = kvcalloc(npossible, sizeof(*tbls->topo), GFP_KERNEL);
> +
> +	if (!tbls->cid_to_cpu || !tbls->cpu_to_cid || !tbls->cid_to_shard ||
> +	    !tbls->shard_node || !tbls->shard_ranges || !tbls->topo) {
> +		scx_cid_tables_free(tbls);
> +		return NULL;
>  	}
>  
> -	WRITE_ONCE(scx_cid_to_cpu_tbl, cid_to_cpu);
> -	WRITE_ONCE(scx_cpu_to_cid_tbl, cpu_to_cid);
> -	WRITE_ONCE(scx_cid_to_shard, cid_to_shard);
> -	WRITE_ONCE(scx_shard_node, shard_node);
> -	WRITE_ONCE(scx_cid_shard_ranges, cid_shard_ranges);
> -	WRITE_ONCE(scx_cid_topo, cid_topo);
> -	return 0;
> +	return tbls;
> +}
> +
> +/**
> + * scx_cid_publish_tables - Publish the tables scx_cid_init() built
> + *
> + * Called after ops.init_cids() where the layout is final.
> + */
> +void scx_cid_publish_tables(void)
> +{
> +	struct scx_cid_tables *tbls = scx_cid_tables;
> +
> +	lockdep_assert_held(&scx_enable_mutex);
> +
> +	scx_nr_cid_shards = tbls->nr_shards;
> +	rcu_assign_pointer(scx_cid_to_cpu_tbl, tbls->cid_to_cpu);
> +	rcu_assign_pointer(scx_cpu_to_cid_tbl, tbls->cpu_to_cid);
> +	rcu_assign_pointer(scx_cid_to_shard, tbls->cid_to_shard);
> +	rcu_assign_pointer(scx_shard_node, tbls->shard_node);
> +	rcu_assign_pointer(scx_cid_shard_ranges, tbls->shard_ranges);
> +	rcu_assign_pointer(scx_cid_topo, tbls->topo);
> +}
> +
> +/**
> + * scx_cid_retire_tables - Unpublish and retire the cid tables
> + *
> + * Called by root disable after the readers which dereference without NULL
> + * checks are drained, inside cpus_read_lock() to exclude the hotplug path.
> + */
> +void scx_cid_retire_tables(void)
> +{
> +	struct scx_cid_tables *tbls = scx_cid_tables;
> +
> +	lockdep_assert_held(&scx_enable_mutex);
> +	lockdep_assert_cpus_held();
> +
> +	if (!tbls)
> +		return;
> +
> +	scx_cid_tables = NULL;
> +	RCU_INIT_POINTER(scx_cid_to_cpu_tbl, NULL);
> +	RCU_INIT_POINTER(scx_cpu_to_cid_tbl, NULL);
> +	RCU_INIT_POINTER(scx_cid_to_shard, NULL);
> +	RCU_INIT_POINTER(scx_shard_node, NULL);
> +	RCU_INIT_POINTER(scx_cid_shard_ranges, NULL);
> +	RCU_INIT_POINTER(scx_cid_topo, NULL);
> +	call_rcu(&tbls->rcu, scx_cid_tables_free_rcufn);
>  }
>  
>  /**
>   * scx_cid_init - build the cid mapping
>   * @sch: the scx_sched being initialized; used as the scx_error() target
>   *
> + * Build a fresh table set. It becomes visible through scx_cid_publish_tables()
> + * and is retired by scx_cid_retire_tables() at disable.
> + *
>   * See "Topological CPU IDs" in cid.h for the model. Walk online cpus by
>   * intersection at each level (parent_scratch & this_level_mask), which keeps
>   * containment correct by construction and naturally splits a physical LLC
> @@ -131,30 +185,32 @@ s32 scx_cid_init(struct scx_sched *sch)
>  	cpumask_var_t core_scratch __free(free_cpumask_var) = CPUMASK_VAR_NULL;
>  	cpumask_var_t llc_fallback __free(free_cpumask_var) = CPUMASK_VAR_NULL;
>  	cpumask_var_t online_no_topo __free(free_cpumask_var) = CPUMASK_VAR_NULL;
> +	struct scx_cid_tables *tbls;
>  	u32 next_cid = 0;
>  	s32 next_node_idx = 0, next_llc_idx = 0, next_core_idx = 0;
>  	s32 next_shard_idx = 0;
>  	u32 shard_size, max_cids;
>  	u32 notopo_in_shard;
>  	s32 notopo_shard_cid, notopo_shard_idx;
> -	s32 cpu, cid, si, ret;
> +	s32 cpu, cid, si;
>  
>  	/* CMASK_MAX_WORDS in cid.bpf.h covers NR_CPUS up to 8192 */
>  	BUILD_BUG_ON(NR_CPUS > 8192);
>  
>  	lockdep_assert_cpus_held();
> +	lockdep_assert_held(&scx_enable_mutex);
>  
>  	shard_size = sch->ops.cid_shard_size ?: SCX_CID_SHARD_SIZE_DFL;
>  	max_cids = min_t(u32, shard_size, SCX_CID_SHARD_MAX_CPUS);
>  
> -	ret = scx_cid_arrays_alloc();
> -	if (ret)
> -		return ret;
> +	tbls = scx_cid_alloc_tables();
> +	if (!tbls)
> +		return -ENOMEM;
> +
> +	scx_cid_tables = tbls;
>  
> -	/* clear shard ranges and reset shard_node for repopulate */
> -	memset(scx_cid_shard_ranges, 0, num_possible_cpus() * sizeof(*scx_cid_shard_ranges));
>  	for (si = 0; si < num_possible_cpus(); si++)
> -		scx_shard_node[si] = NUMA_NO_NODE;
> +		tbls->shard_node[si] = NUMA_NO_NODE;
>  
>  	if (!zalloc_cpumask_var(&to_walk, GFP_KERNEL) ||
>  	    !zalloc_cpumask_var(&node_scratch, GFP_KERNEL) ||
> @@ -166,7 +222,7 @@ s32 scx_cid_init(struct scx_sched *sch)
>  
>  	/* -1 sentinels for sparse-possible cpu id holes (0 is a valid cid) */
>  	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
> -		scx_cpu_to_cid_tbl[cpu] = -1;
> +		tbls->cpu_to_cid[cpu] = -1;
>  
>  	cpumask_copy(to_walk, cpu_online_mask);
>  
> @@ -209,7 +265,7 @@ s32 scx_cid_init(struct scx_sched *sch)
>  			calc_shard_layout(llc_scratch, shard_size, &cores_per_shard, &nr_large);
>  			shard_cid = next_cid;
>  			shard_idx = next_shard_idx++;
> -			scx_shard_node[shard_idx] = nid;
> +			tbls->shard_node[shard_idx] = nid;
>  
>  			while (!cpumask_empty(llc_scratch)) {
>  				s32 lcpu = cpumask_first(llc_scratch);
> @@ -240,7 +296,7 @@ s32 scx_cid_init(struct scx_sched *sch)
>  					cids_in_shard = 0;
>  					shard_cid = next_cid;
>  					shard_idx = next_shard_idx++;
> -					scx_shard_node[shard_idx] = nid;
> +					tbls->shard_node[shard_idx] = nid;
>  				}
>  				cores_in_shard++;
>  				cids_in_shard += cids_in_core;
> @@ -248,10 +304,10 @@ s32 scx_cid_init(struct scx_sched *sch)
>  				for_each_cpu(ccpu, core_scratch) {
>  					s32 cid = next_cid++;
>  
> -					scx_cid_to_cpu_tbl[cid] = ccpu;
> -					scx_cpu_to_cid_tbl[ccpu] = cid;
> -					scx_cid_to_shard[cid] = shard_idx;
> -					scx_cid_topo[cid] = (struct scx_cid_topo){
> +					tbls->cid_to_cpu[cid] = ccpu;
> +					tbls->cpu_to_cid[ccpu] = cid;
> +					tbls->cid_to_shard[cid] = shard_idx;
> +					tbls->topo[cid] = (struct scx_cid_topo){
>  						.core_cid = core_cid,
>  						.core_idx = core_idx,
>  						.llc_cid = llc_cid,
> @@ -283,14 +339,14 @@ s32 scx_cid_init(struct scx_sched *sch)
>  	notopo_shard_idx = -1;
>  
>  	for_each_cpu(cpu, cpu_possible_mask) {
> -		if (__scx_cpu_to_cid(cpu) != -1)
> +		if (tbls->cpu_to_cid[cpu] != -1)
>  			continue;
>  		if (cpu_online(cpu))
>  			cpumask_set_cpu(cpu, online_no_topo);
>  
>  		cid = next_cid++;
> -		scx_cid_to_cpu_tbl[cid] = cpu;
> -		scx_cpu_to_cid_tbl[cpu] = cid;
> +		tbls->cid_to_cpu[cid] = cpu;
> +		tbls->cpu_to_cid[cpu] = cid;
>  
>  		if (notopo_in_shard >= min_t(u32, shard_size, SCX_CID_SHARD_MAX_CPUS)) {
>  			notopo_shard_cid = cid;
> @@ -299,10 +355,10 @@ s32 scx_cid_init(struct scx_sched *sch)
>  		}
>  		notopo_in_shard++;
>  
> -		scx_cid_to_shard[cid] = notopo_shard_idx;
> -		scx_cid_topo[cid] = SCX_CID_TOPO_NEG;
> -		scx_cid_topo[cid].shard_cid = notopo_shard_cid;
> -		scx_cid_topo[cid].shard_idx = notopo_shard_idx;
> +		tbls->cid_to_shard[cid] = notopo_shard_idx;
> +		tbls->topo[cid] = SCX_CID_TOPO_NEG;
> +		tbls->topo[cid].shard_cid = notopo_shard_cid;
> +		tbls->topo[cid].shard_idx = notopo_shard_idx;
>  	}
>  
>  	if (!cpumask_empty(llc_fallback))
> @@ -318,14 +374,14 @@ s32 scx_cid_init(struct scx_sched *sch)
>  	 * shard, nr_cids is the count.
>  	 */
>  	for (cid = 0; cid < next_cid; cid++) {
> -		s32 sidx = scx_cid_to_shard[cid];
> +		s32 sidx = tbls->cid_to_shard[cid];
>  
> -		if (scx_cid_shard_ranges[sidx].nr_cids == 0)
> -			scx_cid_shard_ranges[sidx].base_cid = cid;
> -		scx_cid_shard_ranges[sidx].nr_cids++;
> +		if (tbls->shard_ranges[sidx].nr_cids == 0)
> +			tbls->shard_ranges[sidx].base_cid = cid;
> +		tbls->shard_ranges[sidx].nr_cids++;
>  	}
>  
> -	scx_nr_cid_shards = next_shard_idx;
> +	tbls->nr_shards = next_shard_idx;
>  	return 0;
>  }
>  
> @@ -417,6 +473,7 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  	s32 *cpu_to_cid __free(kfree) = NULL;
>  	s32 *shard_start __free(kfree) = NULL;
>  	u32 npossible = num_possible_cpus();
> +	struct scx_cid_tables *tbls;
>  	struct scx_sched *sch;
>  	u32 nr_shards;
>  	bool alloced;
> @@ -438,6 +495,10 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  	if (unlikely(!sch))
>  		return;
>  
> +	/* called from ops.init_cids(), so the tables exist and are unpublished */
> +	lockdep_assert_held(&scx_enable_mutex);
> +	tbls = scx_cid_tables;
> +
>  	if (!alloced || !node_counts || !cpu_to_cid || !shard_start) {
>  		scx_error(sch, "scx_bpf_cid_override: allocation failed");
>  		return;
> @@ -488,7 +549,7 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  		return;
>  	}
>  
> -	/* Validate first so that invalid input leaves globals untouched. */
> +	/* validate first so that invalid input leaves the tables untouched */
>  	for_each_possible_cpu(cpu) {
>  		s32 c = cpu_to_cid[cpu];
>  
> @@ -503,12 +564,12 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  	for_each_possible_cpu(cpu) {
>  		s32 c = cpu_to_cid[cpu];
>  
> -		scx_cpu_to_cid_tbl[cpu] = c;
> -		scx_cid_to_cpu_tbl[c] = cpu;
> +		tbls->cpu_to_cid[cpu] = c;
> +		tbls->cid_to_cpu[c] = cpu;
>  	}
>  
>  	/*
> -	 * Derive scx_shard_node[] by majority count: an overridden shard may
> +	 * Derive shard_node[] by majority count: an overridden shard may
>  	 * span NUMA nodes, so assign each to the node that owns the most cpus.
>  	 */
>  	for (si = 0; si < nr_shards; si++) {
> @@ -516,12 +577,12 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  
>  		memset(node_counts, 0, nr_node_ids * sizeof(*node_counts));
>  		for (cid = shard_start[si]; cid < end; cid++) {
> -			s32 node = cpu_to_node(scx_cid_to_cpu_tbl[cid]);
> +			s32 node = cpu_to_node(tbls->cid_to_cpu[cid]);
>  
>  			if (numa_valid_node(node))
>  				node_counts[node]++;
>  		}
> -		scx_shard_node[si] = pick_max_node(node_counts, nr_node_ids);
> +		tbls->shard_node[si] = pick_max_node(node_counts, nr_node_ids);
>  	}
>  
>  	/*
> @@ -532,22 +593,22 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid_src, u32 cpu_to_cid_
>  	for (cid = 0; cid < npossible; cid++) {
>  		if (si + 1 < nr_shards && cid >= shard_start[si + 1])
>  			si++;
> -		scx_cid_to_shard[cid] = si;
> -		scx_cid_topo[cid] = SCX_CID_TOPO_NEG;
> -		scx_cid_topo[cid].shard_cid = shard_start[si];
> -		scx_cid_topo[cid].shard_idx = si;
> +		tbls->cid_to_shard[cid] = si;
> +		tbls->topo[cid] = SCX_CID_TOPO_NEG;
> +		tbls->topo[cid].shard_cid = shard_start[si];
> +		tbls->topo[cid].shard_idx = si;
>  	}
>  
> -	/* Rebuild scx_cid_shard_ranges[] for the new layout. */
> -	memset(scx_cid_shard_ranges, 0, npossible * sizeof(*scx_cid_shard_ranges));
> +	/* Rebuild shard_ranges[] for the new layout. */
> +	memset(tbls->shard_ranges, 0, npossible * sizeof(*tbls->shard_ranges));
>  	for (si = 0; si < nr_shards; si++) {
>  		u32 end = (si + 1 < nr_shards) ? shard_start[si + 1] : npossible;
>  
> -		scx_cid_shard_ranges[si].base_cid = shard_start[si];
> -		scx_cid_shard_ranges[si].nr_cids = end - shard_start[si];
> +		tbls->shard_ranges[si].base_cid = shard_start[si];
> +		tbls->shard_ranges[si].nr_cids = end - shard_start[si];
>  	}
>  
> -	scx_nr_cid_shards = nr_shards;
> +	tbls->nr_shards = nr_shards;
>  }
>  
>  /**
> @@ -849,22 +910,25 @@ bool scx_cmask_empty(const struct scx_cmask *m)
>   *
>   * Fill @out__uninit with the topology info for @cid. Trigger scx_error() if
>   * @cid is out of range. If @cid is valid but in the no-topo section, all fields
> - * are set to -1.
> + * are set to -1. All fields are also set to -1 when no cid tables have been
> + * published yet, which a program may observe while racing the root enable.
>   */
>  __bpf_kfunc void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out__uninit,
>  				  const struct bpf_prog_aux *aux)
>  {
> +	struct scx_cid_topo *topo;
>  	struct scx_sched *sch;
>  
>  	guard(rcu)();
>  
>  	sch = scx_prog_sched(aux);
> -	if (unlikely(!sch) || !cid_valid(sch, cid)) {
> +	topo = rcu_dereference(scx_cid_topo);
> +	if (unlikely(!sch) || !cid_valid(sch, cid) || unlikely(!topo)) {
>  		*out__uninit = SCX_CID_TOPO_NEG;
>  		return;
>  	}
>  
> -	*out__uninit = READ_ONCE(scx_cid_topo)[cid];
> +	*out__uninit = topo[cid];
>  }
>  
>  __bpf_kfunc_end_defs();
> @@ -907,6 +971,7 @@ int scx_cmask_ref_init(struct scx_sched *sch, const struct scx_cmask *src,
>  {
>  	struct scx_cmask *kern_src = scx_arena_to_kaddr(sch, src);
>  	u32 base, nr_cids, alloc_words, npossible = num_possible_cpus();
> +	s32 *cid_to_shard;
>  
>  	base = READ_ONCE(kern_src->base);
>  	nr_cids = READ_ONCE(kern_src->nr_cids);
> @@ -921,9 +986,10 @@ int scx_cmask_ref_init(struct scx_sched *sch, const struct scx_cmask *src,
>  	ref->base = base;
>  	ref->nr_cids = nr_cids;
>  
> -	ref->shard_first = scx_cid_to_shard[base];
> +	cid_to_shard = rcu_dereference_all(scx_cid_to_shard);
> +	ref->shard_first = cid_to_shard[base];
>  	if (likely(nr_cids))
> -		ref->shard_end = scx_cid_to_shard[base + nr_cids - 1] + 1;
> +		ref->shard_end = cid_to_shard[base + nr_cids - 1] + 1;
>  	else
>  		ref->shard_end = ref->shard_first;
>  
> @@ -946,6 +1012,8 @@ int scx_cmask_ref_init(struct scx_sched *sch, const struct scx_cmask *src,
>  void scx_cmask_ref_init_kern(struct scx_sched *sch, struct scx_cmask *m,
>  			     u32 base, u32 nr_cids, struct scx_cmask_ref *ref)
>  {
> +	s32 *cid_to_shard;
> +
>  	WRITE_ONCE(m->base, base);
>  	WRITE_ONCE(m->nr_cids, nr_cids);
>  	WRITE_ONCE(m->alloc_words, SCX_CMASK_NR_WORDS(nr_cids));
> @@ -955,9 +1023,10 @@ void scx_cmask_ref_init_kern(struct scx_sched *sch, struct scx_cmask *m,
>  	ref->base = base;
>  	ref->nr_cids = nr_cids;
>  
> -	ref->shard_first = scx_cid_to_shard[base];
> +	cid_to_shard = rcu_dereference_all(scx_cid_to_shard);
> +	ref->shard_first = cid_to_shard[base];
>  	if (likely(nr_cids))
> -		ref->shard_end = scx_cid_to_shard[base + nr_cids - 1] + 1;
> +		ref->shard_end = cid_to_shard[base + nr_cids - 1] + 1;
>  	else
>  		ref->shard_end = ref->shard_first;
>  }
> @@ -976,7 +1045,8 @@ void scx_cmask_ref_init_kern(struct scx_sched *sch, struct scx_cmask *m,
>  void scx_cmask_ref_shard(const struct scx_cmask_ref *ref, s32 shard_idx,
>  			 struct scx_cmask *out)
>  {
> -	const struct scx_cid_shard *shard = &scx_cid_shard_ranges[shard_idx];
> +	const struct scx_cid_shard *shard =
> +		&rcu_dereference_all(scx_cid_shard_ranges)[shard_idx];
>  	u32 shard_base = shard->base_cid;
>  	u32 shard_end = shard_base + shard->nr_cids;
>  	u32 isect_base, isect_end, nr_words, src_off, wi;
> diff --git a/kernel/sched/ext/cid.h b/kernel/sched/ext/cid.h
> index 470ac9224da4..1f74d1f331f5 100644
> --- a/kernel/sched/ext/cid.h
> +++ b/kernel/sched/ext/cid.h
> @@ -48,13 +48,24 @@ struct scx_sched;
>   * See the comment above the table definitions in cid.c for the
>   * memory-ordering and visibility contract.
>   */
> +struct scx_cid_tables {
> +	u32			nr_shards;
> +	s16			*cid_to_cpu;	/* [num_possible_cpus()] */
> +	s16			*cpu_to_cid;	/* [nr_cpu_ids] */
> +	s32			*cid_to_shard;	/* [num_possible_cpus()] */
> +	s32			*shard_node;	/* [num_possible_cpus()] */
> +	struct scx_cid_shard	*shard_ranges;	/* [num_possible_cpus()] */
> +	struct scx_cid_topo	*topo;		/* [num_possible_cpus()] */
> +	struct rcu_head		rcu;
> +};
> +
>  extern u32 scx_nr_cid_shards;
> -extern s16 *scx_cid_to_cpu_tbl;
> -extern s16 *scx_cpu_to_cid_tbl;
> -extern s32 *scx_cid_to_shard;
> -extern s32 *scx_shard_node;
> -extern struct scx_cid_shard *scx_cid_shard_ranges;
> -extern struct scx_cid_topo *scx_cid_topo;
> +extern s16 __rcu *scx_cid_to_cpu_tbl;
> +extern s16 __rcu *scx_cpu_to_cid_tbl;
> +extern s32 __rcu *scx_cid_to_shard;
> +extern s32 __rcu *scx_shard_node;
> +extern struct scx_cid_shard __rcu *scx_cid_shard_ranges;
> +extern struct scx_cid_topo __rcu *scx_cid_topo;
>  extern struct btf_id_set8 scx_kfunc_ids_init_cids;
>  
>  void scx_cmask_clear(struct scx_cmask *m);
> @@ -67,6 +78,8 @@ bool scx_cmask_subset(const struct scx_cmask *sub, const struct scx_cmask *super
>  bool scx_cmask_intersects(const struct scx_cmask *a, const struct scx_cmask *b);
>  bool scx_cmask_empty(const struct scx_cmask *m);
>  s32 scx_cid_init(struct scx_sched *sch);
> +void scx_cid_publish_tables(void);
> +void scx_cid_retire_tables(void);
>  int scx_cid_kfunc_init(void);
>  
>  /**
> @@ -89,14 +102,12 @@ static inline bool cid_valid(struct scx_sched *sch, s32 cid)
>   * __scx_cid_to_cpu - Unchecked cid->cpu table lookup
>   * @cid: cid to look up. Must be in [0, num_possible_cpus()).
>   *
> - * Intended for callsites that have already validated @cid and that hold a
> - * non-NULL @sch from scx_prog_sched() - a live sched implies the table has
> - * been allocated, so no NULL check is needed here.
> + * Intended for callsites that have already validated @cid and that run on a
> + * live scheduler, which guarantees the tables are published and stable.
>   */
>  static inline s32 __scx_cid_to_cpu(s32 cid)
>  {
> -	/* READ_ONCE pairs with WRITE_ONCE in scx_cid_arrays_alloc() */
> -	return READ_ONCE(scx_cid_to_cpu_tbl)[cid];
> +	return rcu_dereference_all(scx_cid_to_cpu_tbl)[cid];
>  }
>  
>  /**
> @@ -107,7 +118,7 @@ static inline s32 __scx_cid_to_cpu(s32 cid)
>   */
>  static inline s32 __scx_cpu_to_cid(s32 cpu)
>  {
> -	return READ_ONCE(scx_cpu_to_cid_tbl)[cpu];
> +	return rcu_dereference_all(scx_cpu_to_cid_tbl)[cpu];
>  }
>  
>  /**
> @@ -116,15 +127,19 @@ static inline s32 __scx_cpu_to_cid(s32 cpu)
>   * @cid: cid to look up
>   *
>   * Return the cpu for @cid or a negative errno on failure. Invalid cid triggers
> - * scx_error() on @sch. The cid arrays are allocated on first scheduler enable
> - * and never freed, so the returned cpu is stable for the lifetime of the loaded
> - * scheduler.
> + * scx_error() on @sch. The mapping is stable while the scheduler is live.
> + *
> + * Return -EINVAL without triggering scx_error() if no tables have been
> + * published yet, which a prog-facing kfunc can observe while racing the root
> + * scheduler enable.
>   */
>  static inline s32 scx_cid_to_cpu(struct scx_sched *sch, s32 cid)
>  {
> -	if (!cid_valid(sch, cid))
> +	s16 *tbl = rcu_dereference_all(scx_cid_to_cpu_tbl);
> +
> +	if (!cid_valid(sch, cid) || unlikely(!tbl))
>  		return -EINVAL;
> -	return __scx_cid_to_cpu(cid);
> +	return tbl[cid];
>  }
>  
>  /**
> @@ -133,13 +148,15 @@ static inline s32 scx_cid_to_cpu(struct scx_sched *sch, s32 cid)
>   * @cpu: cpu to look up
>   *
>   * Return the cid for @cpu or a negative errno on failure. Invalid cpu triggers
> - * scx_error() on @sch. Same lifetime guarantee as scx_cid_to_cpu().
> + * scx_error() on @sch. Same usage rules as scx_cid_to_cpu().
>   */
>  static inline s32 scx_cpu_to_cid(struct scx_sched *sch, s32 cpu)
>  {
> -	if (!scx_cpu_valid(sch, cpu, NULL))
> +	s16 *tbl = rcu_dereference_all(scx_cpu_to_cid_tbl);
> +
> +	if (!scx_cpu_valid(sch, cpu, NULL) || unlikely(!tbl))
>  		return -EINVAL;
> -	return __scx_cpu_to_cid(cpu);
> +	return tbl[cpu];
>  }
>  
>  /**
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index e6c4cbe1f182..6a4e3a4f6098 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -3334,6 +3334,7 @@ static void handle_hotplug(struct rq *rq, bool online)
>  {
>  	struct scx_sched *sch = scx_root;
>  	s32 cpu = cpu_of(rq);
> +	s32 cpu_or_cid = cpu;
>  
>  	atomic_long_inc(&scx_hotplug_seq);
>  
> @@ -3353,10 +3354,26 @@ static void handle_hotplug(struct rq *rq, bool online)
>  	else
>  		scx_offline_ecaps(rq);
>  
> +	/*
> +	 * The tables can't be retired while this function is running as the
> +	 * retirement is inside cpus_read_lock. However, scx_cpu_arg() is
> +	 * awkward here as the tables can be NULL after root enable failure and
> +	 * lockdep would trigger without surrounding rcu_read_lock(). Open code
> +	 * the translation. If the table is NULL, the ops are also cleared and
> +	 * @cpu_or_cid goes unused.
> +	 */
> +	if (scx_is_cid_type()) {
> +		s16 *tbl = rcu_dereference_check(scx_cpu_to_cid_tbl,
> +						 lockdep_is_cpus_held());
> +
> +		if (tbl)
> +			cpu_or_cid = tbl[cpu];
> +	}
> +
>  	if (online && SCX_HAS_OP(sch, cpu_online))
> -		SCX_CALL_OP(sch, cpu_online, NULL, scx_cpu_arg(cpu));
> +		SCX_CALL_OP(sch, cpu_online, NULL, cpu_or_cid);
>  	else if (!online && SCX_HAS_OP(sch, cpu_offline))
> -		SCX_CALL_OP(sch, cpu_offline, NULL, scx_cpu_arg(cpu));
> +		SCX_CALL_OP(sch, cpu_offline, NULL, cpu_or_cid);
>  	else
>  		scx_exit(sch, SCX_EXIT_UNREG_KERN,
>  			 SCX_ECODE_ACT_RESTART | SCX_ECODE_RSN_HOTPLUG,
> @@ -6197,11 +6214,12 @@ static void scx_root_disable(struct scx_sched *sch)
>  	scx_unlink_sched(sch);
>  
>  	/*
> -	 * scx_root clearing must be inside cpus_read_lock(). See
> -	 * handle_hotplug().
> +	 * scx_root clearing and cid table retirement must be inside
> +	 * cpus_read_lock(). See handle_hotplug().
>  	 */
>  	cpus_read_lock();
>  	RCU_INIT_POINTER(scx_root, NULL);
> +	scx_cid_retire_tables();
>  	cpus_read_unlock();
>  
>  	/*
> @@ -7195,10 +7213,9 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  	cpus_read_lock();
>  
>  	/*
> -	 * Build the cid mapping before publishing scx_root. The cid kfuncs
> -	 * dereference the cid arrays unconditionally once scx_prog_sched()
> -	 * returns non-NULL; the rcu_assign_pointer() below pairs with their
> -	 * rcu_dereference() to make the populated arrays visible.
> +	 * Build the cid mapping into a private under-construction set. It
> +	 * becomes visible to readers only through scx_cid_publish_tables() once
> +	 * ops.init_cids() has finalized the layout.
>  	 */
>  	ret = scx_cid_init(sch);
>  	if (ret) {
> @@ -7235,6 +7252,9 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  		}
>  	}
>  
> +	/* the cid layout is final, expose it to readers */
> +	scx_cid_publish_tables();
> +
>  	ret = scx_arena_pool_init(sch);
>  	if (ret) {
>  		cpus_read_unlock();
> @@ -9872,13 +9892,15 @@ __bpf_kfunc u32 scx_bpf_nr_online_cids(void)
>   *
>   * cid-addressed equivalent of bpf_get_smp_processor_id() for scx programs.
>   * The current cpu is trivially valid, so this is just a table lookup. Return
> - * -EINVAL if called from a non-SCX program before any scheduler has ever
> - * been enabled (the cid table is still unallocated at that point).
> + * -EINVAL if called before any scheduler has ever published its cid tables.
>   */
>  __bpf_kfunc s32 scx_bpf_this_cid(void)
>  {
> -	s16 *tbl = READ_ONCE(scx_cpu_to_cid_tbl);
> +	s16 *tbl;
> +
> +	guard(rcu)();
>  
> +	tbl = rcu_dereference(scx_cpu_to_cid_tbl);
>  	if (!tbl)
>  		return -EINVAL;
>  	return tbl[raw_smp_processor_id()];
> @@ -9937,13 +9959,17 @@ __bpf_kfunc s32 scx_bpf_task_cpu(const struct task_struct *p)
>   * @p: task of interest
>   *
>   * cid-addressed equivalent of scx_bpf_task_cpu(). task_cpu(p) is always a
> - * valid cpu, so this is just a table lookup. Return -EINVAL if called from
> - * a non-SCX program before any scheduler has ever been enabled.
> + * valid cpu, so this is just a table lookup. Return -EINVAL if called before
> + * any scheduler has ever published its cid tables.
>   */
>  __bpf_kfunc s32 scx_bpf_task_cid(const struct task_struct *p)
>  {
> -	s16 *tbl = READ_ONCE(scx_cpu_to_cid_tbl);
> +	s16 *tbl;
> +
> +	/* KF_RCU covers only @p - a sleepable program holds no RCU lock */
> +	guard(rcu)();
>  
> +	tbl = rcu_dereference(scx_cpu_to_cid_tbl);
>  	if (!tbl)
>  		return -EINVAL;
>  	return tbl[task_cpu(p)];
> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
> index 26bfda216524..a9a853a71061 100644
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
> @@ -1504,8 +1504,9 @@ struct scx_sched {
>  #ifdef CONFIG_EXT_SUB_SCHED
>  	/*
>  	 * pshard[] size captured at enable for the async RCU free path -
> -	 * scx_nr_cid_shards may be rewritten by a later scx_cid_init() before
> -	 * free runs. While sch is active, use the global.
> +	 * scx_nr_cid_shards may be rewritten by a later enable's
> +	 * scx_cid_publish_tables() before free runs. While sch is active, use
> +	 * the global.
>  	 */
>  	u32			nr_pshards;
>  #endif
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 8d8737149bc0..d7842a609d96 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -138,7 +138,9 @@ void scx_free_pshards(struct scx_sched *sch)
>  
>  static struct scx_pshard *alloc_pshard(struct scx_sched *sch, s32 shard_idx, s32 node)
>  {
> -	const struct scx_cid_shard *shard = &scx_cid_shard_ranges[shard_idx];
> +	const struct scx_cid_shard *shard =
> +		&rcu_dereference_protected(scx_cid_shard_ranges,
> +					   lockdep_is_held(&scx_enable_mutex))[shard_idx];
>  	size_t cmask_size = struct_size_t(struct scx_cmask, bits,
>  					  SCX_CMASK_NR_WORDS(shard->nr_cids));
>  	struct scx_pshard *pshard;
> @@ -176,17 +178,21 @@ static struct scx_pshard *alloc_pshard(struct scx_sched *sch, s32 shard_idx, s32
>  s32 scx_alloc_pshards(struct scx_sched *sch)
>  {
>  	struct scx_pshard **pshard;
> +	s32 *shard_node;
>  	s32 si;
>  
>  	if (!sch->is_cid_type || !sch->arena_pool)
>  		return 0;
>  
> +	shard_node = rcu_dereference_protected(scx_shard_node,
> +					       lockdep_is_held(&scx_enable_mutex));
> +
>  	pshard = kzalloc_objs(pshard[0], scx_nr_cid_shards, GFP_KERNEL);
>  	if (!pshard)
>  		return -ENOMEM;
>  
>  	for (si = 0; si < scx_nr_cid_shards; si++) {
> -		pshard[si] = alloc_pshard(sch, si, scx_shard_node[si]);
> +		pshard[si] = alloc_pshard(sch, si, shard_node[si]);
>  		if (!pshard[si]) {
>  			while (--si >= 0)
>  				free_pshard(pshard[si]);
> @@ -198,8 +204,9 @@ s32 scx_alloc_pshards(struct scx_sched *sch)
>  	sch->nr_pshards = scx_nr_cid_shards;
>  	/*
>  	 * Publish only after every entry is built so a reader observing
> -	 * @sch->pshard never sees a partially-filled array. Pair the store
> -	 * with a barrier and READ_ONCE() on the read side.
> +	 * @sch->pshard never sees a partially-filled array or unpublished cid
> +	 * tables. Pair the store with a barrier and an acquire load on the
> +	 * read side.
>  	 */
>  	smp_wmb();
>  	WRITE_ONCE(sch->pshard, pshard);
> @@ -524,7 +531,7 @@ void scx_process_sync_ecaps(struct rq *rq, struct task_struct *prev)
>  
>  	/* @cid is valid here: the cpu is active with queued syncs */
>  	cid = __scx_cpu_to_cid(cpu);
> -	shard = scx_cid_to_shard[cid];
> +	shard = rcu_dereference_all(scx_cid_to_shard)[cid];
>  
>  	batch = llist_del_all(&rq->scx.ecaps_to_sync);
>  	llist_for_each_safe(pos, tmp, batch) {
> @@ -618,7 +625,7 @@ void scx_unbypass_replay_ecaps(struct rq *rq, struct scx_sched *sch)
>  		return;
>  
>  	cid = __scx_cpu_to_cid(cpu);
> -	ps = sch->pshard[scx_cid_to_shard[cid]];
> +	ps = sch->pshard[rcu_dereference_all(scx_cid_to_shard)[cid]];
>  
>  	guard(raw_spinlock)(&ps->lock);
>  	queue_sync_ecaps(sch, cid);
> @@ -631,12 +638,23 @@ void scx_unbypass_replay_ecaps(struct rq *rq, struct scx_sched *sch)
>   */
>  void scx_online_ecaps(struct rq *rq)
>  {
> -	s32 cid = __scx_cpu_to_cid(cpu_of(rq));
> -	s32 shard = scx_cid_to_shard[cid];
>  	struct scx_sched *pos;
> +	s32 cid, shard;
> +
> +	/*
> +	 * Only a live hierarchy can have ecaps to reseed. This also keeps the
> +	 * table reads below away from an enable that failed before publishing
> +	 * the tables. A concurrent disable can't retire them, see
> +	 * handle_hotplug().
> +	 */
> +	if (!scx_enabled())
> +		return;
>  
>  	guard(rq_lock_irqsave)(rq);
>  
> +	cid = __scx_cpu_to_cid(cpu_of(rq));
> +	shard = rcu_dereference_all(scx_cid_to_shard)[cid];
> +
>  	scx_for_each_descendant_pre(pos, scx_root) {
>  		struct scx_pshard *ps;
>  
> @@ -2074,9 +2092,10 @@ __bpf_kfunc s32 scx_bpf_sub_caps(u64 cgroup_id, u64 caps, struct scx_cmask *out_
>  	/*
>  	 * The target's caps storage may not be set up yet (e.g. a self-read
>  	 * during ops.init_cids()). Pairs with the publish in
> -	 * scx_alloc_pshards(): a non-NULL pshard has every element set.
> +	 * scx_alloc_pshards(): a non-NULL pshard has every element set and the
> +	 * acquire also orders the cid table reads below against it.
>  	 */
> -	pshard = READ_ONCE(target->pshard);
> +	pshard = smp_load_acquire(&target->pshard);
>  	if (unlikely(!pshard)) {
>  		scx_error(sch, "scx_bpf_sub_caps() called before caps storage is initialized");
>  		return -ENODEV;
> @@ -2089,7 +2108,8 @@ __bpf_kfunc s32 scx_bpf_sub_caps(u64 cgroup_id, u64 caps, struct scx_cmask *out_
>  	}
>  
>  	for (si = ref.shard_first; si < ref.shard_end; si++) {
> -		const struct scx_cid_shard *shard = &scx_cid_shard_ranges[si];
> +		const struct scx_cid_shard *shard =
> +			&rcu_dereference_all(scx_cid_shard_ranges)[si];
>  		SCX_CMASK_DEFINE_SHARD(local_out, shard->base_cid, shard->nr_cids);
>  		u32 cap_bit;
>  
> -- 
> 2.55.0
> 

  reply	other threads:[~2026-07-22  8:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 22:31 [PATCHSET v2 sched_ext/for-7.3] sched_ext: Sub-scheduler and cid fixes Tejun Heo
2026-07-21 22:31 ` [PATCH v2 1/4] sched_ext: Blame the DSQ's owning scheduler for a runnable stall Tejun Heo
2026-07-21 22:31 ` [PATCH v2 2/4] sched_ext: Skip the default CPU selection while bypassing Tejun Heo
2026-07-21 22:31 ` [PATCH v2 3/4] sched_ext: Drop unused scx_cpumask_to_cmask() Tejun Heo
2026-07-21 22:31 ` [PATCH v2 4/4] sched_ext: Build the cid tables privately and publish them with RCU Tejun Heo
2026-07-22  7:30   ` Andrea Righi
2026-07-22  8:23   ` [PATCH v3 " Tejun Heo
2026-07-22  8:29     ` Andrea Righi [this message]
2026-07-22  8:00 ` [PATCHSET v2 sched_ext/for-7.3] sched_ext: Sub-scheduler and cid fixes Andrea Righi
2026-07-22  8:34 ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amB_Y-s4wcQL0_J8@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=emil@etsalapatis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox