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

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

On 15/7/26 23:55, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `test_global_percpu_data_subskel` selftest fails unconditionally because it asserts `var_cnt > 0` on a subskeleton that is generated with zero variables.
> --
> 
> --- Patch [6]: [PATCH bpf-next v10 6/9] selftests/bpf: Add tests to verify global percpu data ---
> commit c5a32c0630bc8d9b3aedfc264c75289a1f8e6d98
> Author: Leon Hwang <leon.hwang@linux.dev>
> 
> selftests/bpf: Add tests to verify global percpu data
> 
> This commit adds selftests to verify operations on global percpu data in BPF
> programs. It tests various libbpf APIs and skeleton behaviors for percpu
> maps. It also adds a test to ensure live variables of a subskeleton do not
> include 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
>> @@ -0,0 +1,37 @@
> 
> [ ... ]
> 
>> +	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]
> Since test_global_percpu_data.c only contains variables in .percpu* sections,
> and the subskeleton generator skips percpu array variables, will var_cnt
> always be exactly 0 here, causing this test to unconditionally fail?
> 
> Would it make sense to either add a standard global variable to
> test_global_percpu_data.c (e.g. in .bss or .data), or adjust this assertion?

My bad to miss a new 'int data3 SEC(".data.percpu")' in
test_global_percpu_data.c in this revision, which would mute this review.

Thanks,
Leon

> [...]