Re: [PATCH v4 3/3] Docs/mm/damon/design: Document hugepage_mem_bp target metric

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Integer overflow in `mult_frac` on 32-bit systems for memory exceeding ~1.75 GB of huge pages, resulting in incorrect DAMOS quota auto-tuning.
- [Medium] The newly introduced DAMOS quota goal metric 'hugepage_mem_bp' is not tested in the DAMON sysfs selftests.
--

commit 59f2c739bce03363ce501965d6708d807908af18
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning

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

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..9c0e42a1d2cfc 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3007,6 +3007,22 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
>  	return mult_frac(inactive, 10000, total);
>  }
>  
> +static unsigned int damos_hugepage_mem_bp(void)
> +{
> +	unsigned long thp, total_pages, free_pages;
> +
> +	total_pages = totalram_pages();
> +	free_pages = global_zone_page_state(NR_FREE_PAGES);
> +
> +	if (total_pages <= free_pages)
> +		return 10000;
> +
> +	thp = global_node_page_state(NR_ANON_THPS) +
> +				global_node_page_state(NR_SHMEM_THPS) +
> +				global_node_page_state(NR_FILE_THPS);
> +	return mult_frac(thp, 10000, total_pages - free_pages);

[Severity: High]
Does this calculation overflow on 32-bit systems?

Since thp is an unsigned long, on a 32-bit architecture it is a 32-bit
integer. If the system has more than ~1.75 GB of huge pages (about 429,496
pages), the internal multiplication thp * 10000 will exceed 4,294,967,295
and wrap around.

This leads to an incorrect value being returned from mult_frac() and results
in incorrect DAMOS auto-tuning.

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

mm/damon/sysfs: support hugepage_mem_bp quota goal metric

DAMOS has a new autotune policy metric: DAMOS_QUOTA_HUGEPAGE_MEM_BP.
This patch exposes DAMOS_QUOTA_HUGEPAGE_MEM_BP through sysfs.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 32f495a96b17a..d9b81d7b5910e 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
[ ... ]
> @@ -1269,6 +1269,10 @@ struct damos_sysfs_qgoal_metric_name damos_sysfs_qgoal_metric_names[] = {
>  		.metric = DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP,
>  		.name = "node_eligible_mem_bp",
>  	},
> +	{
> +		.metric = DAMOS_QUOTA_HUGEPAGE_MEM_BP,
> +		.name = "hugepage_mem_bp",
> +	},
>  };

[Severity: Medium]
Should there be an accompanying update to the selftests for this new sysfs
metric?

Currently, the hugepage_mem_bp metric is missing from the tests. For
example, in tools/testing/selftests/damon/sysfs.sh:test_goal(), the new
metric is not verified:

tools/testing/selftests/damon/sysfs.sh:test_goal() {
    ...
    ensure_write_succ "$fpath" "node_eligible_mem_bp" "valid input"
    ensure_write_fail "$fpath" "foo" "invalid input"
    ...
}

Running the sysfs.sh selftest will silently skip testing the new metric
because it wasn't added to the hardcoded list of valid inputs.

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