> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d7647..b0a0d4c55dc4 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/memcontrol.h>
> +#include <linux/swap.h>
> #include <linux/bpf.h>
>
> __bpf_kfunc_start_defs();
[ ... ]
> +/*
> + * Reclaim must not recurse. try_to_free_mem_cgroup_pages() unconditionally
> + * overwrites current->reclaim_state on entry and resets it to NULL on exit.
> + * So invoking it from an in-flight reclaim would clobber the outer reclaim
> + * state and corrupt its accounting.
> + *
> + * The guard is PF_MEMALLOC. Every reclaim entry point marks the current
> + * task with it for the whole reclaim window: try_to_free_mem_cgroup_pages()
> + * and __perform_reclaim() do so via memalloc_noreclaim_save(), and kswapd
> + * keeps it set for its entire lifetime. A hook inside the reclaim path
> + * (shrink_node, shrink_slab, ...) executes in the context of the
> + * reclaiming task, where current->flags already carries the flag. The page
> + * allocator, the memcg charging path and node_reclaim() rely on the same
> + * flag to avoid reclaim recursion.
> + *
> + * In try_to_free_mem_cgroup_pages(), reclaim_state is set slightly before
> + * PF_MEMALLOC, with only a tracepoint in between, which a sleepable BPF
> + * program cannot attach to.
> + * Also, PF_MEMALLOC is set in some non-reclaim contexts (e.g. direct
> compaction
> + * and vmalloc), where the kfunc conservatively refuses to reclaim as well.
> + */
> +static bool bpf_in_reclaim_context(void)
> +{
> + return current->flags & PF_MEMALLOC;
> +}
> +
> +/*
> + * 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).
> + */
> +static unsigned long
> +bpf_proactive_reclaim_pages(struct mem_cgroup *memcg, unsigned long nr_pages,
> + int *swappiness)
> +{
> + if (!nr_pages || unlikely(bpf_in_reclaim_context()))
> + return 0;
> +
> + return try_to_free_mem_cgroup_pages(memcg, nr_pages, GFP_KERNEL,
> + MEMCG_RECLAIM_MAY_SWAP |
> + MEMCG_RECLAIM_PROACTIVE,
> + swappiness);
> +}
Does the PF_MEMALLOC-only guard fully close the recursion window?
The comment states "In try_to_free_mem_cgroup_pages(), reclaim_state
is set slightly before PF_MEMALLOC, with only a tracepoint in between,
which a sleepable BPF program cannot attach to."
The restriction on tracepoint attachment is accurate - sleepable BPF
programs cannot attach to non-faultable tracepoints like
mm_vmscan_memcg_reclaim_begin. But the tracepoint dispatcher itself is
attachable:
Looking at mm/vmscan.c:try_to_free_mem_cgroup_pages():
set_task_reclaim_state(current, &sc.reclaim_state);
trace_mm_vmscan_memcg_reclaim_begin(sc.gfp_mask, 0, memcg);
noreclaim_flag = memalloc_noreclaim_save();
The trace_mm_vmscan_memcg_reclaim_begin() macro expands to a call to
__traceiter_mm_vmscan_memcg_reclaim_begin(), which is a plain global
function generated by __DEFINE_TRACE_EXT(). It carries no notrace
attribute, mm/Makefile doesn't remove ftrace flags from vmscan.o, and
it isn't in btf_id_deny, making it fentry-attachable.
can_be_sleepable() in kernel/bpf/verifier.c returns true for
BPF_TRACE_FENTRY, and check_attach_btf_id() applies no further
restriction beyond btf_id_deny. So a sleepable fentry program on
__traceiter_mm_vmscan_memcg_reclaim_begin would be accepted by the
verifier.
Since the kfuncs are registered for BPF_PROG_TYPE_UNSPEC, such a program
can call bpf_proactive_reclaim().
In that window, current->reclaim_state points to the outer reclaim's
state but PF_MEMALLOC is still clear, so bpf_in_reclaim_context()
returns false. The nested try_to_free_mem_cgroup_pages() would then:
- Trip the WARN_ON_ONCE(rs && task->reclaim_state) in
set_task_reclaim_state()
- On exit, execute set_task_reclaim_state(current, NULL), leaving
current->reclaim_state == NULL for the rest of the outer reclaim
The outer reclaim continues and under MGLRU, shrink_lruvec() calls
lru_gen_shrink_lruvec() which calls set_mm_walk() in mm/vmscan.c:
struct lru_gen_mm_walk *walk = current->reclaim_state->mm_walk;
...
current->reclaim_state->mm_walk = walk;
evict_folios() has the same pattern. Result: NULL pointer dereference.
On the classic LRU path the outcome would be lost reclaim accounting in
flush_reclaim_state(), plus the WARN_ON_ONCE on the outer exit.
Would testing current->reclaim_state in addition to PF_MEMALLOC close
this window?
> +/**
> + * 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.
> + *
> + * Return:
> + * The number of pages actually reclaimed, 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);
> +}
The kfunc takes @size in bytes but returns a count in pages
(try_to_free_mem_cgroup_pages() returns nr_reclaimed in pages). Input
and output use different units.
Every other kfunc in this file normalizes to bytes:
bpf_mem_cgroup_usage() returns page_counter_read(&memcg->memory) *
PAGE_SIZE and is documented as "The current memory cgroup size in
bytes"; bpf_mem_cgroup_page_state() documents "The output is in bytes."
A natural BPF loop like:
usage = bpf_mem_cgroup_usage(memcg);
while (usage > target)
usage -= bpf_proactive_reclaim(memcg, usage - target);
would be off by PAGE_SIZE. The kdoc does say "The number of pages
actually reclaimed", but since this is UAPI-visible the unit asymmetry
is worth resolving before it freezes. Would it make sense to return
nr * PAGE_SIZE, or take @nr_pages instead?
> +/**
> + * 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 number of pages actually reclaimed, or 0 if @size is smaller
> + * than a page, @swappiness is out of range, 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 0;
> +
> + return bpf_proactive_reclaim_pages(memcg, size / PAGE_SIZE,
> + &swappiness);
> +}
An out-of-range @swappiness returns 0, the same value as a successful
call that reclaimed nothing, and the same as a call refused due to
PF_MEMALLOC context. A BPF program cannot distinguish a programming
error from legitimate lack of progress.
The sibling kfuncs in this file use a distinguishable sentinel for
invalid parameters:
bpf_mem_cgroup_vm_events(): if
(unlikely(!memcg_vm_event_item_valid(event)))
return (unsigned long)-1;
bpf_mem_cgroup_page_state(): if (unlikely(!memcg_stat_item_valid(idx)))
return (unsigned long)-1;
The userspace equivalent also rejects rather than ignores:
user_proactive_reclaim() returns -EINVAL for out-of-range swappiness in
mm/vmscan.c. Would returning (unsigned long)-1 here match both this
file's convention and the write(2) behaviour of memory.reclaim?
> +__bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(bpf_memcontrol_kfuncs)
> @@ -172,6 +270,9 @@ BTF_ID_FLAGS(func, bpf_mem_cgroup_usage)
> BTF_ID_FLAGS(func, bpf_mem_cgroup_page_state)
> BTF_ID_FLAGS(func, bpf_mem_cgroup_flush_stats, KF_SLEEPABLE)
>
> +BTF_ID_FLAGS(func, bpf_proactive_reclaim, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_proactive_reclaim_swappiness, KF_SLEEPABLE)
> +
> BTF_KFUNCS_END(bpf_memcontrol_kfuncs)
>
> static const struct btf_kfunc_id_set bpf_memcontrol_kfunc_set = {
---
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/32224943030
