Re: [PATCH v3 2/4] selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs

> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index ea7044f30adc..482b40dde2f9 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -11,6 +11,9 @@ CONFIG_BPF_STREAM_PARSER=y
>  CONFIG_BPF_SYSCALL=y
>  # CONFIG_BPF_UNPRIV_DEFAULT_OFF is not set
>  CONFIG_CGROUP_BPF=y
> +CONFIG_CGROUP_SCHED=y
> +CONFIG_FAIR_GROUP_SCHED=y
> +CONFIG_CFS_BANDWIDTH=y
>  CONFIG_CRYPTO_HMAC=y
>  CONFIG_CRYPTO_SHA256=y
>  CONFIG_CRYPTO_USER_API=y

This isn't a bug, but CONFIG_CGROUP_SCHED already appears in all five
config.<arch> fragments (config.x86_64:30, config.aarch64:26,
config.s390x:19, config.ppc64el:17, config.riscv64:17), and
CONFIG_CFS_BANDWIDTH already appears in config.x86_64:24.

Now that they are in the common config, should the per-arch copies be
removed so each option lives in one place?

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c
> new file mode 100644
> index 000000000000..cd7e92ababfb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c

[ ... ]

> @@ -0,0 +1,259 @@
> +void test_cgroup_iter_cpu(void)
> +{

[ ... ]

> +	/*
> +	 * cpu.stat cputime grows on every tick a task in the cgroup runs, so
> +	 * stop them all before comparing
> +	 */
> +	if (test__start_subtest("cgroup_iter_cpu__match")) {
> +		struct cpu_query filev = {};
> +		int i, stable = 0;
> +
> +		kill(hog, SIGSTOP);
> +		waitpid(hog, NULL, WUNTRACED);
> +		if (!ASSERT_OK(join_root_cgroup(), "join_root_cgroup"))
> +			goto cleanup_hog;
> +
> +		/*
> +		 * The period timer keeps adding to nr_periods for a while
> +		 * after the hog stops
> +		 */
> +		for (i = 0; i < 20; i++) {
> +			struct cpu_query before = {}, after = {};
> +
> +			if (!ASSERT_OK(parse_stats(cgroup_fd, &before, have_bw), "cpu.stat") ||
> +			    !ASSERT_OK(read_stats(link), "read stats") ||
> +			    !ASSERT_OK(parse_stats(cgroup_fd, &after, have_bw), "cpu.stat"))
> +				goto cleanup_hog;
> +
> +			if (!memcmp(&before, &after, sizeof(before))) {
> +				filev = before;
> +				stable = 1;
> +				break;
> +			}
> +			usleep(100000);
> +		}

This isn't a bug, but could the 20 iterations and usleep(100000) get names
the way cgroup_iter_memcg.c names MEMCG_STAT_RETRIES /
MEMCG_STAT_RETRY_DELAY_US, so the relationship to the 100ms cpu.max period
is visible?

[ ... ]


---
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/32419919743