DAMON development mailing list
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: gutierrez.asier@huawei-partners.com
Cc: damon@lists.linux.dev
Subject: Re: [PATCH v3 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning
Date: Mon, 20 Jul 2026 12:16:57 +0000	[thread overview]
Message-ID: <20260720121658.0AFC01F00AC4@smtp.kernel.org> (raw)
In-Reply-To: <20260720120140.881468-2-gutierrez.asier@huawei-partners.com>

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

New issues:
- [High] Unsigned underflow in `total` bypasses the division-by-zero guard, causing `damos_hugepage_mem_bp` to return 0 instead of the intended safeguard `INT_MAX`.
- [Medium] The new `DAMOS_QUOTA_HUGEPAGE_MEM_BP` metric is missing from the sysfs parser mapping array, rendering the feature completely inaccessible to userspace.
- [Medium] 32-bit integer overflow in `mult_frac()` yields drastically incorrect ratios when the number of huge pages exceeds ~1.7GB.

Pre-existing issues:
- [High] Preexisting division by zero in `damos_get_in_active_mem_bp` when LRU lists are empty.
--

--- Patch [1]: [PATCH v3 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning ---
commit 8d24f093924a5075fd67c8f7808eada2166af60d
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning

Introduce DAMOS_QUOTA_HUGEPAGE_MEM_BP auto tuning and add a new DAMOS
quota goal metric to measure huge page consumption. Add a guard to avoid
division by 0 when vmstat lags.

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

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
[ ... ]
> @@ -167,6 +168,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_MEM_BP,

[Severity: Medium]
Is this feature accessible from userspace?

If a user writes hugepage_mem_bp to the target_metric sysfs file,
target_metric_store() iterates over the damos_sysfs_qgoal_metric_names
array to find a match.

Because DAMOS_QUOTA_HUGEPAGE_MEM_BP is missing from that array in
mm/damon/sysfs-schemes.c, will the kernel always reject the user's input
with -EINVAL?

>  	NR_DAMOS_QUOTA_GOAL_METRICS,
>  };
>  
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3007,6 +3007,24 @@ 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?

In damos_get_in_active_mem_bp(), total is used as the divisor without a zero
check. If all memory is forced onto the unevictable list (e.g. via
mlockall()), or if the system is completely idle with only kernel allocations,
active + inactive can be zero.

If DAMOS_QUOTA_ACTIVE_MEM_BP is used in such a state, would mult_frac()
trigger a hardware divide-by-zero exception and panic the system?

>  }
>  
> +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);
> +
> +	/*
> +	 * vmstat may lag in idle systems and lead to free pages equaling to
> +	 * the total RAM in the system.
> +	 */
> +	if (!total)
> +		return INT_MAX;

[Severity: High]
Can total underflow here?

Because global_zone_page_state(NR_FREE_PAGES) relies on vmstat, which uses
per-CPU caching, the global NR_FREE_PAGES counter can temporarily exceed
totalram_pages() if some CPUs have flushed frees while others have pending
allocations.

If this happens, total (which is unsigned long) underflows to a value near
ULONG_MAX. Could this bypass the !total guard and pass a massive integer
to mult_frac(), which then incorrectly returns 0?

> +	return mult_frac(thp, 10000, total);

[Severity: Medium]
Will this overflow on 32-bit architectures?

mult_frac() evaluates the intermediate multiplication using the types of its
arguments. Since thp is an unsigned long, evaluating thp * 10000 uses 32-bit
arithmetic on 32-bit systems like ARM LPAE.

If the system has more than 429,496 base pages of THP (approximately 1.7GB),
will this silently wrap around and yield drastically incorrect quota goal
ratios?

> +}

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

  reply	other threads:[~2026-07-20 12:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 12:01 [PATCH v3 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning gutierrez.asier
2026-07-20 12:01 ` [PATCH v3 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE " gutierrez.asier
2026-07-20 12:16   ` sashiko-bot [this message]
2026-07-21  3:47     ` SJ Park
2026-07-20 12:01 ` [PATCH v3 2/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric gutierrez.asier
2026-07-20 12:15   ` sashiko-bot
2026-07-21  3:50     ` SJ Park
2026-07-20 12:01 ` [PATCH v3 3/3] Docs/mm/damon/design: Document hugepage_mem_bp target metric gutierrez.asier
2026-07-20 12:17   ` sashiko-bot
2026-07-21  3:52     ` SJ Park
2026-07-21  3:58 ` [PATCH v3 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning SJ Park
2026-07-21 12:58   ` Gutierrez Asier
2026-07-21 14:03     ` SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260720121658.0AFC01F00AC4@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=gutierrez.asier@huawei-partners.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox