Re: [PATCH 16/17] tools/sched_ext: scx_qmap: Port to cid-form struct_ops

[Andrea Righi <arighi@nvidia.com>]

Hello,

On Wed, Apr 29, 2026 at 09:47:12PM +0900, Changwoo Min wrote:
> 
> On 4/29/26 5:35 AM, Tejun Heo wrote:
> > /* @@ -1083,6 +1082,18 @@s32·BPF_STRUCT_OPS_SLEEPABLE(qmap_init) › ›
> > return·-EINVAL; › } +› /*+› ·*·cid-
> > override·test·hook.·Must·run·before·anything·that·reads·the+›
> > ·*·cid·space·(scx_bpf_nr_cids,·cmask_init,·etc.).·On·invalid·input,+›
> > ·*·the·kfunc·calls·scx_error()·which·aborts·the·scheduler.+› ·*/+›
> > if·(cid_override_mode)·{+› ›
> > scx_bpf_cid_override((const·s32·*)cid_override_cpu_to_cid,+› › › ›
> > ·····cid_override_nr_cpus·*·sizeof(s32),+› › › ›
> > ·····(const·s32·*)cid_override_shard_start,+› › › ›
> > ·····cid_override_nr_shards·*·sizeof(s32));+› }+
> 
> This cause the following compilation error due to argument mismatch:
> 
> scx_qmap.bpf.c:1093:10: error: too many arguments to function call, expected
> 2, have 4
>  1091 |                 scx_bpf_cid_override((const s32
> *)cid_override_cpu_to_cid,
>       |                 ~~~~~~~~~~~~~~~~~~~~
> 
>  1092 |                                      cid_override_nr_cpus *
> sizeof(s32),
>  1093 |                                      (const s32
> *)cid_override_shard_start,
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  1094 |                                      cid_override_nr_shards *
> sizeof(s32));
>       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/changwoo/ws-multics69/dev/linux-tj/tools/sched_ext/include/scx/compat.bpf.h:130:20:
> note:
>       'scx_bpf_cid_override' declared here
> 
>   130 | static inline void scx_bpf_cid_override(const s32 *cpu_to_cid, u32
> cpu_to_cid__sz)
>       |                    ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The correct one should be as follows:
> 
> > scx_bpf_cid_override((const·s32·*)cid_override_cpu_to_cid,+› › › ›
> > ·····cid_override_nr_cpus * sizeof(s32));
> 
> Reviewed-by: Changwoo Min <changwoo@igalia.com>

And after fixing scx_bpf_cid_override() I'm also getting this with
`scx_qmap -C shuffle`:

0: R1=ctx() R10=fp0
; s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init) @ scx_qmap.bpf.c:1069
0: (b4) w1 = 0                        ; R1=0
; u32 nr_pages, key = 0, i; @ scx_qmap.bpf.c:1072
1: (63) *(u32 *)(r10 -4) = r1         ; R1=0 R10=fp0 fp-8=0000????
; if (scx_bpf_nr_cids() > SCX_QMAP_MAX_CPUS) { @ scx_qmap.bpf.c:1076
2: (85) call scx_bpf_nr_cids#110275   ; R0=scalar()
3: (a6) if w0 < 0x401 goto pc+14 18: R10=fp0 fp-8=0000pppp
; if (cid_override_mode) { @ scx_qmap.bpf.c:1087
18: (18) r1 = 0xffffc90000322260      ; R1=map_value(map=scx_qmap.rodata,ks=4,vs=964,imm=608)
20: (61) r1 = *(u32 *)(r1 +0)         ;
21: (05) goto pc+0
; scx_bpf_nr_cpu_ids() * (u32)sizeof(s32)); @ scx_qmap.bpf.c:1090
22: (85) call scx_bpf_nr_cpu_ids#110276       ; R0=scalar()
; if (bpf_ksym_exists(scx_bpf_cid_override___compat)) @ compat.bpf.h:132
23: (18) r1 = 0xffffffff81464430      ; R1=rdonly_mem(sz=0)
25: (15) if r1 == 0x0 goto pc+5       ; R1=rdonly_mem(sz=0)
; scx_bpf_nr_cpu_ids() * (u32)sizeof(s32)); @ scx_qmap.bpf.c:1090
26: (64) w0 <<= 2                     ; R0=scalar(smin=0,smax=umax=umax32=0xfffffffc,smax32=0x7ffffffc,var_off=(0x0; 0xfffffffc))
; return scx_bpf_cid_override___compat(cpu_to_cid, cpu_to_cid__sz); @ compat.bpf.h:133
27: (18) r1 = 0xffffc90001526000      ; R1=map_value(map=scx_qmap.bss,ks=4,vs=4128)
29: (bc) w2 = w0                      ; R0=scalar(id=2,smin=0,smax=umax=umax32=0xfffffffc,smax32=0x7ffffffc,var_off=(0x0; 0xfffffffc)) R2=scalar(id=2,smin=0,smax=umax=umax32=0xfffffffc,smax32=0x7ffffffc,var_off=(0x0; 0xfffffffc))
30: (85) call scx_bpf_cid_override#110197
R2 unbounded memory access, use 'var &= const' or 'if (var < const)'
arg#0 arg#1 memory, len pair leads to invalid memory access
processed 28 insns (limit 1000000) max_states_per_insn 0 total_states 2 peak_states 2 mark_read 0

The following seems to fix everything for me.

