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
next prev parent reply other threads:[~2026-07-20 12:17 UTC|newest]
Thread overview: 7+ 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-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-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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.