DAMON development mailing list
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [PATCH v4 3/3] Docs/mm/damon/design: Document hugepage_mem_bp target metric
Date: Mon, 31 Aug 2026 17:48:54 -0700	[thread overview]
Message-ID: <20260901004855.89809-1-sj@kernel.org> (raw)
In-Reply-To: <20260831183558.F0B4D1F000E9@smtp.kernel.org>

On Mon, 31 Aug 2026 18:35:58 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Integer overflow in `mult_frac` on 32-bit systems for memory exceeding ~1.75 GB of huge pages, resulting in incorrect DAMOS quota auto-tuning.
> - [Medium] The newly introduced DAMOS quota goal metric 'hugepage_mem_bp' is not tested in the DAMON sysfs selftests.
> --
> 
> commit 59f2c739bce03363ce501965d6708d807908af18
> Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> 
> mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE auto tuning
> 
> Introduce DAMOS_QUOTA_HUGEPAGE_MEM_BP auto tuning. Add a new DAMOS quota
> goal metric to measure the amount of huge page consumption to total
> memory consumption ratio.
> 
> Link: https://lore.kernel.org/all/20260715151615.99767-1-sj@kernel.org/
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 644daf5a16560..9c0e42a1d2cfc 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3007,6 +3007,22 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
> >  	return mult_frac(inactive, 10000, total);
> >  }
> >  
> > +static unsigned int damos_hugepage_mem_bp(void)
> > +{
> > +	unsigned long thp, total_pages, free_pages;
> > +
> > +	total_pages = totalram_pages();
> > +	free_pages = global_zone_page_state(NR_FREE_PAGES);
> > +
> > +	if (total_pages <= free_pages)
> > +		return 10000;
> > +
> > +	thp = global_node_page_state(NR_ANON_THPS) +
> > +				global_node_page_state(NR_SHMEM_THPS) +
> > +				global_node_page_state(NR_FILE_THPS);
> > +	return mult_frac(thp, 10000, total_pages - free_pages);
> 
> [Severity: High]
> Does this calculation overflow on 32-bit systems?
> 
> Since thp is an unsigned long, on a 32-bit architecture it is a 32-bit
> integer. If the system has more than ~1.75 GB of huge pages (about 429,496
> pages), the internal multiplication thp * 10000 will exceed 4,294,967,295
> and wrap around.
> 
> This leads to an incorrect value being returned from mult_frac() and results
> in incorrect DAMOS auto-tuning.

Known low priority issue.

> 
> commit a2c5ad81dadcc820e74efd9d73c1f576bbe76457
> Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> 
> mm/damon/sysfs: support hugepage_mem_bp quota goal metric
> 
> DAMOS has a new autotune policy metric: DAMOS_QUOTA_HUGEPAGE_MEM_BP.
> This patch exposes DAMOS_QUOTA_HUGEPAGE_MEM_BP through sysfs.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 32f495a96b17a..d9b81d7b5910e 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> [ ... ]
> > @@ -1269,6 +1269,10 @@ struct damos_sysfs_qgoal_metric_name damos_sysfs_qgoal_metric_names[] = {
> >  		.metric = DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP,
> >  		.name = "node_eligible_mem_bp",
> >  	},
> > +	{
> > +		.metric = DAMOS_QUOTA_HUGEPAGE_MEM_BP,
> > +		.name = "hugepage_mem_bp",
> > +	},
> >  };
> 
> [Severity: Medium]
> Should there be an accompanying update to the selftests for this new sysfs
> metric?
> 
> Currently, the hugepage_mem_bp metric is missing from the tests. For
> example, in tools/testing/selftests/damon/sysfs.sh:test_goal(), the new
> metric is not verified:
> 
> tools/testing/selftests/damon/sysfs.sh:test_goal() {
>     ...
>     ensure_write_succ "$fpath" "node_eligible_mem_bp" "valid input"
>     ensure_write_fail "$fpath" "foo" "invalid input"
>     ...
> }
> 
> Running the sysfs.sh selftest will silently skip testing the new metric
> because it wasn't added to the hardcoded list of valid inputs.

I'm working for it, separately.  No blocker.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260831144732.80910-1-sj@kernel.org?part=3


Thanks,
SJ

  reply	other threads:[~2026-09-01  0:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 14:47 [PATCH v4 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning SJ Park
2026-08-31 14:47 ` [PATCH v4 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE " SJ Park
2026-08-31 18:16   ` sashiko-bot
2026-09-01  0:45     ` SJ Park
2026-09-01  6:58   ` Lian Wang
2026-09-01 14:23     ` SJ Park
2026-09-02  1:56       ` Lian Wang
2026-09-02  4:23         ` SJ Park
2026-09-02 15:03       ` Gutierrez Asier
2026-09-02 15:15         ` SJ Park
2026-08-31 14:47 ` [PATCH v4 2/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric SJ Park
2026-08-31 18:22   ` sashiko-bot
2026-09-01  0:46     ` SJ Park
2026-08-31 14:47 ` [PATCH v4 3/3] Docs/mm/damon/design: Document hugepage_mem_bp target metric SJ Park
2026-08-31 15:45   ` Randy Dunlap
2026-08-31 20:00     ` Randy Dunlap
2026-09-01  0:47       ` SJ Park
2026-09-01  5:20         ` Gutierrez Asier
2026-09-01  5:31           ` SJ Park
2026-08-31 18:35   ` sashiko-bot
2026-09-01  0:48     ` SJ Park [this message]
2026-09-01  0:49 ` [PATCH v4 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning 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=20260901004855.89809-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-bot@kernel.org \
    /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