On Tue, 1 Sept 2026 at 21:04, <[email protected]> wrote:
>
> > diff --git
> > a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > index bfb1bf3fd427b..849f33259c003 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > @@ -2,29 +2,45 @@
> > /* Copyright (c) 2022 Bytedance */
> >
> > #include <test_progs.h>
> > +#include "bpf/libbpf_internal.h"
> > #include "test_map_lookup_percpu_elem.skel.h"
> >
> > void test_map_lookup_percpu_elem(void)
> > {
> > struct test_map_lookup_percpu_elem *skel;
> > - __u64 key = 0, sum;
> > - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> > + bool *possible = NULL;
> > + __u64 key = 0, sum = 0;
> > + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> > __u64 *buf;
> >
> > - buf = malloc(nr_cpus*sizeof(__u64));
> > - if (!ASSERT_OK_PTR(buf, "malloc"))
> > + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible",
> > &possible,
> > + &nr_cpu_ids);
> > + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> > return;
> >
> > - for (i = 0; i < nr_cpus; i++)
> > - buf[i] = i;
> > - sum = (nr_cpus - 1) * nr_cpus / 2;
> > + nr_cpus = libbpf_num_possible_cpus();
> > + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> > + goto free_mask;
> > +
> > + buf = malloc(nr_cpus * sizeof(*buf));
> > + if (!ASSERT_OK_PTR(buf, "malloc"))
> > + goto free_mask;
> > +
> > + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> > + if (!possible[cpu])
> > + continue;
> > + buf[slot++] = cpu;
> > + sum += cpu;
> > + }
>
> Does this buffer layout match how the kernel expects it?
>
> The userspace side fills buf[] with one entry per possible CPU, storing
> the logical CPU ID at packed index 'slot' (0, 1, 2, ...). For a mask
> like 0,2-3, this creates buf = {0, 2, 3} with 3 entries total.
>
> Looking at the kernel's update path, both percpu array and hash maps
> index the user buffer by logical CPU ID:
>
> kernel/bpf/arraymap.c:469 bpf_percpu_array_update()
> for_each_possible_cpu(cpu) {
> ptr = per_cpu_ptr(pptr, cpu);
> val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
>
> kernel/bpf/hashtab.c:1038 pcpu_copy_value()
> for_each_possible_cpu(cpu) {
> ...
> copy_map_value_long(&htab->map, per_cpu_ptr(pptrs, cpu),
> value + off + size * cpu);
>
> With the mask 0,2-3 (num_possible_cpus() = 3, nr_cpu_ids = 4), the
> update walks possible CPUs {0, 2, 3} and indexes the buffer as:
>
> cpu 0: *(value + 0) = buf[0] = 0 (intended)
> cpu 2: *(value + 16) = buf[2] = 3 (expects 2, gets 3)
> cpu 3: *(value + 24) (past the 24-byte buffer end)
>
> The userspace layout is packed (3 entries), but the kernel accesses it
> as sparse (indexed by CPU ID). The test sized the buffer for
> num_possible_cpus() bytes, but the kernel's 'size * cpu' indexing can
> exceed that when the possible mask has holes.
>
> Should the userspace buffer either be laid out by CPU ID (sized for
> nr_cpu_ids, with holes at non-possible indices), or should the kernel's
> update path use packed indexing like the lookup direction does?
>
> > + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> > + goto exit;
> >
> > skel = test_map_lookup_percpu_elem__open();
> > if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> > goto exit;
> >
> > skel->rodata->my_pid = getpid();
> > - skel->rodata->nr_cpus = nr_cpus;
> > + skel->rodata->nr_cpu_ids = nr_cpu_ids;
> >
> > ret = test_map_lookup_percpu_elem__load(skel);
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33521264909
For context on the review, there's a commit that's directly relevant
here but isn't in the mainline or bpf-next tree yet, so flagging it
explicitly:
Commit: 75b0a6db4300e4c2c9e97a0848deaa7acfb42fb7 ("bpf: Fix percpu map
update indexing with sparse CPU IDs")
That commit changes the array, hash, and cgroup-storage update paths
from value + size * cpu to a dense running offset, matching the lookup
side. With it, a possible mask of 0,2-3 maps the packed buffer as:
slot 0 -> CPU 0
slot 1 -> CPU 2
slot 2 -> CPU 3
I developed this selftest change on top of that commit but failed to
state the dependency.
