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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect fallback values in `damos_get_node_mem_bp` when `i.freeram` exceeds `i.totalram`.
--

commit a048cc23be81d8e450e9ee2ce22eb0a86b0cb1ba
Author: SJ Park <sj@kernel.org>

mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()

This commit handles extreme corner cases where total memory is zero or
reported used memory exceeds total memory. It updates
damos_get_node_memcg_used_bp() to prevent division by zero or underflows by
clamping the results to 100% or 0% appropriately.

Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 75d71ac09515e..3779a04753061 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2863,6 +2863,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 is a pre-existing issue, but I noticed a similar block of code just
above this in damos_get_node_mem_bp() that appears to handle the fallback
logic incorrectly:

mm/damon/core.c:damos_get_node_mem_bp() {
    ...
    si_meminfo_node(&i, goal->nid);
    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;
    }
    ...
}

When i.freeram exceeds i.totalram (which can happen due to per-CPU stat drift
during large frees or memory hot-remove), this logic returns 10000 (100%) for
DAMOS_QUOTA_NODE_MEM_USED_BP and 0 for DAMOS_QUOTA_NODE_MEM_FREE_BP.

Since the node is effectively completely free, shouldn't it return 0 for used
and 10000 for free?

Could this cause DAMON memory reclaim schemes to trigger aggressively on
nodes that are actually completely free, potentially causing performance
regressions?

>  	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
>  		numerator = used_pages;
>  	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */

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