* Re: [PATCH] mm: memcontrol: raise MEMCG_MAX for charges that fail without reclaiming
2026-08-27 23:31 [PATCH] mm: memcontrol: raise MEMCG_MAX for charges that fail without reclaiming Joe Damato
@ 2026-08-28 0:13 ` Shakeel Butt
0 siblings, 0 replies; 2+ messages in thread
From: Shakeel Butt @ 2026-08-28 0:13 UTC (permalink / raw)
To: Joe Damato
Cc: linux-kernel, Johannes Weiner, Michal Hocko, Roman Gushchin,
Muchun Song, Andrew Morton, cgroups, linux-mm, bpf
On Thu, Aug 27, 2026 at 04:31:18PM -0700, Joe Damato wrote:
> Charges that exceed memory.max and return through the nomem label can
> raise no event and simply return -ENOMEM.
>
> A non-blocking charge can hit the limit, get rejected, but is not
> visible in memory.events.
>
> This was noticed in a production setting where bpf_mem_alloc() attempted
> to refill its per-cpu freelists, which triggered a non-blocking charge
> while at the limit.
>
> Move the event so that it is raised as soon as the charge is known not
> to fit.
>
> Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Joe Damato <joe@dama.to>
> ---
> mm/memcontrol.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1271d390b617..3904fe9a7b2e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2683,6 +2683,11 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
> goto retry;
> }
>
> + if (!raised_max_event) {
> + __memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
> + raised_max_event = true;
> + }
> +
> /*
> * Prevent unbounded recursion when reclaim operations need to
> * allocate memory. This might exceed the limits temporarily,
> @@ -2711,9 +2716,6 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
> mm_flags_test(MMF_OOM_SKIP, current->signal->oom_mm))
> goto nomem;
>
> - __memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
> - raised_max_event = true;
I just noticed that this will change the current behavior where we keep
increasing MAX counter while charge request loops through reclaim and retry.
Though we have not documented that behavior, so not sure if it is worth
preserving. It does help identify cases where a request keep looping in the
charge/reclaim/retry path.
Let's see what others say. Keeping the behavior should not be that hard if we
decide to keep it. Something like below (untested):
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 1271d390b617..6bfa4ad30b24 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2656,10 +2656,11 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
bool raised_max_event = false;
unsigned long pflags;
bool allow_spinning = gfpflags_allow_spinning(gfp_mask);
+ int ret = 0;
retry:
if (consume_stock(memcg, nr_pages))
- return 0;
+ return ret;
if (!allow_spinning)
/* Avoid the refill and flush of the older stock */
@@ -2770,16 +2771,11 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
* put the burden of reclaim on regular allocation requests
* and let these go through as privileged allocations.
*/
- if (!(gfp_mask & (__GFP_NOFAIL | __GFP_HIGH)))
- return -ENOMEM;
+ if (!(gfp_mask & (__GFP_NOFAIL | __GFP_HIGH))) {
+ ret = -ENOMEM;
+ goto out;
+ }
force:
- /*
- * If the allocation has to be enforced, don't forget to raise
- * a MEMCG_MAX event.
- */
- if (!raised_max_event)
- __memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
-
/*
* The allocation either can't fail or will lead to more memory
* being freed very soon. Allow memory usage go over the limit
@@ -2789,7 +2785,15 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
if (do_memsw_account())
page_counter_charge(&memcg->memsw, nr_pages);
- return 0;
+out:
+ /*
+ * Don't forget to raise a MEMCG_MAX event for forced or rejected requests.
+ */
+ if (!raised_max_event)
+ __memcg_memory_event(mem_over_limit, MEMCG_MAX, allow_spinning);
+
+ return ret;
done_restock:
if (batch > nr_pages)
@@ -2848,7 +2852,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
!(current->flags & PF_MEMALLOC) &&
gfpflags_allow_blocking(gfp_mask))
__mem_cgroup_handle_over_high(gfp_mask);
- return 0;
+ return ret;
}
static inline int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
^ permalink raw reply [flat|nested] 2+ messages in thread