Re: [PATCH 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address

[SJ Park <sj@kernel.org>]

On Fri, 10 Jul 2026 13:56:08 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `damon_pa_apply_probes` callback fails to implement the newly added `set_samples` parameter, violating the updated API operations contract.
> --
> 
> --- Patch [2]: [PATCH 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address ---
> commit 2da9028945951622496aef07a601ae5bfba7972a
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: ask apply_probes() ops callback to set sampling address
> 
> This commit updates the DAMON ops callback apply_probes() to accept a
> set_samples boolean parameter. The parameter allows the callback to setup
> the sampling address per region to minimize unnecessary regions iteration.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index d8251c83e6e55..aee28b05acc6c 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> [ ... ]
> > @@ -647,7 +647,8 @@ enum damon_ops_id {
> >   * It should also return max number of observed accesses that made as a result
> >   * of its update.  The value will be used for regions adjustment threshold.
> >   * @apply_probes should apply the data attribute probes to each region and
> > - * accordingly update the probe hits counter of the region.
> > + * accordingly update the probe hits counter of the region.  It should also
> > + * set &damon_region->sampling_addr of each region if ``set_samples`` is true.
> >   * @get_scheme_score should return the priority score of a region for a scheme
> >   * as an integer in [0, &DAMOS_MAX_SCORE].
> >   * @apply_scheme is called from @kdamond when a region for user provided
> 
> [ ... ]
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 6172464981730..b13bf7c6eade3 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> [ ... ]
> > @@ -166,7 +166,7 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
> >  	return pass;
> >  }
> >  
> > -static void damon_pa_apply_probes(struct damon_ctx *ctx)
> > +static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> >  {
> >  	struct damon_target *t;
> >  	struct damon_region *r;
> 
> [Severity: Medium]
> Will callers passing set_samples=true end up using stale sampling addresses?
> 
> The documentation in include/linux/damon.h states that apply_probes should
> set &damon_region->sampling_addr if set_samples is true.
> 
> However, the updated damon_pa_apply_probes() does not seem to handle the
> new parameter:
> 
> mm/damon/paddr.c:damon_pa_apply_probes() {
>     ...
> 		damon_for_each_region(r, t) {
> 			int i = 0;
> 			phys_addr_t pa;
> 			struct folio *folio;
> 
> 			pa = damon_pa_phys_addr(r->sampling_addr,
> 					ctx->addr_unit);
>     ...
> }
> 
> It reads the existing r->sampling_addr without generating a new random address
> when set_samples is true.

No issue since no caller sets set_samples true.

And a later patch in this series will respect the argument before a caller sets
it true.

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


Thanks,
SJ