Re: [RFC PATCH v1.1 8/9] 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] This problem wasn't introduced by this patch, but damos_get_node_mem_bp() contains a severe logic bug and underflow when free memory exceeds total memory (i.totalram < i.freeram).
--

--- Patch [8]: [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
commit 0fb31bc202c766c30f5d776d57b867135fba4aa6
Author: SJ Park <sj@kernel.org>
mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()

This commit addresses division by zero and underflow issues in
damos_get_node_memcg_used_bp() by handling cases where total memory is zero or
used memory exceeds total memory. It returns 100% and 0% for these respective
edge cases to prevent anomalous behavior.

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index e3f3ee75a3d33..67ad1f07c29a4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2862,10 +2862,16 @@ static unsigned long damos_get_node_memcg_used_bp(
>  	mem_cgroup_put(memcg);
>  
>  	si_meminfo_node(&i, goal->nid);
> -	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> +	if (!i.totalram)
> +		return 10000;
> +	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) {
>  		numerator = used_pages;
> -	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> +	} else {
> +		/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> +		if (i.totalram < used_pages)
> +			return 0;

[Severity: High]
This isn't a bug introduced by this patch, but does the adjacent
damos_get_node_mem_bp() function contain a logic bug when free memory exceeds
total memory?

If i.totalram < i.freeram, it appears that for DAMOS_QUOTA_NODE_MEM_USED_BP,
i.totalram - i.freeram will unconditionally underflow, falsely throttling or
activating schemes. For DAMOS_QUOTA_NODE_MEM_FREE_BP, returning 0 means it
reports 0% free memory when the node is actually >100% free, which could cause
schemes waiting for low free memory to trigger aggressively.

mm/damon/core.c:damos_get_node_mem_bp() {
	...
	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP) {
		numerator = i.totalram - i.freeram;
	} else {
		/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
		if (i.totalram < i.freeram)
			return 0;
		numerator = i.freeram;
	}
	...
}

Should this pre-existing issue in damos_get_node_mem_bp() be addressed as well,
perhaps by returning 0 for USED_BP and 10000 for FREE_BP when
i.totalram < i.freeram?

>  		numerator = i.totalram - used_pages;
> +	}
>  	return mult_frac(numerator, 10000, i.totalram);
>  }

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