Thanks,
-Andrea

 tools/sched_ext/scx_qmap.bpf.c | 26 +++++++++++++++++---------
 tools/sched_ext/scx_qmap.c     | 16 ++--------------
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
index f55192c7c51aa..800a92fdb6db7 100644
--- a/tools/sched_ext/scx_qmap.bpf.c
+++ b/tools/sched_ext/scx_qmap.bpf.c
@@ -63,8 +63,6 @@ const volatile u32 max_tasks;
  *   3 = invalid: non-monotonic shard_start
  */
 const volatile u32 cid_override_mode;
-const volatile u32 cid_override_nr_cpus;
-const volatile u32 cid_override_nr_shards;
 /*
  * Arrays live in bss (writable) because scx_bpf_cid_override()'s BPF
  * verifier signature treats its len-paired pointer as read/write - rodata
@@ -72,7 +70,6 @@ const volatile u32 cid_override_nr_shards;
  * them before SCX_OPS_LOAD, same as rodata, and nothing writes them after.
  */
 s32 cid_override_cpu_to_cid[SCX_QMAP_MAX_CPUS];
-s32 cid_override_shard_start[SCX_QMAP_MAX_CPUS];
 
 UEI_DEFINE(uei);
 
@@ -1073,12 +1070,25 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init)
 {
 	u8 __arena *slab;
 	u32 nr_pages, key = 0, i;
+	u32 nr_cids, nr_cpu_ids;
 	struct bpf_timer *timer;
 	s32 ret;
 
-	if (scx_bpf_nr_cids() > SCX_QMAP_MAX_CPUS) {
+	nr_cids = scx_bpf_nr_cids();
+	nr_cpu_ids = scx_bpf_nr_cpu_ids();
+
+	/*
+	 * Separate compares so the verifier tracks each upper bound; needed for
+	 * scx_bpf_cid_override(ptr, nr_cpu_ids * sizeof(s32)) vs bss array size.
+	 */
+	if (nr_cids > SCX_QMAP_MAX_CPUS) {
 		scx_bpf_error("nr_cids=%u exceeds SCX_QMAP_MAX_CPUS=%d",
-			      scx_bpf_nr_cids(), SCX_QMAP_MAX_CPUS);
+			      nr_cids, SCX_QMAP_MAX_CPUS);
+		return -EINVAL;
+	}
+	if (nr_cpu_ids > SCX_QMAP_MAX_CPUS) {
+		scx_bpf_error("nr_cpu_ids=%u exceeds SCX_QMAP_MAX_CPUS=%d",
+			      nr_cpu_ids, SCX_QMAP_MAX_CPUS);
 		return -EINVAL;
 	}
 
@@ -1089,9 +1099,7 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init)
 	 */
 	if (cid_override_mode) {
 		scx_bpf_cid_override((const s32 *)cid_override_cpu_to_cid,
-				     cid_override_nr_cpus * sizeof(s32),
-				     (const s32 *)cid_override_shard_start,
-				     cid_override_nr_shards * sizeof(s32));
+				     nr_cpu_ids * (u32)sizeof(s32));
 	}
 
 	/*
@@ -1133,7 +1141,7 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init)
 		scx_bpf_error("failed to allocate idle cmask");
 		return -ENOMEM;
 	}
-	cmask_init(qa_idle_cids, 0, scx_bpf_nr_cids());
+	cmask_init(qa_idle_cids, 0, nr_cids);
 
 	ret = scx_bpf_create_dsq(SHARED_DSQ, -1);
 	if (ret) {
diff --git a/tools/sched_ext/scx_qmap.c b/tools/sched_ext/scx_qmap.c
index a533542e3ca52..f3218610b5e5c 100644
--- a/tools/sched_ext/scx_qmap.c
+++ b/tools/sched_ext/scx_qmap.c
@@ -155,7 +155,6 @@ int main(int argc, char **argv)
 		case 'C': {
 			u32 nr_cpus = libbpf_num_possible_cpus();
 			u32 mode, i;
-			s32 shard_sz = 4;
 
 			if (!strcmp(optarg, "shuffle"))
 				mode = 1;
@@ -168,7 +167,6 @@ int main(int argc, char **argv)
 				return 1;
 			}
 			skel->rodata->cid_override_mode = mode;
-			skel->rodata->cid_override_nr_cpus = nr_cpus;
 
 			/* shuffle: reversed cpu_to_cid, bad-dup: dup cid 0, bad-mono: identity */
 			for (i = 0; i < nr_cpus; i++) {
@@ -179,19 +177,9 @@ int main(int argc, char **argv)
 			}
 			if (mode == 2 && nr_cpus >= 2)
 				skel->bss->cid_override_cpu_to_cid[1] = 0;
+			if (mode == 3)
+				skel->bss->cid_override_cpu_to_cid[0] = (s32)nr_cpus;
 
-			/* shards of shard_sz each */
-			skel->rodata->cid_override_nr_shards = (nr_cpus + shard_sz - 1) / shard_sz;
-			for (i = 0; i < skel->rodata->cid_override_nr_shards; i++)
-				skel->bss->cid_override_shard_start[i] = i * shard_sz;
-
-			if (mode == 3 && skel->rodata->cid_override_nr_shards >= 3) {
-				/* swap [1] and [2] so shard_start is not monotonically increasing */
-				s32 tmp = skel->bss->cid_override_shard_start[1];
-				skel->bss->cid_override_shard_start[1] =
-					skel->bss->cid_override_shard_start[2];
-				skel->bss->cid_override_shard_start[2] = tmp;
-			}
 			break;
 		}
 		case 'v':