Re: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The commit message incorrectly identifies the function being modified, failing to align its claims with the code changes.

Pre-existing issues:
- [High] Multiplication overflow in `mult_frac()` and other basis point scaling calculations on 32-bit architectures leads to incorrect memory ratio calculations and scaling.
--

--- Patch [9]: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
commit fee9e9f299e48399ca370c1f9725936302eaeb8c
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: handle extreme memory state in get_in_active_mem_bp()

This patch prevents a divide-by-zero risk in DAMON's active memory ratio
calculations. In extreme environments where active and inactive memory
both amount to zero, the patch clamps the total memory denominator to a
minimum of 1 to avoid a crash.

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

> damon_get_intervals_adaptation_bp() uses the sum of the active and
> inactive memory amount as a denominator.  In an extreme and unlikely
> environment, active and inactive memory might be zero.

[Severity: Medium]
Is the function name referenced in the commit message correct?

The commit message explicitly states that damon_get_intervals_adaptation_bp()
is being modified to fix a divide-by-zero bug, but the patch actually
modifies damos_get_in_active_mem_bp().

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 76764a2056f7c..4944cf2c5afae 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3010,7 +3010,7 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
>  		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
>  	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
>  		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
> -	total = active + inactive;
> +	total = max(active + inactive, 1);
>  	if (active_ratio)
>  		return mult_frac(active, 10000, total);

[Severity: High]
This is a pre-existing issue, but does this calculation overflow on 32-bit
architectures?

In damos_get_in_active_mem_bp(), if active memory exceeds approximately
429,496 pages (~1.75 GB), multiplying the active page count by 10000 inside
mult_frac() will overflow an unsigned long on 32-bit systems before the
division by total. This could result in artificially low active/inactive
memory ratios and break DAMON's adaptive tuning.

Does this same overflow affect damon_feed_loop_next_input()?

That function returns last_input * score_cg / goal. When last_input is
passed as 100,000,000 and score_cg can be up to 10,000, that multiplication
would similarly wrap around 32-bit boundaries.

>  	return mult_frac(inactive, 10000, total);
>  }

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