Re: [PATCH bpf-next v9 6/9] selftests/bpf: Add tests to verify global percpu data

[Leon Hwang <leon.hwang@linux.dev>]

On 14/7/26 01:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The test `test_global_percpu_data_subskel` unconditionally asserts that `var_cnt > 0`, but the compiled BPF object contains no non-percpu global variables, meaning `var_cnt` will always be exactly 0 and the test will reliably fail.
> --
> 
> --- Patch [6]: [PATCH bpf-next v9 6/9] selftests/bpf: Add tests to verify global percpu data ---
> commit 3e1db5f1fcec57a130c30e415035e1b1944b5e17
> Author: Leon Hwang <leon.hwang@linux.dev>
> 
> selftests/bpf: Add tests to verify global percpu data
> 
> This commit introduces tests for BPF global percpu data, verifying APIs like
> bpf_map__set_initial_value() and bpf_map__initial_value(). It also adds a
> subskeleton test to ensure that the live variables of a subskeleton do not
> include those for global percpu data.
> 
>> 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 0000000000000..8aebd533d86b5
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/global_percpu_subskel.c
> [ ... ]
>> +	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");
>> +	ASSERT_GT(subskel->subskel->var_cnt, 0, "var_cnt");
> 
> [Severity: Medium]
> Will this test unconditionally fail here?
> 
> Since all global variables in test_global_percpu_data.c are placed in .percpu
> sections:
> 
> int unused SEC(".percpu.looooooooong");
> int data2 SEC(".percpu.data");
> int data SEC(".percpu") = -1;
> 
> bpftool generates a subskeleton with a variable count of 0, because maps of type
> BPF_MAP_TYPE_PERCPU_ARRAY are skipped during subskeleton generation.
> 
> Thus, subskel->subskel->var_cnt should be exactly 0, which would cause this
> assertion to fail.
> 

Cite my reply in v7 [1]:

Looking at test_global_percpu_data.c, there are 5 non-percpu global
variables, which were added in the last patch.

volatile const __u32 num_cpus = 0;
volatile const int offsetof_num;
volatile const int elem_sz;
__u32 percpu_data_sum = 0;
bool run_iter = false;

So, subskel->subskel->var_cnt will be 5.

In the generated test_global_percpu_data.subskel.h:
        /* vars */
        s->var_cnt = 5;

hmm, if the next revision is needed, will add a global data variable to
mute this Sashiko review.

[1]
https://lore.kernel.org/bpf/6c0bc7ef-90b9-450d-969d-b7e0bc5932f8@linux.dev/

Thanks,
Leon