Re: [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum

[SJ Park <sj@kernel.org>]

On Mon, 06 Jul 2026 14:44:25 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The `probe_hits` array in `struct damon_region` is of type `unsigned char`, causing silent wrap-around if the number of sampling intervals in an aggregation interval (`aggr_samples`) exceeds 255.
> --
> 
> commit d141f3812ddd7e9395e9f362f8ddc5918e7ab555
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/paddr: respect return_max_wsum
> 
> apply_probes() ops implementation in DAMON_PADDR is ignoring
> return_max_wsum.  Respect it.
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 3eba3564a31fc..b85f88a7a38f4 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> 
> [ ... ]
> 
> > @@ -193,9 +193,12 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> >  			}
> >  			if (folio)
> >  				folio_put(folio);
> > +			if (return_max_wsum)
> > +				max_wsum = max(damon_probe_hits_wsum(r, false,
> > +							ctx), max_wsum);
> >  		}
> >  	}
> > -	return 0;
> > +	return max_wsum;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed something about the probe_hits
> array while reviewing this area. Does this code silently wrap around if the
> number of samples in an aggregation interval exceeds 255?
> 
> Looking at the damon_region structure in include/linux/damon.h, probe_hits
> is defined as an unsigned char array:
> 
> include/linux/damon.h:struct damon_region {
>     ...
>     unsigned char probe_hits[DAMON_MAX_PROBES];
>     ...
> };
> 
> In mm/damon/paddr.c:damon_pa_apply_probes(), this value is incremented on
> every sampling interval without bounds checking:
> 
> mm/damon/paddr.c:damon_pa_apply_probes() {
>     ...
>     damon_for_each_probe(p, ctx) {
>         if (damon_pa_filter_pass(pa, folio, p))
>             r->probe_hits[i]++;
>         i++;
>     }
>     ...
> }
> 
> If a user configures the sysfs settings such that the aggregation interval
> is significantly larger than the sample interval (for example, a 1ms
> sample interval and a 1s aggregation interval yields 1000 samples), will
> r->probe_hits[i] wrap around and start over from zero?
> 
> If so, it appears this might break DAMON region merging and monitoring
> statistics by operating on incorrect, wrapped-around numbers.

Known low priority issue.  No blocker.

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


Thanks,
SJ