Re: [RFC PATCH v4 2/4] mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Mismatch between commit message documentation and actual module parameter name for the target PID.
- [Low] Missing Kconfig dependency causes build failure on architectures/configs without huge page leaf support.

Pre-existing issues:
- [High] Potential divide-by-zero kernel panic in `damos_hugepage_mem_bp` when total memory evaluates to zero.
--

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

mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing

This patch introduces the SAMPLE_DAMON_HUGEPAGE module to monitor a task
and collapse hot memory regions into huge pages. It adds the necessary
Kconfig, Makefile, and sample code implementation.

>     SAMPLE_DAMON_HUGEPAGE operates in the virtual memory space, for a
>     specific task. The user is expected to supply the PID of the task
>     that is going to be monitored through the monitored_pid module
>     variable.

[Severity: Medium]
Is this module variable name accurate? The commit message documents that users
should supply the PID via the monitored_pid module variable, but the Kconfig
and C code below actually define and register it as target_pid.

> diff --git a/samples/damon/Kconfig b/samples/damon/Kconfig
> index cbf96fd8a8bf9..512f150aaabba 100644
> --- a/samples/damon/Kconfig
> +++ b/samples/damon/Kconfig
> @@ -40,4 +40,16 @@ config SAMPLE_DAMON_MTIER
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_DAMON_HUGEPAGE
> +	bool "Build DAMON-based collapse of hot regions (SAMPLE_DAMON_HUGEPAGE)"
> +	depends on DAMON && DAMON_VADDR

[Severity: Low]
Does this require an explicit dependency on TRANSPARENT_HUGEPAGE or
PGTABLE_HAS_HUGE_LEAVES?

If huge page leaves are not supported on the target architecture,
HPAGE_PMD_SHIFT evaluates to a BUILD_BUG() expression. Because
SAMPLE_DAMON_HUGEPAGE evaluates this unconditionally below, configuring a
kernel without huge page support will result in an immediate compile-time
failure.

> +	help
> +	  This module monitors a certain PID provided by the user through
> +	  monitored_pid attribute. Hot regions are determined by DAMON-based

[ ... ]

> diff --git a/samples/damon/hugepage.c b/samples/damon/hugepage.c
> new file mode 100644
> index 0000000000000..2c7fd213a8db1
> --- /dev/null
> +++ b/samples/damon/hugepage.c
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0

[ ... ]

> +static int damon_sample_hugepage_start(void)
> +{
> +	int err;
> +	struct damon_target *target;
> +	struct damos *scheme;
> +	struct damos_quota_goal *goal;

[ ... ]

> +	scheme = damon_new_scheme(&(struct damos_access_pattern) {
> +				.min_sz_region = HPAGE_PMD_SIZE,

[Severity: Low]
This is where HPAGE_PMD_SIZE is evaluated unconditionally, which will trigger
the BUILD_BUG() if the Kconfig dependencies are not updated.

[Severity: High]
This isn't a bug introduced by this patch, but can this feature trigger a
divide-by-zero kernel panic?

This problem was introduced by a preceding patch in the same series
('mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning'), and it appears to
remain unfixed at the end of the series.

Looking at damos_hugepage_mem_bp() in mm/damon/core.c:

mm/damon/core.c:damos_hugepage_mem_bp() {
    ...
    total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);
    return mult_frac(thp, 10000, total);
}

Due to the nature of per-CPU counter caches,
global_zone_page_state(NR_FREE_PAGES) can drift. In extreme edge cases,
such as when nearly all managed pages are free, the free pages count can
equal or temporarily exceed totalram_pages(), causing total to evaluate to 0.

Would passing total == 0 to mult_frac() trigger a division-by-zero
exception?

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