Re: [PATCH 7/8] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()

[SJ Park <sj@kernel.org>]

On Tue, 01 Sep 2026 13:41:08 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Integer overflow in basis point calculation on 32-bit systems for large memory sizes.
> - [High] Incorrect saturation logic for negative global memory utilization triggers false maximum reclaim.
> --
> 
> commit 23b94672ac5d1c09d669ade5732261e82d129c96
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
> 
> This commit handles corner cases where total memory is zero or
> lruvec_page_state returns more used memory than total memory. It returns
> 100% or 0% for used and free memory queries respectively in these scenarios.
> 
> Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ad3657d356fbc..3060edf5e4fa6 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2854,6 +2854,12 @@ static unsigned long damos_get_node_memcg_used_bp(
> >  	mem_cgroup_put(memcg);
> >  
> >  	si_meminfo_node(&i, goal->nid);
> > +	if (!i.totalram || i.totalram < used_pages) {
> > +		if (goal->metric = DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> > +			return 10000;
> > +		else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +			return 0;
> > +	}
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at similar logic in
> damos_get_node_mem_bp():
> 
>     if (!i.totalram || i.totalram < i.freeram) {
>         if (goal->metric = DAMOS_QUOTA_NODE_MEM_USED_BP)
>             return 10000;
>         else    /* DAMOS_QUOTA_NODE_MEM_FREE_BP */
>             return 0;
>     }
> 
> Can this regression trigger unwarranted memory reclaim pressure? If transient
> per-CPU stats drift causes i.freeram to slightly exceed i.totalram, this
> condition evaluates to true.
> 
> If free memory exceeds total memory, it implies used memory is less than 0.
> Should the correct saturation be 0% used and 100% free instead? Returning
> 10000 (100%) for used memory tells DAMON that the empty node is completely
> full, which might cause aggressive reclaim behavior on mostly empty nodes.

No issue.  It is a different perspective.  And DAMOS auto-tuning logic limits
increasing quota arbitrarily.  Increasing quota is capped to only 2x of the
current effective quota.

> 
> >  	if (goal->metric = DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> >  		numerator = used_pages;
> >  	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but does this basis point calculation overflow
> on 32-bit systems for large memory sizes?
> 
> A little further down in damos_get_node_memcg_used_bp() (and similarly in
> damos_get_node_mem_bp()):
> 
>     return mult_frac(numerator, 10000, i.totalram);
> 
> The mult_frac macro evaluates its internal remainder multiplication using
> the type of the numerator. On 32-bit systems, the numerator is a 32-bit
> unsigned long.
> 
> If the numerator is greater than 429,496 (which corresponds to 1.75 GB of
> RAM with 4K pages), the multiplication by 10000 wraps around ULONG_MAX. For
> instance, 429,497 * 10000 modulo 2^32 evaluates to 2704, which then divided
> by total RAM evaluates to near 0%.
> 
> Could this regression lead to incorrect memory quota enforcement for memcgs
> or nodes exceeding this threshold?

Known low priority issue.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901131850.98037-1-sj@kernel.org?part=7
> 


Thanks,
SJ