Re: [RFC PATCH 03/16] mm/damon/paddr: set samples in apply_probes() if requested

[SJ Park <sj@kernel.org>]

On Sun, 05 Jul 2026 21:11:47 +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` counter in `struct damon_region` is an 8-bit integer (`unsigned char`), which overflows if the ratio of aggregation interval to sampling interval exceeds 255. When this wrapped value is used by `damon_merge_score()` (introduced in this series via `damon_probe_hits_wsum()`), DAMON incorrectly merges regions, breaking its core tracking logic.
> --
> 
> commit 26a2b31429c08214403a84c01f1b8d381e461d92
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/paddr: set samples in apply_probes() if requested
> 
> The apply_probe() callback implementation in DAMON_PADDR ignores the
> set_samples parameter. This commit updates it to respect the parameter by
> setting the sampling address if requested.
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index b13bf7c6eade3..ff4fcca949660 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -178,6 +178,10 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> >  			phys_addr_t pa;
> >  			struct folio *folio;
> >  
> > +			if (set_samples)
> > +				r->sampling_addr = damon_rand(ctx, r->ar.start,
> > +						r->ar.end);
> > +
> >  			pa = damon_pa_phys_addr(r->sampling_addr,
> >  					ctx->addr_unit);
> >  			folio = damon_get_folio(PHYS_PFN(pa));
> 
> [Severity: High]
> This is a pre-existing issue, but does this code overflow probe_hits[]?
> 
> Looking at include/linux/damon.h, probe_hits is defined as an 8-bit integer:
> 
> struct damon_region {
>     ...
>     unsigned char probe_hits[DAMON_MAX_PROBES];
> 
> Shortly after the sampling address is set in damon_pa_apply_probes(), this
> counter is incremented:
> 
>     damon_for_each_probe(p, ctx) {
>         if (damon_pa_filter_pass(pa, folio, p))
>             r->probe_hits[i]++;
> 
> If a user configures DAMON such that the ratio of aggregation interval to
> sampling interval exceeds 255, this counter will overflow and wrap around to
> zero.
> 
> Since this patch series modifies damon_merge_score() in mm/damon/core.c to
> rely on this counter:
> 
>     if (use_probe_hits)
>         return damon_probe_hits_wsum(r, last, ctx);
> 
> Could this overflow cause highly-accessed regions to be treated as having few
> matches, leading to incorrect region merging and breaking DAMON's core tracking
> logic for filters?

It can overflow, and it is the intended behavior.

Since the core layer parameters validation series is just merged into mm-new,
I'm considering limiting aggregation interval to be less than 256 * sampling
interval using the validation mechanism, though.

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


Thanks,
SJ