On 18/7/26 06:47, Emil Tsalapatis wrote:
> On Wed Jul 15, 2026 at 11:32 AM EDT, Leon Hwang wrote:
[...]
>> +
>> +static void test_percpu_data_on_cpus(int map_fd, int prog_fd)
>> +{
>> + __u64 args[2] = {0x1234ULL, 0x5678ULL};
>> + LIBBPF_OPTS(bpf_test_run_opts, topts,
>> + .ctx_in = args,
>> + .ctx_size_in = sizeof(args),
>> + .flags = BPF_F_TEST_RUN_ON_CPU,
>> + );
>> + int i, err, key = 0, num_online;
>> + bool *online;
>> +
>> + err = parse_cpu_mask_file("/sys/devices/system/cpu/online", &online,
>> &num_online);
>> + if (!ASSERT_OK(err, "parse_cpu_mask_file"))
>> + return;
>> +
>> + /* run on every online-CPU */
>> + for (i = 0; i < num_online; i++) {
>> + struct test_global_percpu_data__percpu data = {};
>> + __u64 flags;
>> +
>> + if (!online[i])
>> + continue;
>> +
>> + topts.cpu = i;
>> + topts.retval = -1;
>> + err = bpf_prog_test_run_opts(prog_fd, &topts);
>> + ASSERT_OK(err, "bpf_prog_test_run_opts");
>> + ASSERT_EQ(topts.retval, 0, "bpf_prog_test_run_opts retval");
>> +
>> + flags = ((__u64) i << 32) | BPF_F_CPU;
>> + err = bpf_map_lookup_elem_flags(map_fd, &key, &data, flags);
>> + if (!ASSERT_OK(err, "bpf_map_lookup_elem_flags"))
>> + break;
>> +
>> + ASSERT_EQ(data.data, 1, "data.data");
>> + ASSERT_TRUE(data.run, "data.run");
>> + ASSERT_EQ(data.nums[6], 0xc0de, "data.nums[6]");
>> + ASSERT_EQ(data.struct_data.i, 1, "struct_data.i");
>> + ASSERT_TRUE(data.struct_data.set, "struct_data.set");
>> + ASSERT_EQ(data.struct_data.nums[6], 0xc0de,
>> "struct_data.nums[6]");
>
> Can we add a pre-run assert to ensure that the per-cpu data has not
> already been modified by another run? Can we also add some cpuid
> specific assignment to ensure the runs are done on the proper CPU?
Ack.
>
>> + }
>> +
>> + free(online);
>> +}
>> +
[...]
>> diff --git a/tools/testing/selftests/bpf/prog_tests/global_percpu_subskel.c
>> b/tools/testing/selftests/bpf/prog_tests/global_percpu_subskel.c
>> new file mode 100644
>> index 000000000000..8aebd533d86b
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/global_percpu_subskel.c
>> @@ -0,0 +1,37 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <test_progs.h>
>> +#include "test_global_percpu_data.subskel.h"
>> +
>> +void test_global_percpu_data_subskel(void)
>> +{
>> + struct test_global_percpu_data *subskel = NULL;
>> + struct bpf_object *obj;
>> + int i;
>> +
>> + obj = bpf_object__open_file("./test_global_percpu_data.bpf.o", NULL);
>> + if (!ASSERT_OK_PTR(obj, "bpf_object__open_file"))
>> + return;
>> +
>> + subskel = test_global_percpu_data__open(obj);
>> + if (!ASSERT_OK_PTR(subskel, "test_global_percpu_data__open"))
>> + goto out;
>> +
>> + if (!ASSERT_OK_PTR(subskel->subskel, "subskel"))
>> + goto out;
>> + if (!ASSERT_OK_PTR(subskel->maps.percpu, "maps.percpu"))
>> + goto out;
>> + ASSERT_EQ(bpf_map__type(subskel->maps.percpu),
>> BPF_MAP_TYPE_PERCPU_ARRAY,
>> + "percpu_map_type");
>
> Not sure why these assertions would be necessary, wouldn't the test crash or
> quickly fail if they didn't hold?
>
>> + ASSERT_GT(subskel->subskel->var_cnt, 0, "var_cnt");
>> +
>> + for (i = 0; i < subskel->subskel->var_cnt; i++) {
>> + const struct bpf_var_skeleton *var;
>> +
>> + var = (void *) subskel->subskel->vars + i *
>> subskel->subskel->var_skel_sz;
>> + ASSERT_NEQ(var->map, &subskel->maps.percpu, "var");
>> + }
>
> If we turn the substest to SYSCALL instead of TRACEPOINT we can read
> and report the return value as the test happens and keep them
> self-contained.
It is to verify that the bpftool-generated subskeleton should not
contain the global percpu variables.
I think I should test it manually by checking the generated .subskel.h.
Will drop this subskel test.
>
>> +
>> +out:
>> + test_global_percpu_data__destroy(subskel);
>> + bpf_object__close(obj);
>> +}
>> diff --git a/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
>> b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
>> new file mode 100644
>> index 000000000000..54380dfb11a5
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>> +
>> +int unused SEC(".percpu.looooooooong");
>> +int data2 SEC(".percpu.data");
>> +int data SEC(".percpu") = -1;
>> +int nums[7] SEC(".percpu");
>> +char run SEC(".percpu") = 0;
>
> Can we add a comment that those are used to test the names in the
> userspace portion of the test? As they stand they seem unused.
Ack.
>
>> +struct {
>> + char set;
>> + int i;
>> + int nums[7];
>> +} struct_data SEC(".percpu") = {
>> + .set = 0,
>> + .i = -1,
>> +};
>> +
>> +SEC("raw_tp/task_rename")
>
> Is this actually installed as a tracepoint? I think we run it as a
> program. Is putting it in raw_tp doing anything in any way?
Will drop '/task_rename'.
Thanks,
Leon
>
>> +__auxiliary
>> +int update_percpu_data(void *ctx)
>> +{
>> + struct_data.nums[6] = 0xc0de;
>> + struct_data.set = 1;
>> + struct_data.i = 1;
>> + nums[6] = 0xc0de;
>> + data = 1;
>> + run = 1;
>> + return 0;
>> +}
>> +
>> +char _license[] SEC("license") = "GPL";
>
