Re: [PATCH bpf-next v2] selftests/bpf: Add bpf_proactive_reclaim test

[bot+bpf-ci]

> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2b883b388f90..5621ef94ad7e 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -57,6 +57,7 @@ CONFIG_LIRC=y
>  CONFIG_LIVEPATCH=y
>  CONFIG_LWTUNNEL=y
>  CONFIG_LWTUNNEL_BPF=y
> +CONFIG_MEMCG=y
>  CONFIG_MODULE_SIG=y
>  CONFIG_MODULE_SRCVERSION_ALL=y
>  CONFIG_MODULE_UNLOAD=y

[ ... ]

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

[ ... ]

> +	/*
> +	 * Charge FILE_SIZE of page cache to the cgroup. Reading rather than
> +	 * writing keeps the pages clean, so reclaim does not have to start
> +	 * writeback before it can evict them.
> +	 */
> +	if (!ASSERT_OK(ftruncate(data_fd, FILE_SIZE), "ftruncate"))
> +		goto out;
> +	for (off = 0; off < (off_t)FILE_SIZE; off += sizeof(buf))
> +		if (!ASSERT_GT(read(data_fd, buf, sizeof(buf)), 0, "read"))
> +			goto out;

Could this allocation method cause issues if file holes are mapped to the
zero PFN in the future? The read() approach assumes that accessing a
ftruncate()-created sparse file will allocate page cache, but if the kernel
someday maps file holes to a shared zero page instead of allocating backing
pages, this wouldn't necessarily give us page cache to reclaim.

Barry Song suggested using write() followed by fsync() as a more future-proof
way to ensure actual page cache allocation, which would also allow removing
the two lines of explanatory comments above.

> +
> +	args.cgroup_id = cgroup_id;
> +	args.size = FILE_SIZE;
> +	skel->bss->reclaimed = 0;
> +	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.memcg_proactive_reclaim),
> +				     &opts);
> +	if (!ASSERT_OK(err, "test_run"))
> +		goto out;
> +	if (!ASSERT_EQ(opts.retval, 0, "retval"))
> +		goto out;
> +
> +	/*
> +	 * A single call is a single bounded pass: it reclaims something, but
> +	 * stops well short of the requested size instead of retrying until the
> +	 * goal is reached the way a write to memory.reclaim does.
> +	 */
> +	ASSERT_GT(skel->bss->reclaimed, 0, "reclaimed");
> +	ASSERT_LT(skel->bss->reclaimed, (__s64)FILE_SIZE, "single pass");

[ ... ]


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