bpf: BPF-driven proactive memcg reclaim

[kernel-patches-daemon-bpf]

Pull request for series with
subject: bpf: BPF-driven proactive memcg reclaim
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841

[kernel-patches-daemon-bpf]

Upstream branch: f79066c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-review-bot]

commit 0043ab339dce32da90812e3e3c3fcf71d043cf57
Author: Hui Zhu <zhuhui@kylinos.cn>

selftests/bpf: add memcg async reclaim test

Add memcg_async_reclaim selftest that verifies BPF-driven async
proactive reclaim can mitigate refault-induced slowdown under memory
pressure. The test creates a parent cgroup with a fixed memory.max, and
two child cgroups (high/low) under it. Both children concurrently write
and repeatedly read-fault a file larger than the shared limit.

> diff --git tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
> new file mode 100644
> index 000000000000..ece7bceaab3c
> --- /dev/null
> +++ tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
> @@ -0,0 +1,480 @@

[ ... ]

> +/*
> + * The workload files must sit on a regular filesystem: with swap
> + * disabled for the cgroup, tmpfs/ramfs pages are unevictable and would
> + * OOM the cgroup instead of exercising reclaim; they are also charged
> + * as anonymous memory, so they never raise the WORKINGSET_REFAULT_FILE
> + * events the BPF program monitors. Fall back to the current directory
> + * when /tmp is backed by such a filesystem.
> + */

The comment states that tmpfs/ramfs pages "are also charged as anonymous
memory." Is this accurate?

Looking at the page cache charge path (mm/filemap.c __filemap_add_folio()),
tmpfs/shmem folios go through folio_memcg_charge() and land in
NR_FILE_PAGES/NR_SHMEM, which shows up in memory.stat's file/shmem
counters, not as anonymous memory.

The reason they don't trigger WORKINGSET_REFAULT_FILE is different:
shmem folios are swap-backed, so folio_is_file_lru() returns false
(include/linux/mm_inline.h), and workingset_refault() accounts them at
WORKINGSET_REFAULT_ANON instead (mm/workingset.c).

The conclusion the comment draws is correct, but the stated reason is
not. For a patch aimed at the mm/memcg audience, could the wording be
adjusted to reflect that they sit on the anon LRU because they are
swap-backed?

> +static const char *workload_files_dir(void)
> +{
> +	struct statfs st;
> +
> +	if (!statfs("/tmp", &st) &&
> +	    (st.f_type == TMPFS_MAGIC || st.f_type == RAMFS_MAGIC))
> +		return ".";
> +	return "/tmp";
> +}

[ ... ]

