Re: [RFC PATCH v1.1 05/13] mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions

[SeongJae Park <sj@kernel.org>]

On Sat, 20 Jun 2026 17:37:48 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] If a user configures `sample_interval=0` and `aggr_interval=0`, the time difference calculation inside `damon_nr_accesses_mvsum()` underflows, resulting in a huge garbage value for `nr_accesses`. This corrupts sysfs outputs and erratically influences DAMOS operations.
> --
> 
> commit 2a77ab9c787a549b18af4699f5ffa8d3e4d60ca2
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions
> 
> This commit modifies the DAMON sysfs interface to calculate region
> accesses using damon_nr_accesses_mvsum() rather than a simple division.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 329cfd0bbe9f3..e648b0f809332 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -159,7 +159,7 @@ struct damon_sysfs_scheme_region {
> >  };
> >  
> >  static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
> > -		struct damon_region *region)
> > +		struct damon_region *region, struct damon_ctx *ctx)
> >  {
> >  	struct damon_sysfs_scheme_region *sysfs_region = kmalloc_obj(*sysfs_region);
> >  
> > @@ -167,7 +167,7 @@ static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
> >  		return NULL;
> >  	sysfs_region->kobj = (struct kobject){};
> >  	sysfs_region->ar = region->ar;
> > -	sysfs_region->nr_accesses = region->nr_accesses_bp / 10000;
> > +	sysfs_region->nr_accesses = damon_nr_accesses_mvsum(region, ctx);
> 
> [Severity: High]
> Could this lead to an unsigned integer underflow if a user configures
> both sample_us and aggr_us to 0 via sysfs?
> 
> If a user configures sample_interval=0 and aggr_interval=0, the
> intervals bypass the validation in damon_set_attrs(). Inside kdamond_fn(),
> ctx->next_aggregation_sis is updated by adding 0 and never advances,
> but ctx->passed_sample_intervals continues to increment on every loop
> iteration.
> 
> When damon_nr_accesses_mvsum() is called, it calculates the difference:
> 
> mm/damon/core.c:damon_nr_accesses_mvsum() {
>         ...
>         left_window = ctx->next_aggregation_sis - ctx->passed_sample_intervals;
>         ...
> }
> 
> Because ctx->passed_sample_intervals quickly exceeds
> ctx->next_aggregation_sis, does this subtraction result in an unsigned
> integer underflow?
> 
> If left_window becomes a massive value, it appears it would propagate
> through mult_frac() and damon_mvsum() to yield a garbage value for
> nr_accesses. Since __damos_valid_target() evaluates scheme criteria
> using this value, could this cause DAMON to act on wildly inaccurate
> access scores and disrupt system stability?

Good finding.  I will fix this by making damon_nr_accesses_mvsum() aware of
this case.


Thanks,
SJ

[...]