Re: [RESEND PATCH v2 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 4 potential issue(s) to consider:

New issues:
- [High] Potential division by zero in `damos_hugepage_mem_bp()` when computing the hugepage memory ratio due to an unvalidated denominator.
- [Low] Inaccurate metric name in the commit message.

Pre-existing issues:
- [High] Potential division by zero in `damos_get_in_active_mem_bp()` when calculating the active/inactive memory ratio.
- [High] Potential division by zero in `damos_get_node_mem_bp()` and `damos_get_node_memcg_used_bp()` when a NUMA node's managed pages evaluate to 0.
--

--- Patch [3]: [RESEND PATCH v2 3/3] Docs/mm/damon/design: Document hugepage_mem_bp target metric ---
commit d0083321a3dc33547522a0679c37d9ea234b4e37
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

Docs/mm/damon/design: Document hugepage_mem_bp target metric

This commit documents the hugepage_mem_bp metric exposed by sysfs for
DAMON quota goals. It updates the design documentation to include the new
ratio of huge pages to total used memory.

> Docs/mm/damon/design: Document hugepage_mem_bp target metric
> 
> Document huge_mem_bp metric exposed by sysfs.

[Severity: Low]
This isn't a bug, but is the metric name in the commit message accurate?
The code and the subject line refer to hugepage_mem_bp, but the commit
description refers to huge_mem_bp.

[ ... ]

> 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 is a pre-existing issue, but can this divide by zero?

If the sum of active and inactive LRU pages is zero (e.g., in highly
constrained environments or due to negative per-CPU counter drift), 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);
    return mult_frac(inactive, 10000, total);
}

Passing total to mult_frac() without validating if it is non-zero
can crash the kernel via a division 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]
Does this code risk a division by zero panic?

The calculation unconditionally passes total as the denominator to
mult_frac() without validating it. 

Because global_zone_page_state() reads from atomic per-CPU counters without
strict synchronization, it can temporarily drift. If this per-CPU drift causes
NR_FREE_PAGES to equal totalram_pages(), total will evaluate exactly to 0,
crashing the kernel when the quota goal is evaluated.

> +}

[ ... ]

While reviewing the DAMON quota calculations in mm/damon/core.c, I noticed
some pre-existing regressions in related functions:

[Severity: High]
This isn't a bug introduced by this patch, but are damos_get_node_mem_bp()
and damos_get_node_memcg_used_bp() susceptible to a division by zero?

In both functions, if a NUMA node has N_MEMORY set while all its memory is
reserved (meaning zone_managed_pages is 0), si_meminfo_node() sets i.totalram
to 0.

mm/damon/core.c:damos_get_node_mem_bp() {
    ...
    si_meminfo_node(&i, goal->nid);
    if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
        numerator = i.totalram - i.freeram;
    else
        numerator = i.freeram;
    return mult_frac(numerator, 10000, i.totalram);
}

mm/damon/core.c:damos_get_node_memcg_used_bp() {
    ...
    si_meminfo_node(&i, goal->nid);
    ...
    return mult_frac(numerator, 10000, i.totalram);
}

Passing i.totalram as the denominator to mult_frac() without checking if it
is zero will trigger a division by zero panic. Users configuring DAMOS with
node-specific metrics on a node with zero managed pages can reliably trigger
this.

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