> diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
> b/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
> new file mode 100644
> index 0000000000000..65f5006844636
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
[ ... ]
> +static int write_file(const char *filename)
> +{
> + int ret = -1;
> + size_t written = 0;
> + char *buffer;
> + FILE *fp;
> +
> + fp = fopen(filename, "wb");
> + if (!fp)
> + goto out;
> +
> + buffer = malloc(BUFFER_SIZE);
> + if (!buffer)
> + goto cleanup_fp;
> +
> + memset(buffer, 'A', BUFFER_SIZE);
> +
> + while (written < FILE_SIZE) {
> + size_t to_write = FILE_SIZE - written < BUFFER_SIZE ?
> + FILE_SIZE - written : BUFFER_SIZE;
> +
> + if (fwrite(buffer, 1, to_write, fp) != to_write)
> + goto cleanup;
> + written += to_write;
> + }
> +
> + ret = 0;
> +cleanup:
> + free(buffer);
> +cleanup_fp:
> + fclose(fp);
> +out:
> + return ret;
> +}
Does write_file() report buffered flush failures? It sets ret = 0
before fclose() and then discards fclose()'s return value, so if the
final flush fails (for example, ENOSPC when writing the 32 MiB file
into /tmp), the function still returns 0.
The truncated file is caught later by read_file()'s st_size check, but
the child exits with CHILD_EXIT_READ_FILE instead of
CHILD_EXIT_WRITE_FILE, so child_exit_str() reports "read data file"
for what was really a write failure. The per-step exit codes added by
this patch exist precisely to pin down which step failed.
[ ... ]
> +/*
> + * Remove the reclaim target while the BPF program keeps running and
> + * verify that reclaim stops on the dying/removed cgroup instead of
> + * reclaiming from it.
> + *
> + * The target stays empty; the workload lives in the trigger cgroup and
> + * only keeps refaults flowing so that reclaim rounds keep starting,
> + * both before and after the target is removed. reclaim_calls growing
> + * while the target is alive proves that rounds really run (the kfunc
> + * returns 0 on the empty target, but the call is still counted), and
> + * after the removal the skip counters must grow while reclaim_calls
> + * and reclaimed_bytes stay frozen.
> + */
> +void test_memcg_async_reclaim_dying(void)
> +{
> + u64 trigger_cgroup_id, target_cgroup_id;
> + u64 calls_before, bytes_before;
> + char data_file[PATH_MAX] = "";
> + struct memcg_async_reclaim *skel = NULL;
> + pid_t reader_pid = -1;
> + int err, fd, i;
> +
> + err = setup_dying_cgroups(&trigger_cgroup_id, &target_cgroup_id);
> + if (!ASSERT_OK(err, "setup_dying_cgroups"))
> + return;
> +
> + err = setup_bpf(trigger_cgroup_id, target_cgroup_id, &skel);
> + if (!ASSERT_OK(err, "setup_bpf"))
> + goto out;
[ ... ]
> + /* Wait for reclaim rounds to reach the live target cgroup. */
> + for (i = 0; i < DYING_POLL_ITERS; i++) {
> + if (skel->bss->reclaim_calls > 0)
> + break;
> + usleep(DYING_POLL_INTERVAL_US);
> + }
> + if (!ASSERT_GT(skel->bss->reclaim_calls, 0, "reclaim_calls"))
> + goto out;
> +
> + remove_cgroup(CG_DYING_TARGET_DIR);
> +
> + /* Let reclaim passes that were already in flight drain. */
> + usleep(DYING_SETTLE_US);
> +
> + calls_before = skel->bss->reclaim_calls;
> + bytes_before = skel->bss->reclaimed_bytes;
> +
> + /* Wait for reclaim rounds to hit the removed cgroup. */
> + for (i = 0; i < DYING_POLL_ITERS; i++) {
> + if (skel->bss->reclaim_target_gone ||
> + skel->bss->reclaim_skipped_dying)
> + break;
> + usleep(DYING_POLL_INTERVAL_US);
> + }
> +
> + if (!skel->bss->reclaim_target_gone &&
> + !skel->bss->reclaim_skipped_dying) {
> + PRINT_FAIL("no reclaim round hit the removed cgroup (gone=%llu,
> dying=%llu)",
> + (unsigned long long)skel->bss->reclaim_target_gone,
> + (unsigned long
> long)skel->bss->reclaim_skipped_dying);
> + goto out;
> + }
> +
> + /*
> + * reclaim_skipped_dying shows that the CSS_DYING/CSS_ONLINE check
> + * caught the cgroup mid-teardown. Whether it is hit is timing
> + * dependent, because the cgroup may already be fully released, so
> + * only the combined skip count above is asserted.
> + */
> + printf("memcg_async_reclaim_dying: skips on removed cgroup: gone=%llu,
> dying=%llu\n",
> + (unsigned long long)skel->bss->reclaim_target_gone,
> + (unsigned long long)skel->bss->reclaim_skipped_dying);
> +
> + /* Nothing may have been reclaimed from the removed target. */
> + if (!ASSERT_EQ(skel->bss->reclaim_calls, calls_before, "reclaim_calls"))
> + goto out;
> + if (!ASSERT_EQ(skel->bss->reclaimed_bytes, bytes_before,
> + "reclaimed_bytes"))
> + goto out;
Can this test distinguish a BPF program that implements the
CSS_DYING/CSS_ONLINE check from one that does not?
The commit message says "Looking the target up by id is not enough:
bpf_cgroup_from_id() keeps handing back a cgroup until its last
reference is dropped, so the program checks the css flags and skips
reclaim once the target is offlined or dying", and the comment above
this test says it "verifies that reclaim stops on the removed target
instead of reclaiming from it".
But bpf_cgroup_from_id() does not keep handing back the cgroup after
rmdir. The lookup sequence is:
bpf_cgroup_from_id()
-> cgroup_get_from_id()
-> __cgroup_get_from_id() in kernel/cgroup/cgroup.c
-> kernfs_find_and_get_node_by_id()
kernfs_find_and_get_node_by_id() in fs/kernfs/dir.c fails on an
inactive node:
if (unlikely(!__kernfs_active(kn) || !atomic_inc_not_zero(&kn->count)))
goto err_unlock;
cgroup_destroy_locked() runs synchronously inside the rmdir syscall and
calls kernfs_remove(cgrp->kn) before it returns. By the time
remove_cgroup(CG_DYING_TARGET_DIR) at line 635 returns, every subsequent
reclaim round takes the get_cgroup_memcg_from_id() failure path in
progs/memcg_async_reclaim.c:132, bumping reclaim_target_gone and leaving
reclaim_calls / reclaimed_bytes frozen.
That happens whether or not cgroup_is_dying() exists in the program.
reclaim_skipped_dying is only reachable through a sub-microsecond race
in reclaim_cgroup() where the reference is acquired before rmdir but the
flags are read after, and the deliberate usleep(DYING_SETTLE_US) = 200
ms at line 638, placed before the calls_before/bytes_before snapshot,
makes even that window essentially unobservable.
So the break condition at lines 645-647 and all three assertions (lines
651-657, 670, 672) are satisfied by the lookup-failure path alone.
Deleting cgroup_is_dying() from the BPF program would not make this test
fail.
To actually exercise the dying window the test would have to keep the
target cgroup pinned so that bpf_cgroup_from_id() still resolves it
while it is offline (for example, hold a cgroup fd or a task in it, or
drive the removal concurrently with reclaim without the 200 ms settle)
and then assert on reclaim_skipped_dying specifically rather than on the
OR of the two counters.
> diff --git a/tools/testing/selftests/bpf/progs/memcg_async_reclaim.c
> b/tools/testing/selftests/bpf/progs/memcg_async_reclaim.c
> new file mode 100644
> index 0000000000000..e6839ade472bb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/memcg_async_reclaim.c
[ ... ]
> +/*
> + * One reclaim round targets RECLAIM_MAX_ITER batches of RECLAIM_SIZE
> + * each. Each bpf_wq callback reclaims a single batch and requeues the
> + * same work item for the next one, so no callback runs longer than one
> + * bounded reclaim pass.
> + */
> +#define RECLAIM_SIZE (32 * PAGE_SIZE)
> +#define RECLAIM_MAX_ITER 32
[ ... ]
> +/*
> + * A cgroup is dying once it has been offlined (CSS_ONLINE cleared) or
> + * CSS_DYING has been raised, mirroring cgroup_is_dead()/css_is_dying()
> + * in include/linux/cgroup.h. bpf_cgroup_from_id() can still hand back
> + * such a cgroup, because it only fails once the last reference has been
> + * dropped, so reclaim has to check these flags instead of relying on
> + * the lookup failing.
> + *
> + * CSS_ONLINE and CSS_DYING come from vmlinux.h: the kernel defines them
> + * in an anonymous enum, so bpf_core_enum_value() has no enum type to
> + * bind to, and redeclaring them locally would clash with the vmlinux.h
> + * enumerators. vmlinux.h is generated from the running kernel's BTF, so
> + * the values already match the target kernel.
> + */
> +static bool cgroup_is_dying(struct cgroup *cgrp)
> +{
> + unsigned int flags = cgrp->self.flags;
> +
> + return (flags & CSS_DYING) || !(flags & CSS_ONLINE);
> +}
Does the (flags & CSS_DYING) term ever fire?
CSS_DYING is never set on cgrp->self. The only assignment in the tree is
css->flags |= CSS_DYING in kill_css_sync() at kernel/cgroup/cgroup.c:6147.
kill_css_sync() has exactly two callers, and both pass a subsystem css,
never the cgroup's own self css:
- cgroup_destroy_locked() calls it under for_each_css(css, ssid, cgrp),
and that macro only ever yields cgrp->subsys[ssid].
- cgroup_apply_control_disable() calls it with css = cgroup_css(dsct, ss)
where ss comes from for_each_subsys(), so again dsct->subsys[ss->id].
kill_css_sync() also unconditionally dereferences css->ss->css_killed,
which would NULL-deref for &cgrp->self whose ss is NULL. So cgrp->self.flags
& CSS_DYING is always 0 and cgroup_is_dying() reduces to cgroup_is_dead().
Also, the comment above the helper claims bpf_cgroup_from_id() keeps
handing back the cgroup until the last reference is dropped. That is not
what the lookup does. bpf_cgroup_from_id() -> cgroup_get_from_id() ->
__cgroup_get_from_id() is kernfs_find_and_get_node_by_id() followed by
cgroup_tryget(). It starts failing as soon as cgroup_destroy_locked()
reaches kernfs_remove(cgrp->kn) or percpu_ref_kill(&cgrp->self.refcnt),
both long before the last reference is dropped. The window in which the
surviving !CSS_ONLINE check can still observe a cgroup is between
cgrp->self.flags &= ~CSS_ONLINE and kernfs_remove() a few statements
later, all under cgroup_mutex.
The comment at line 112 also states "vmlinux.h is generated from the
running kernel's BTF". tools/testing/selftests/bpf/vmlinux.h is generated
at build time from $(VMLINUX_BTF), which for a cross-built or packaged
selftest is not necessarily the kernel the test runs on.
[ ... ]
> +static int reclaim_work_fn(void *map, int *key, void *value)
> +{
> + struct wq_elem *elem = value;
> + u64 nr, size;
> +
> + if (!elem->remaining) {
> + /*
> + * Timer-triggered entry: start a new round only when the
> + * high cgroup refaults enough. Requeued entries skip this
> + * check and only look at remaining, so the refault delta
> + * is consumed once per round.
> + */
> + if (!should_reclaim_cgroup(wq_high_cgroup_id, &elem->prev_event,
> + elem->event_delta_threshold))
> + return 0;
> + elem->remaining = RECLAIM_MAX_ITER * RECLAIM_SIZE;
> + }
> +
> + /* One bounded reclaim pass per callback */
> + size = elem->remaining < RECLAIM_SIZE ? elem->remaining : RECLAIM_SIZE;
> + nr = reclaim_cgroup(wq_low_cgroup_id, size);
> + if (!nr) {
> + elem->remaining = 0;
> + return 0;
> + }
> +
> + /* try_to_free_mem_cgroup_pages() may reclaim more than requested */
> + if (nr >= elem->remaining)
> + elem->remaining = 0;
> + else
> + elem->remaining -= nr;
> +
> + /* Requeue the same work item for the next batch */
> + if (elem->remaining)
> + bpf_wq_start(&elem->work, 0);
> +
> + return 0;
> +}
Does RECLAIM_MAX_ITER actually bound an iteration count?
The macro is only ever used as a byte multiplier:
elem->remaining = RECLAIM_MAX_ITER * RECLAIM_SIZE = 32 * 128 KiB = 4 MiB
and the requeue loop is bounded by bytes actually reclaimed, not by
callback count. bpf_proactive_reclaim() returns the bytes that
try_to_free_mem_cgroup_pages() actually freed, which is frequently less
than the requested 128 KiB. A single page (4 KiB) is enough to keep the
round alive, so a round in which each pass frees one page needs 4 MiB /
4 KiB = 1024 callbacks, i.e. 32x more than the name and the file header
comment suggest.
Total work per round is still bounded at 4 MiB and each individual
callback is still one bounded pass, so there is no runaway. The defect
is that the documented iteration bound does not exist, which makes the
requeue budget hard to reason about.
---
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/33864052510
