Re: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight()

[SJ Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Enabling probe weights for DAMON operations that do not implement `apply_probes` (e.g., VADDR and FVADDR) silently disables memory monitoring and region aggregation.
> --
> 
> --- Patch [15]: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() ---
> commit 13bb6c9724d49615512e0e4f353c5d02210d2c11
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: implement damon_has_probe_weight()
> 
> This commit completes the implementation of damon_has_probe_weights()
> to return true if any probe has a non-zero weight. This allows DAMON
> to detect probe weights and use them for monitoring.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 12c66edecd783..6661d4bfab5b9 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -211,6 +211,12 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
> >  
> >  static bool damon_has_probe_weights(struct damon_ctx *c)
> >  {
> > +	struct damon_probe *p;
> > +
> > +	damon_for_each_probe(p, c) {
> > +		if (p->weight)
> > +			return true;
> > +	}
> 
> [Severity: High]
> Does this silently break monitoring for operation sets that do not implement
> apply_probes?
> 
> When this function returns true, access_check_disabled evaluates to true in
> kdamond_fn():
> 
> mm/damon/core.c:kdamond_fn() {
>     ...
>     bool access_check_disabled = damon_has_probe_weights(ctx);
>     ...
>     if (!access_check_disabled && ctx->ops.prepare_access_checks)
>         ctx->ops.prepare_access_checks(ctx);
>     ...
>     if (!access_check_disabled && ctx->ops.check_accesses)
>         max_merge_score = ctx->ops.check_accesses(ctx);
> 
>     if (ctx->ops.apply_probes) {
>         ...
>     }
>     ...
> }
> 
> If a user configures probe weights for operation sets that lack apply_probes,
> all access checks are skipped. Since apply_probes is NULL, no memory access
> information is gathered at all. This causes regions to endlessly report 0
> accesses and incorrectly merge up to the maximum limit.
> 
> Should __damon_commit_ctx() or damon_valid_probe_params() validate that
> ctx->ops.apply_probes is non-NULL when probe weights are used, returning
> -EINVAL to prevent this failure?

This is an intentional design.  Users should correctly set DAMON.

> 
> >  	return false;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=15


Thanks,
SJ