Re: [PATCH bpf-next v2] selftests/bpf: Avoid flaky resize value test for percpu data

[email protected]
Newsgroups org.kernel.vger.linux-kselftest,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <948e7ff41f942519880f9789faf6ca8ed9d0e2cf42eab5b9e1567ead5367b830@mail.kernel.org>
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_data_init.c b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> index 5671c31085cd..2b69290bcc3e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/global_data_init.c

[ ... ]

> @@ -236,6 +223,57 @@ static void test_global_percpu_data_lskel(void)
>  	test_global_percpu_data_lskel__destroy(lskel);
>  }
>
> +#define PAGE_SIZE 4096
> +
> +static void test_global_percpu_data_map_resize(void)
> +{
> +	const __u32 desired_sz = sysconf(_SC_PAGE_SIZE) * 2;
> +	struct test_global_percpu_data *skel;
> +	const int arr_value = -1;
> +	struct bpf_map *map;
> +	int err, prog_fd;
> +	size_t data_sz;
> +
> +	if (sysconf(_SC_PAGE_SIZE) != PAGE_SIZE) {
> +		test__skip();
> +		return;
> +	}
> +
> +	skel = test_global_percpu_data__open();
> +	if (!ASSERT_OK_PTR(skel, "test_global_percpu_data__open"))
> +		return;
> +
> +	map = skel->maps.percpu_arr;
> +	skel->percpu_arr->arr[0] = arr_value;
> +	err = bpf_map__set_value_size(map, desired_sz);

This isn't a bug, but since the guard above already pins the page size to
4096, would `const __u32 desired_sz = PAGE_SIZE * 2;` read more directly
here - and is the unguarded `#define PAGE_SIZE 4096` preferable to the
`#ifndef PAGE_SIZE` form used in sockopt.c?

> +	if (!ASSERT_OK(err, "bpf_map__set_value_size"))
> +		goto out;
> +	if (!ASSERT_EQ(bpf_map__value_size(map), desired_sz, "bpf_map__value_size"))
> +		goto out;
> +	if (!ASSERT_NEQ(bpf_map__btf_value_type_id(map), 0, "bpf_map__btf_value_type_id"))
> +		goto out;
> +
> +	skel->percpu_arr = bpf_map__initial_value(map, &data_sz);
> +	if (!ASSERT_EQ(data_sz, desired_sz, "data_sz"))
> +		goto out;
> +	if (!ASSERT_OK_PTR(skel->percpu_arr, "skel->percpu_arr"))
> +		goto out;

This isn't a bug, but would it read more consistently with
test_global_percpu_data_init() to check ASSERT_OK_PTR(skel->percpu_arr)
before ASSERT_EQ(data_sz, desired_sz)? The block this replaces checked
pointer-then-size, and the surviving test_global_percpu_data_init() at the
same file still does pointer-then-size.

[ ... ]


---
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/32156312094
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.