Re: [RFC PATCH v4 1/4] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incomplete Metric Implementation (mTHP & PTE-mapped THP omitted)
- [High] 32-bit Integer Overflow in Hugepage Metric

Pre-existing issues:
- [High] Pre-existing 32-bit Integer Overflows in DAMON metrics
- [High] Pre-existing Division by Zero in damos_get_in_active_mem_bp
--

commit 7873411ecddc79aa5afd0d6d56dfa25345f98454
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning

This commit introduces a new DAMOS quota goal metric to measure the amount
of huge page consumption to total memory consumption ratio. It adds the
DAMOS_QUOTA_HUGEPAGE_MEM_BP metric and its corresponding calculation logic
for scheme auto-tuning.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 7e4b9affc5b06..b001f80681b12 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2795,6 +2795,17 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
>  	return mult_frac(inactive, 10000, total);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the use of mult_frac()
in damos_get_in_active_mem_bp() risk a division by zero?

If a system has no active or inactive pages (which is structurally possible
if user processes are non-existent, completely swapped out, or if all pages
are mlocked into the UNEVICTABLE LRU), the total evaluates to 0:

mm/damon/core.c:damos_get_in_active_mem_bp() {
    ...
    total = active + inactive;
    if (active_ratio)
        return mult_frac(active, 10000, total);
    ...
}

Since mult_frac() divides by the denominator, could this trigger a
division-by-zero kernel panic?

[Severity: High]
This is a pre-existing issue, but can the mult_frac() calculation overflow
on 32-bit architectures?

In both damos_get_in_active_mem_bp() and damos_get_node_mem_bp(), the metrics
evaluate macros like:

    mult_frac(active, 10000, total);
    mult_frac(i.totalram - i.freeram, 10000, i.totalram);

The mult_frac() macro performs an intermediate multiplication of
(x % d) * 10000.

On 32-bit architectures with more than ~1.76 GB of memory, can this
intermediate multiplication exceed UINT_MAX and silently wrap around,
corrupting the memory ratios?

>  
> +static unsigned int damos_hugepage_mem_bp(void)
> +{
> +	unsigned long thp, total;
> +
> +	thp = global_node_page_state(NR_ANON_THPS) +
> +				global_node_page_state(NR_SHMEM_THPS) +
> +				global_node_page_state(NR_FILE_THPS);

[Severity: High]
Are multi-size THPs (mTHPs) and PTE-mapped THPs omitted from this calculation?

The NR_ANON_THPS counter strictly tracks the number of base pages in
PMD-mapped anonymous THPs. It appears to ignore THPs that are mapped via
PTEs (e.g., due to VMA boundary crossing).

Furthermore, modern setups heavily rely on multi-size THPs (such as 64KB
or 128KB), which are tracked separately via per-order mthp_stats.

Will ignoring these lead to an incorrectly reported near-zero huge page
consumption on modern mTHP setups, breaking the auto-tuning behavior?

> +	total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);
> +	return mult_frac(thp, 10000, total);

[Severity: High]
Will this new metric also be impacted by the 32-bit integer overflow issue
mentioned above?

On 32-bit architectures, thp and total are 32-bit unsigned longs.

The intermediate multiplication in the mult_frac() macro evaluates to
(thp % total) * 10000.

If the system has more than UINT_MAX / 10000 memory pages (about 429,496
pages, or ~1.76 GB of RAM), could this intermediate result exceed UINT_MAX
and silently wrap around, leading to an incorrectly low hugepage ratio?

> +}
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611150244.3454699-1-gutierrez.asier@huawei-partners.com?part=1