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 v1 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning
Date: Tue, 16 Jun 2026 15:20:16 +0000	[thread overview]
Message-ID: <20260616152017.29E1C1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260616150316.580819-2-gutierrez.asier@huawei-partners.com>

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

New issues:
- [High] The `mult_frac(thp, 10000, total)` macro call silently overflows `unsigned long` on 32-bit architectures, yielding incorrect DAMOS quota metrics.

Pre-existing issues:
- [Critical] The `damos_get_in_active_mem_bp()` function can trigger a divide-by-zero exception if there are no active and inactive LRU pages.
--

commit 812a9d3523a15be0a409c86d1076f2098e299808
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning

This commit introduces a new DAMOS quota goal metric to measure the amount
of huge page consumption to total memory consumption ratio.

[ ... ]

> 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: Critical]
This is a pre-existing issue, but can this divide by zero if there are no
active and inactive LRU pages?

If a system has no active or inactive anonymous/file LRU pages (for example,
immediately after early boot before userspace populates the LRU), total
evaluates to exactly 0. The mult_frac macro performs division by total,
which would trigger a divide-by-zero hardware exception.

>  }
>  
> +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 calculation silently overflow unsigned long on 32-bit
architectures?

The mult_frac(x, n, d) macro computes (x / d) * n + (x % d) * n / d. Because
thp is always less than or equal to total, thp / total evaluates to 0,
leaving (thp * 10000) / total.

Both thp and 10000 evaluate as 32-bit unsigned long on 32-bit systems, so
the intermediate multiplication thp * 10000 will wrap around if thp exceeds
roughly 1.75 GB (429,496 pages). This would cause the metric to under-report
hugepage usage.

> +}

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

  reply	other threads:[~2026-06-16 15:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 15:03 [PATCH v1 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning gutierrez.asier
2026-06-16 15:03 ` [PATCH v1 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE " gutierrez.asier
2026-06-16 15:20   ` sashiko-bot [this message]
2026-06-16 19:33     ` Gutierrez Asier
2026-06-17  3:31   ` SeongJae Park
2026-06-16 15:03 ` [PATCH v1 2/3] mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing gutierrez.asier
2026-06-16 15:21   ` sashiko-bot
2026-06-16 19:27     ` Gutierrez Asier
2026-06-17  4:09       ` SeongJae Park
2026-06-17  4:04   ` SeongJae Park
2026-06-16 15:03 ` [PATCH v1 3/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric gutierrez.asier
2026-06-16 15:21   ` sashiko-bot
2026-06-16 19:35     ` Gutierrez Asier
2026-06-17  4:15       ` SeongJae Park
2026-06-17  4:16   ` SeongJae Park
2026-06-17  1:44 ` [PATCH v1 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning SeongJae 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=20260616152017.29E1C1F00A3A@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