Re: [PATCH] Fix unbounded loop within try_charge_memcg

[Audra Mitchell <audra@redhat.com>]

On Fri, Aug 07, 2026 at 10:06:53AM +0200, Michal Hocko wrote:
> On Thu 06-08-26 11:10:03, Audra Mitchell wrote:
> > Originally nr_retries was actually nr_oom_retries and we used it to track (and
> > limit) the number of times we entered the mem_cgroup_oom path and then attempted
> > a retry. The purpose of nr_retries counter changed with the introduction of
> > 9b1306192d33 ("mm: memcontrol: retry reclaim for oom-disabled and __GFP_NOFAIL
> > charges") so that the oom-disabled and __GFP_NOFAIL charges would also continue
> > to retry within the desired nr_retries threshold. Later d977aa939fca
> > ("mm, memcg: unify reclaim retry limits with page allocator") changed the
> > nr_retries counter from 5 to 16.
> > 
> > As the function has evolved we now have multiple paths that have a goto retry
> > path and we have lost the original purpose of the nr_retries counter, allowing
> > us to take a goto retry path an unbounded number of times.
> > 
> > Fix the unbounded retries by nesting the code in a loop and decrementing the
> > nr_retries counter correctly.
> 
> Are you trying to fix a theoretical problem spotted by the code review
> or is there any actual problem that you are trying to fix?

We have had some customer complaints that performance has slowed to a crawl when
the cgroup memory limit has come close to the maximum limit. In those cases, we
have noticed that each process is spending a large amount of time in the direct
reclaim path acquiring just enough memory for their specific allocation, thus
by-passing the oom condition yet degrading the system's overall performance.
In the global case, direct reclaim is bounded by DEF_PRIORITY, however, a cgroup
will go through the try_charge_memcg path which will call
try_to_free_mem_cgroup_pages->do_try_to_free_pages each time it does a retry (16
times). If we bound the loop in try_charge_memcg, the worst case is 16*12 passes
attempting to reclaim. This patch is meant to address the unbound case, limiting
the loops to 16 attempts at following the direct reclaim path. An argument could
be made to reduce nr_retries as well, but given that the nr_retries has been set
to 16 for sometime, it seemed unlikely such a change would be considered.