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

[SJ Park <sj@kernel.org>]

On Fri, 10 Jul 2026 00:33:27 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `set_samples` parameter in `damon_pa_apply_probes()` is ignored, failing to set `r->sampling_addr` when requested.
> --
> 
> --- Patch [2]: [RFC PATCH v1.3 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address ---
> commit 5855b0123b327412c4065a90363e8586c4bafda9
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: ask apply_probes() ops callback to set sampling address
> 
> This commit updates the protocol for the DAMON apply_probes callback to
> accept a boolean set_samples argument. It updates the callback signatures
> so it can handle sampling address setup in the future to minimize region
> 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
> 
> [ ... ]
> 
> > 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]
> Does this implementation actually use the new set_samples parameter?
> 
> The updated documentation in include/linux/damon.h states that apply_probes
> should set the region's sampling_addr when set_samples is true, but it
> appears this parameter is ignored in damon_pa_apply_probes() in
> mm/damon/paddr.c.
> 
> While it is currently called with false from kdamond_fn(), if future code
> passes true as the docstring suggests is possible, could this result in
> using a stale or uninitialized address because the API contract is not
> fulfilled here?

Intentional.  The later patch of this series implement that.

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


Thanks,
SJ