> +static int setup_high_low_cgroups(u64 *high_cgroup_id, u64 *low_cgroup_id)
> +{
> +	int ret;
> +	char limit_buf[20];
> +
> +	ret = setup_cgroup_environment();
> +	if (!ASSERT_OK(ret, "setup_cgroup_environment"))
> +		goto cleanup;
> +
> +	ret = create_and_get_cgroup(CG_DIR);
> +	if (!ASSERT_GE(ret, 0, "create_and_get_cgroup " CG_DIR))
> +		goto cleanup;
> +	close(ret);
> +
> +	ret = enable_controllers(CG_DIR, "memory");
> +	if (!ASSERT_OK(ret, "enable_controllers"))
> +		goto cleanup;

A subsystem pattern flags this as potentially concerning: the test
treats an absent memory controller as a test failure rather than a skip.

enable_controllers() ends up in __enable_controllers() which writes to
cgroup.subtree_control and returns 1 when that write fails. On a kernel
built with CONFIG_MEMCG=n or booted with cgroup_disable=memory, the
memory controller does not exist, the write is rejected, and the test
reports FAIL.

The feature being tested is CONFIG_MEMCG-gated: mm/Makefile builds
bpf_memcontrol.o as obj-$(CONFIG_MEMCG), so on such a kernel there is
nothing to test. Note that 123 other prog_tests use test__skip() with
a reason string for missing capabilities.

However, CONFIG_MEMCG=y is present in every per-arch BPF selftest config
(config.x86_64, config.aarch64, config.s390x, etc.), so BPF CI will
never hit this path. It is only reachable with a hand-rolled kernel
config that omits it. Given this context, is the ASSERT_OK() treatment
acceptable?

[ ... ]

> +void test_memcg_wq_async_reclaim(void)
> +{
> +	u64 high_cgroup_id, low_cgroup_id;
> +	int err;
> +	double high_time = 0.0, low_time = 0.0;
> +	struct memcg_async_reclaim *skel = NULL;
> +
> +	err = setup_high_low_cgroups(&high_cgroup_id, &low_cgroup_id);
> +	if (!ASSERT_OK(err, "setup_high_low_cgroups reclaim"))
> +		return;
> +
> +	err = setup_bpf(high_cgroup_id, low_cgroup_id, &skel);
> +	if (!ASSERT_OK(err, "setup_bpf"))
> +		goto out;
> +
> +	err = run_high_low_workload(&high_time, &low_time, READ_TIMES);
> +	if (!ASSERT_OK(err, "run_high_low_workload reclaim"))
> +		goto out;
> +
> +	/*
> +	 * The timing comparison below alone cannot distinguish a working
> +	 * reclaim from a no-op one, so require that the BPF program
> +	 * actually reclaimed memory from the low cgroup.
> +	 */
> +	if (!ASSERT_GT(skel->bss->reclaim_calls, 0, "reclaim_calls"))
> +		goto out;
> +	if (!ASSERT_GT(skel->bss->reclaimed_bytes, 0, "reclaimed_bytes"))
> +		goto out;
> +
> +	if (high_time >= low_time)
> +		PRINT_FAIL("high cgroup not improved with async reclaim: high_time=%f low_time=%f",
> +			   high_time, low_time);

Does the zero-margin wall-clock comparison make the test brittle?

The test's final pass/fail criterion is a single-run timing comparison
between two symmetric workloads with no baseline. PRINT_FAIL() expands to
test__fail(), so this marks the whole test FAILED whenever high_time >=
low_time, including on a tie or a 1-microsecond inversion.

The two children are symmetric by construction: run_high_low_workload()
forks them within microseconds of each other and both run the identical
real_test_child_work() body (write a 32 MB file, then read-fault it 50
times) inside sibling cgroups under one shared memory.max of 32 MB.

Which one finishes first is decided by page-reclaim ordering, disk service
order, CPU scheduling, and host load. The test pins only memory.max and
memory.swap.max but does not pin the reclaim implementation (classic LRU
vs MGLRU, selected at runtime), the dirty-ratio sysctls, the CPU count, or
the backing device.

On a machine where the BPF-driven reclaim of the low cgroup does not
translate into a measurable ordering advantage, a fully correct kernel
reports FAIL.

The comment just above acknowledges that the timing signal is weak. The two
ASSERT_GT checks on reclaim_calls and reclaimed_bytes are deterministic and
verify that bpf_proactive_reclaim() actually ran. The timing comparison
adds no deterministic coverage on top of them, only a nondeterministic
failure mode.

For comparison, prog_tests/test_overhead.c is the only other prog_test that
uses clock_gettime(CLOCK_MONOTONIC, ...), and it only prints its
measurements. Could the comparison either require a substantial margin, be
bounded with retry logic, or report the ratio informationally with
fprintf(stdout, ...) instead of PRINT_FAIL()?

---

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

In-Reply-To-Subject: selftests/bpf: add memcg async reclaim test
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32339450346

[kernel-patches-review-bot]

commit ef25428021444857ad3bc688f2a0d302b6109d5e
Author: Hui Zhu <zhuhui@kylinos.cn>

mm/bpf: Add bpf_proactive_reclaim kfuncs

Expose memcg proactive reclaim to sleepable BPF programs:
unsigned long bpf_proactive_reclaim(memcg, size);
unsigned long bpf_proactive_reclaim_swappiness(memcg, size, swappiness);

They perform one reclaim pass on @memcg, like a write to memory.reclaim:
swap is allowed, and the anon/file balance follows the cgroup's
swappiness or an explicit override in [MIN_SWAPPINESS, MAX_SWAPPINESS]
plus SWAPPINESS_ANON_ONLY. Both go through a shared helper,
bpf_proactive_reclaim_pages(), which guards against reclaim recursion
and calls try_to_free_mem_cgroup_pages() with GFP_KERNEL and
MEMCG_RECLAIM_MAY_SWAP | MEMCG_RECLAIM_PROACTIVE, the same parameters
user_proactive_reclaim() uses, and unlike memory.reclaim they do not
retry until @size is reached.

> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d7647..dc51868b3acf 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c

[ ... ]

> @@ -159,6 +160,120 @@ __bpf_kfunc void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
>  	mem_cgroup_flush_stats(memcg);
>  }
>
> +/*
> + * Shared implementation of the proactive reclaim kfuncs: performs one
> + * reclaim pass on @memcg with @nr_pages as the goal, allowing swap, and
> + * @swappiness as the anon/file balance override (NULL to follow the
> + * cgroup's own swappiness setting). Returns the reclaimed amount in
> + * bytes, keeping the byte-based unit of the kfuncs' @size argument.
> + */
> +static unsigned long
> +bpf_proactive_reclaim_pages(struct mem_cgroup *memcg, unsigned long nr_pages,
> +			    int *swappiness)

This isn't a bug, but would it be worth trimming the comment above
bpf_proactive_reclaim_pages() and de-duplicating the identical 'Return:'
paragraph that now appears in both kfunc kernel-docs, so the byte/page
unit conversion is documented in exactly one place?

> +{
> +	unsigned long nr_reclaimed;
> +
> +	if (!nr_pages || unlikely(bpf_in_reclaim_context()))
> +		return 0;
> +
> +	nr_reclaimed = try_to_free_mem_cgroup_pages(memcg, nr_pages, GFP_KERNEL,
> +						    MEMCG_RECLAIM_MAY_SWAP |
> +						    MEMCG_RECLAIM_PROACTIVE,
> +						    swappiness);
> +
> +	return nr_reclaimed * PAGE_SIZE;
> +}
> +
> +/**
> + * bpf_proactive_reclaim - proactively reclaim memory from a memory
> + *                         cgroup
> + * @memcg: the target memory cgroup to reclaim from
> + * @size:  the amount of memory to reclaim, in bytes
> + *
> + * Trigger one proactive reclaim pass on @memcg, similar to a write to
> + * the memory.reclaim cgroup file: pages are reclaimed according to the
> + * cgroup's own swappiness setting and swap is allowed. Note that,
> + * unlike memory.reclaim, this does not retry until @size is reached;
> + * callers can invoke it again if needed.
> + *
> + * The reclaim runs with GFP_KERNEL, so this function must not be called
> + * from a context that holds a filesystem lock (e.g. an LSM hook invoked
> + * with inode_lock held): the reclaim path may enter filesystem shrinkers
> + * and deadlock trying to reacquire the lock. Contexts that set
> + * PF_MEMALLOC_NOFS/NOIO are handled by the gfp context inheritance.
> + *
> + * Return:
> + *   The amount of memory actually reclaimed, in bytes (rounded to full
> + *   pages), or 0 if @size is smaller than a page or the calling task is
> + *   already in a reclaim/freeing context (PF_MEMALLOC).
> + */
> +__bpf_kfunc unsigned long bpf_proactive_reclaim(struct mem_cgroup *memcg,
> +						unsigned long size)
> +{
> +	return bpf_proactive_reclaim_pages(memcg, size / PAGE_SIZE, NULL);
> +}
> +
> +/**
> + * bpf_proactive_reclaim_swappiness - proactively reclaim memory from a
> + *                                    memory cgroup with an explicit
> + *                                    swappiness
> + * @memcg:      the target memory cgroup to reclaim from
> + * @size:       the amount of memory to reclaim, in bytes
> + * @swappiness: swappiness override for this reclaim pass
> + *
> + * Same as bpf_proactive_reclaim(), except that the anon/file reclaim
> + * balance is controlled by @swappiness instead of the cgroup's
> + * swappiness setting. Valid values are [MIN_SWAPPINESS, MAX_SWAPPINESS]
> + * and SWAPPINESS_ANON_ONLY, which restricts reclaim to anon folios.
> + *
> + * Return:
> + *   The amount of memory actually reclaimed, in bytes (rounded to full
> + *   pages), (unsigned long)-1 if @swappiness is out of range, or 0 if
> + *   @size is smaller than a page or the calling task is already in a
> + *   reclaim/freeing context (PF_MEMALLOC).
> + */
> +__bpf_kfunc unsigned long
> +bpf_proactive_reclaim_swappiness(struct mem_cgroup *memcg, unsigned long size,
> +				 int swappiness)
> +{
> +	if (swappiness < MIN_SWAPPINESS || swappiness > SWAPPINESS_ANON_ONLY)
> +		return (unsigned long)-1;
> +
> +	return bpf_proactive_reclaim_pages(memcg, size / PAGE_SIZE,
> +					   &swappiness);
> +}

[ ... ]

---

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

In-Reply-To-Subject: mm/bpf: Add bpf_proactive_reclaim kfuncs
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32339450346

[kernel-patches-daemon-bpf]

Forwarding comment 5352475824 via email
In-Reply-To: 53762e3403fe8e8c263cf685cea5d55377ec5204.1787205002.git.zhuhui@kylinos.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/53762e3403fe8e8c263cf685cea5d55377ec5204.1787205002.git.zhuhui@kylinos.cn/

[kernel-patches-daemon-bpf]

Forwarding comment 5352494179 via email
In-Reply-To: e9b2a34418919f5c3ee2b80bcb4f9d80ce6ec422.1787205002.git.zhuhui@kylinos.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/e9b2a34418919f5c3ee2b80bcb4f9d80ce6ec422.1787205002.git.zhuhui@kylinos.cn/

[kernel-patches-daemon-bpf]

Upstream branch: 91ec203
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: aed1bf1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: d19862d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 4191e49
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 4954de7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: e40f61a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: a284ed4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: d83fba2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: ce36e38
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 05ea1b6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

Upstream branch: 1555de3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841
version: 4

[kernel-patches-daemon-bpf]

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1148841 expired. Closing PR.