Re: [PATCH v2 bpf-next] selftests/bpf: Retry stat generation in cgroup_iter_memcg

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> index b7c18d590b99..5a1e08d39a06 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c

[ ... ]

> @@ -100,6 +128,7 @@ static void test_file(struct bpf_link *link, struct memcg_query *memcg_query)
>
>  static void test_shmem(struct bpf_link *link, struct memcg_query *memcg_query)
>  {
> +	int retries = 0;
>  	size_t len;
>  	int fd;
>
> @@ -113,12 +142,18 @@ static void test_shmem(struct bpf_link *link, struct memcg_query *memcg_query)
>  	if (!ASSERT_OK_FD(fd, "memfd_create"))
>  		return;
>
> +retry:
>  	if (!ASSERT_OK(fallocate(fd, 0, 0, len), "fallocate"))
>  		goto cleanup;
>
>  	if (!ASSERT_OK(read_stats(link), "read stats"))
>  		goto cleanup;
>
> +	if (!memcg_query->nr_shmem && ++retries < MEMCG_STAT_RETRIES) {
> +		usleep(MEMCG_STAT_RETRY_DELAY_US);
> +		goto retry;
> +	}
> +
>  	ASSERT_GT(memcg_query->nr_shmem, 0, "final shmem value");
>
>  cleanup:

Does the retry loop in test_shmem actually regenerate stat updates the way
test_anon, test_file, and test_pgfault do?

The retry: label sits above the fallocate() call, and nothing inside the
loop tears down the allocation. From the second iteration on, the range is
already populated. shmem_fallocate() in mm/shmem.c (mode 0 path) walks
each index with shmem_get_folio(..., SGP_FALLOC), every index becomes a
page-cache hit, and no new charge or NR_SHMEM/NR_FILE_PAGES delta gets
posted to vmstats->stats_updates.

So vmstats->stats_updates stays where the first pass left it (around 1-4k
units on the 128-CPU host the commit message mentions), which is well
under the 64 * 128 = 8192 threshold. That leaves
memcg_vmstats_needs_flush() returning false, and
bpf_mem_cgroup_flush_stats() exiting without flushing.

test_anon, test_file, and test_pgfault all munmap and then remap+refault
1024 pages on each iteration, posting a fresh ~2k abs() worth of deltas,
so they can reach both mechanism (a) the periodic flusher and mechanism
(b) crossing MEMCG_CHARGE_BATCH * num_online_cpus(). test_shmem appears
to depend entirely on stats_flush_dwork landing inside the 15 x 250ms =
3.75s window.

That is still better than the original single-pass behavior, but on a
large, mostly-idle host with NOHZ, deferrable timers (which
stats_flush_dwork is, per mm/memcontrol.c DECLARE_DEFERRABLE_WORK) may
not fire reliably while the machine is idle.

Would moving retry: above memfd_create(), or punching a hole with
FALLOC_FL_PUNCH_HOLE after the usleep() and before re-fallocating, give
test_shmem the same regeneration the other three subtests have? As the
commit message notes for nr_anon_mapped, the hole punch would need to come
after the sleep so the flusher observes the allocated state.


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