> diff --git a/samples/bpf/memcg_async_reclaim.h
> b/samples/bpf/memcg_async_reclaim.h
> new file mode 100644
> index 000000000000..96fe03d12912
> --- /dev/null
> +++ b/samples/bpf/memcg_async_reclaim.h
> @@ -0,0 +1,39 @@
[ ... ]
> +struct reclaim_cfg {
> + /* Cgroup whose refaults are watched, i.e. the one under pressure. */
> + __u64 monitor_cgroup_id;
> + /* Cgroup that gets reclaimed from when the monitor refaults. */
> + __u64 target_cgroup_id;
> + /* Refault delta per tick that starts a reclaim round. */
> + __u64 refault_threshold;
> + /* Monitor tick period, in nanoseconds. */
> + __u64 interval_ns;
> + /* Bytes requested per bpf_wq callback. */
> + __u64 batch_bytes;
> + /* Callbacks per reclaim round. */
> + __u64 max_batches;
^^^^
Does this comment accurately describe max_batches? The BPF side does not
count callbacks. It uses max_batches only to size a byte budget and keeps
requeueing until that budget is spent:
samples/bpf/memcg_async_reclaim.bpf.c:reclaim_work_fn() {
elem->remaining = elem->max_batches * elem->batch_bytes;
...
if (nr >= elem->remaining)
elem->remaining = 0;
else
elem->remaining -= nr;
if (elem->remaining)
bpf_wq_start(&elem->work, 0);
}
bpf_proactive_reclaim() limits each pass to MEMCG_CHARGE_BATCH (64 pages),
which is 256 KiB with 4K pages.
With --batch 1M --max-batches 32 each callback reclaims at most about 256
KiB, so the round runs at least 128 callbacks, not 32. A partial reclaim
also adds callbacks. An overshoot, or a zero return that ends the round,
gives fewer.
Two other comments in the same patch describe it correctly: the .bpf.c
comment says "a clamped batch only means a round needs more callbacks,
because remaining is decremented by the bytes actually reclaimed", and the
loader says "One round budgets max_batches * batch_bytes". The header,
which documents the BPF/userspace interface, contradicts both.
The same wording is in the --max-batches help text in
memcg_async_reclaim_user.c.
Suggested wording: "Round budget, in units of batch_bytes."
> diff --git a/samples/bpf/memcg_async_reclaim_user.c
> b/samples/bpf/memcg_async_reclaim_user.c
> new file mode 100644
> index 000000000000..d5296fd0fab1
> --- /dev/null
> +++ b/samples/bpf/memcg_async_reclaim_user.c
> @@ -0,0 +1,1141 @@
[ ... ]
> +static void session_stop(struct session *s)
> +{
> + unsigned long long failures, last_err;
> +
> + if (!s->skel)
> + return;
> +
> + ring_buffer__free(s->rb);
> + s->rb = NULL;
^^^^
In bench mode the reclaim_events ring buffer is never consumed, so the
event counters and --verbose output do not work there. s->called,
s->skipped_dying and s->target_gone are only updated from
on_reclaim_event(), and that callback only runs from ring_buffer__poll().
The only caller of ring_buffer__poll() is session_poll(), and the only
caller of session_poll() is do_watch().
do_bench() goes session_start() -> run_workload() -> session_stop(), and
session_stop() calls ring_buffer__free(s->rb) without a final
ring_buffer__consume().
So every bench run prints "events: called=0 skipped_dying=0 target_gone=0"
even when the "total:" line read from .bss shows calls=N > 0.
> + failures = s->skel->bss->timer_failures;
> + last_err = s->skel->bss->last_reclaim_err;
> +
> + printf("\nran for %.1fs\n", now_sec() - s->start);
> + print_counters(s, "total:");
> + printf("events: called=%llu skipped_dying=%llu target_gone=%llu\n",
> + s->called, s->skipped_dying, s->target_gone);
> + if (failures)
> + printf("WARNING: monitor timer rearm failed %llu time(s),
> reclaim stopped early\n",
> + failures);
> + if (last_err)
> + printf("WARNING: bpf_proactive_reclaim() last failed with
> -%llu\n",
> + last_err);
The -v option, which usage() lists under "Common options" as "print every
reclaim event", prints nothing in bench mode.
The same missing final drain also drops any watch-mode events that arrive
between the last session_poll() and session_stop().
Should do_bench() poll or consume the ring buffer while the workload runs,
or should session_stop() call ring_buffer__consume(s->rb) before freeing
it?
[ ... ]
> +static int do_watch(struct options *o)
> +{
> + struct session s = {};
> + __u64 monitor_id, target_id;
> + double next_stats, deadline = 0;
> + int ret = 1;
[ ... ]
> + while (!READ_ONCE(exiting)) {
> + double now = now_sec();
> + int wait_ms;
> +
> + if (deadline && now >= deadline)
> + break;
> +
> + wait_ms = (int)((next_stats - now) * 1000);
> + if (wait_ms < 0)
> + wait_ms = 0;
> +
> + if (session_poll(&s, wait_ms)) {
^^^^
The --duration option is only checked once per statistics interval, so
watch mode can run well past the requested time.
wait_ms is computed only from next_stats and is never clamped to the
deadline. session_poll() keeps calling ring_buffer__poll() in 200 ms
slices until the whole wait_ms is used up, and it only returns early on a
signal or an error.
For example, with the default --stats 10 and --duration 3, nothing is
checked until about 10 s have passed on an idle system, so the program runs
for about 10 s instead of 3 s. With -s 60 -D 5 it runs for about 60 s.
usage() says "-D, --duration SEC stop after SEC seconds".
Should wait_ms be limited to min(next_stats, deadline) - now?
---
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/35955613591
