Re: [PATCH v1 3/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The new `DAMOS_QUOTA_HUGEPAGE_MEM_BP` metric is exposed via sysfs but lacks required documentation updates.

Pre-existing issues:
- [Critical] Potential divide-by-zero in `damos_hugepage_mem_bp()` and `damos_get_in_active_mem_bp()` due to an unhandled zero denominator.
- [High] 32-bit integer overflow in `damos_hugepage_mem_bp()` and `damos_get_in_active_mem_bp()` when calculating memory ratios.
--

commit be304699fb94d4d73bd979237fb4d7b08b7216f7
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 329cfd0bbe9f3..4b5429a5b4158 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -1267,6 +1267,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: Critical]
This isn't a bug introduced by this patch, but exposing this metric directly
to userspace makes a potential divide-by-zero triggerable by users
configuring DAMON quotas.

In mm/damon/core.c:damos_hugepage_mem_bp(), the denominator is computed as:

    total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);

Due to per-CPU vmstat lag or an idle system, the free pages counter can equal
totalram_pages(), resulting in total being 0. This is then passed to
mult_frac():

    return mult_frac(thp, 10000, total);

This inherently computes thp / total, causing a strict division by zero and a
kernel panic. The same issue exists in damos_get_in_active_mem_bp() where
total = active + inactive, which can be 0 if the LRU lists are empty.

Should a zero-check on total be added before performing the division?

[Severity: High]
This is a pre-existing issue, but exposing this metric also surfaces a 32-bit
integer overflow in mm/damon/core.c:damos_hugepage_mem_bp().

The mult_frac(x, n, d) macro performs the multiplication (x % d) * n. Here x
is thp, which is an unsigned long.

On 32-bit systems, if thp exceeds ~429,496 pages (around 1.75 GB),
(thp % total) * 10000 will silently overflow the 32-bit unsigned long type.

This wrap-around yields an erroneously low ratio instead of the correct value.
The same overflow applies to damos_get_in_active_mem_bp() if the active or
inactive pages exceed 1.75 GB.

Could this be fixed by casting the first argument to u64 to prevent the
overflow?

[Severity: Low]
Does this patch also require an update to the authoritative DAMON sysfs
documentation?

Documentation/mm/damon/design.rst explicitly lists all valid string values
for the target_metric sysfs file, but does not include hugepage_mem_bp.
Without documenting the new metric, it may remain undiscoverable for users.

>  };
>  
>  static ssize_t target_metric_show(struct kobject *kobj,

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