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

[sashiko-bot@kernel.org]

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?

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