Re: [PATCH] mm: memcontrol: raise MEMCG_MAX for charges that fail without reclaiming

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Aug 27, 2026 at 05:13:44PM -0700, Shakeel Butt wrote:
> 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):
> 

Hi Joe, let's go with the following patch. No need to change the semantics. Also
I think we should Cc stable as getting ENOMEM/allocation-failures without the
corresponding counter i.e. MAX getting increased is clearly a bug and unexpected
to the users and will make debugging/monitoring harder.

> 
> 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,
>  
>