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

[sashiko-bot@kernel.org]

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

New issues:
- [High] A potential division by zero in damos_hugepage_mem_bp() can cause a kernel panic if the total non-free RAM pages evaluate to 0.
- [Medium] The new DAMOS_QUOTA_HUGEPAGE metric is inaccessible from userspace because it was not added to the sysfs mapping array damos_sysfs_qgoal_metric_names.
- [Medium] The implementation calculates the ratio of all THP memory to total used memory, directly contradicting the commit message's claim that it measures huge page consumption relative to total anonymous memory.
- [Medium] The calculation in mult_frac() can silently overflow a 32-bit unsigned long, resulting in arbitrarily small, incorrect quota targets.

Pre-existing issues:
- [High] The pre-existing DAMOS_QUOTA_ACTIVE_MEM_BP and INACTIVE_MEM_BP metric computation can trigger a kernel panic via division by zero.
--

commit 1f37bbf7381e392f999f133e0444c2faad754df9
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, DAMOS_QUOTA_HUGEPAGE,
which is intended to measure the ratio of huge page consumption against
total anonymous memory.

> Introduce DAMOS_QUOTA_HUGEPAGE auto tuning Add a
> new DAMOS quota goal metric to measure the amount of huge
> page consumption to total anonymous memory consumption
> ratio.

[Severity: Medium]
Does this description match the implementation?

The commit message states this measures huge page consumption against total
anonymous memory, but the code in damos_hugepage_mem_bp() calculates the sum
of all transparent huge pages relative to total used memory.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 6f7edb3590ef9..d840b0bc0b953 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
[ ... ]
> @@ -176,6 +176,7 @@ enum damos_quota_goal_metric {
>  	DAMOS_QUOTA_ACTIVE_MEM_BP,
>  	DAMOS_QUOTA_INACTIVE_MEM_BP,
>  	DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP,
> +	DAMOS_QUOTA_HUGEPAGE,

[Severity: Medium]
Does this new metric need to be added to damos_sysfs_qgoal_metric_names[]
in mm/damon/sysfs-schemes.c?

Without adding it to the string mapping array, it appears this metric cannot
be configured from userspace via sysfs.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 265d51ade25bf..71c8699bdd476 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2756,6 +2756,17 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
>  	return mult_frac(inactive, 10000, total);
>  }

[Severity: High]
This is a pre-existing issue, but can this cause a divide by zero kernel
panic?

If there are no active or inactive pages, or if per-CPU vmstat counter drift
evaluates total to 0, mult_frac() will divide by zero.

> +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);
> +	total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);
> +	return mult_frac(thp, 10000, total);

[Severity: High]
Can mult_frac() divide by zero here?

If the system is highly idle or due to per-CPU vmstat counter drift, total
could evaluate to exactly 0.

[Severity: Medium]
Is there a risk of a 32-bit overflow in this calculation?

On 32-bit architectures, mult_frac() preserves types, and computing
thp * 10000 could exceed the maximum unsigned long value if there are
more than ~1.7 GB of THPs, producing an arbitrarily small quota target.

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