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

[SJ Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] When probe weights are enabled, `damos_apply_target()` aggressively merges adjacent regions regardless of their `probe_hits`, permanently losing monitoring data.
> - [High] Setting probe weights on unsupported operations backends (like DAMON_OPS_VADDR) silently disables all memory monitoring, breaking the DAMON core logic.
> 
> Pre-existing issues:
> - [High] Calling `damon_call()` on an inactive DAMON context leads to an indefinite hang in `wait_for_completion()`, allowing a local DoS via sysfs.
> --
> 
> --- Patch [15]: [RFC PATCH v1.3 15/19] mm/damon/core: implement damon_has_probe_weight() ---
> commit 55c8b58c370f614fee60cf04b8ed7e90eb167302
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: implement damon_has_probe_weight()
> 
> damon monitors only data attributes when probe weights are set.
> However, the function for detecting it is intentionally returning false
> always. Complete the mode detection function so that the mode can really
> start working.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 91964336ce31e..806a67d02a6e9 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 returning true here cause adjacent regions to be aggressively merged in
> damos_apply_target(), resulting in the loss of monitoring data?
> 
> When access_check_disabled becomes true, nr_accesses remains 0. In
> damos_apply_target(), regions with identical nr_accesses and age are merged
> to repair quota-induced splits:
> 
> mm/damon/core.c:damos_apply_target() {
>     ...
>     prev_r->ar.end = r->ar.end;
>     damon_destroy_region(r, t);
>     ...
> }
> 
> Since damos_apply_target() unconditionally merges these regions without
> combining the probe_hits arrays, does this permanently destroy the
> fine-grained probe_hits data?

No issue.  Sane users shouldn't run DAMOS in attrs only monitoring mode.

> 
> [Severity: High]
> Can returning true here also silently disable memory monitoring if the
> operations backend does not support probes?
> 
> If a backend like vaddr or fvaddr is used, damon_valid_probe_params() does
> not verify if ctx->ops.apply_probes is implemented before allowing probe
> weights.
> 
> When this function returns true, standard access checks are bypassed:
> 
> 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);
> }
> 
> Because ctx->ops.apply_probes is null, neither standard accesses nor probe
> accesses are evaluated. Will the kdamond worker thread spin without
> recording any memory activity?

Intended behavior.

> 
> > +	}
> >  	return false;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but is there a risk of an indefinite hang when
> calling damon_call() from sysfs?
> 
> Looking at the sysfs store functions for reclaim and lru_sort, they invoke
> damon_call() without verifying if the kdamond thread is actively running:
> 
> mm/damon/reclaim.c:damon_reclaim_commit_inputs_store() {
>     ...
>     if (!ctx)
>         return -EINVAL;
> 
>     err = damon_call(ctx, &control);
>     ...
> }
> 
> If commit_inputs is written to while the module is disabled, damon_call()
> will enqueue the request and block indefinitely on wait_for_completion()
> because there is no kdamond thread to process the queue. Could we check
> damon_is_running() before calling damon_call()?

No.  call_controls_obsolete avoids this problem.

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


Thanks,
SJ