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

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 mod=
ified, failing to align its claims with the code changes.

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

--- Patch [9]: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state i=
n get_in_active_mem_bp() ---
commit fee9e9f299e48399ca370c1f9725936302eaeb8c
Author: SJ Park <[email protected]>
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/[email protected]

> 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 =3D global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
>  		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
> -	total =3D active + inactive;
> +	total =3D 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);
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260801173554.9471=
[email protected]?part=3D9