Re: [PATCH v3 2/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric

On Mon, 20 Jul 2026 12:15:38 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Integer underflow when computing 'total' in `damos_hugepage_mem_bp` causes DAMON to become maximally aggressive on highly idle systems.
> - [Medium] The new `hugepage_mem_bp` sysfs metric is missing from the DAMON sysfs selftest script.
> - [Low] Missing documentation for the `hugepage_mem_bp` metric in the DAMON design guide.
> --
> 
> --- Patch [2]: [PATCH v3 2/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric ---
> commit 304fd61bee6da0356569e9c8297451d15a43e10f
> Author: Asier Gutierrez <[email protected]>
> 
> mm/damon/sysfs: support hugepage_mem_bp quota goal metric
> 
> This commit exposes the new DAMOS_QUOTA_HUGEPAGE_MEM_BP autotune policy
> metric through the DAMON sysfs interface. It adds "hugepage_mem_bp" to
> the sysfs-schemes interface.
> 
> > 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: High]
> While this patch exposes the new metric, is there a potential integer
> underflow in the underlying implementation of damos_hugepage_mem_bp() in
> mm/damon/core.c?
> 
> Due to per-CPU vmstat counter drift, pending positive deltas from freed
> memory can cause the global NR_FREE_PAGES count to temporarily exceed the
> fixed totalram_pages(). In damos_hugepage_mem_bp(), total is computed as:
> 
>     total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);
> 
> On a highly idle system, this could cause the unsigned subtraction to
> underflow, resulting in a value near ULONG_MAX. The subsequent !total check
> would fail, and mult_frac(thp, 10000, total) would evaluate to 0 due to the
> enormous denominator.
> 
> Would this score of 0 lead damon_feed_loop_next_input() to perceive a maximal
> divergence from the goal, forcing the scheme to maximize its quota
> aggressiveness instead of minimizing it?

As I replied to the same report, the user impact is trivial.  No blocker.

> 
> [Severity: Medium]
> Should tools/testing/selftests/damon/sysfs.sh be updated to include the new
> metric?
> 
> The test_goal() function validates writes to target_metric for all existing
> metrics:
> 
>     ensure_write_succ "$fpath" "active_mem_bp" "valid input"
>     ensure_write_succ "$fpath" "inactive_mem_bp" "valid input"
>     ensure_write_succ "$fpath" "node_eligible_mem_bp" "valid input"
> 
> Could hugepage_mem_bp be added to this sequence to ensure the sysfs interface
> is regression tested?

Definitely better to do this.  No blocker, though.  I will do this unless
someone steps up earlier.

> 
> [Severity: Low]
> Does this new metric need to be documented in
> Documentation/mm/damon/design.rst?
> 
> The documentation lists all available target_metric values, such as
> inactive_mem_bp and node_eligible_mem_bp, but it appears hugepage_mem_bp
> is missing.

No blocker.  The next patch of this series does that.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
> 


Thanks,
SJ