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

[SJ Park <sj@kernel.org>]

On Thu, 09 Jul 2026 14:16:46 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `damon_pa_apply_probes()` callback ignores the newly added `set_samples` parameter, violating the updated API contract documented in the header.
> - [Low] The use of `&set` in the kernel-doc comment will generate a warning during the documentation build.
> --
> 
> --- Patch [2]: [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address ---
> commit d206ad52f1017ca79add3bb060530dc8e4e317fd
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: ask apply_probes() ops callback to set sampling address
> 
> This patch updates the apply_probes() DAMON ops callback by adding a
> set_samples boolean argument. This allows the callback to optionally handle
> sampling address setup to minimize unnecessary region iteration.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index d8251c83e6e55..3c2a4d673bf62 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 sampling_addr of each region if ``set_samples`` is true.
> 
> [Severity: Low]
> This isn't a bug, but will this use of '&set' in include/linux/damon.h
> generate a kernel-doc build warning? 
> 
> The '&' prefix in kernel-doc is strictly reserved for referencing structures,
> enumerations, typedefs, or unions. Using it to emphasize the verb 'set' is
> invalid syntax.

Good finding.  It is a typo.  I will fix in the next revision.
> 
> [ ... ]
> 
> > 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 damon_pa_apply_probes() in mm/damon/paddr.c need to use the new
> set_samples parameter?
> 
> According to the updated API documentation, this callback should conditionally
> set r->sampling_addr if set_samples is true. However, the parameter appears to
> be ignored in the function body in this commit, which could lead to incorrect
> or stale sampling if a future caller passes true.

Yes, the later patch in this series will do that.

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


Thanks,
SJ