Re: [PATCH 08/17] mm/damon/paddr: support damon_prep

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `damon_pa_prep_probes` applies probe preparation actions without checking probe filters, causing unintended page state modifications and redundant folio lookups.
--

commit 05f1236264960a0054ba7741a1b9b7ae78666ee1
Author: SJ Park <sj@kernel.org>

mm/damon/paddr: support damon_prep

This commit implements the prep_probes() callback for damon_operations. It
adds support for the DAMON_PREP_SET_PGIDLE preparation action by unsetting
page table accessed bits and setting the PG_idle flag via damon_pa_mkold().

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 5b307dcbb2a50..8fb71183b76c0 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c

[ ... ]

> @@ -105,6 +105,40 @@ static unsigned int damon_pa_check_accesses(struct damon_ctx *ctx)
>  	return max_nr_accesses;
>  }
>  
> +static void damon_pa_prep_probes_region(struct damon_region *r,
> +		struct damon_probe *probe, struct damon_ctx *ctx)
> +{
> +	struct damon_prep *p;
> +
> +	damon_for_each_prep(p, probe) {
> +		switch (p->action) {
> +		case DAMON_PREP_SET_PGIDLE:
> +			damon_pa_mkold(damon_pa_phys_addr(r->sampling_addr,
> +						ctx->addr_unit));

[Severity: Medium]
Will this unconditionally mutate the page state for every prep action,
regardless of whether the page passes the probe's filters?

Because damon_pa_prep_probes_region() doesn't check the filters, it appears
it will clear the accessed bit and set PG_idle on pages that are explicitly
excluded (e.g., when a probe only targets a specific memcg).

Could this disrupt legitimate LRU tracking and reclaim for processes not
targeted by the probe?

> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +static void damon_pa_prep_probes(struct damon_ctx *ctx, bool set_samples)
> +{
> +	struct damon_target *t;
> +	struct damon_region *r;
> +	struct damon_probe *p;
> +
> +	damon_for_each_target(t, ctx) {
> +		damon_for_each_region(r, t) {
> +			if (set_samples)
> +				r->sampling_addr = damon_rand(ctx, r->ar.start,
> +						r->ar.end);
> +			damon_for_each_probe(p, ctx)
> +				damon_pa_prep_probes_region(r, p, ctx);

[Severity: Medium]
Since damon_pa_prep_probes_region() is called in a nested loop for each probe,
could this cause redundant work?

If multiple probes use DAMON_PREP_SET_PGIDLE, this will redundantly invoke
damon_pa_mkold() multiple times on the exact same physical address, leading to
unnecessary folio lookups and reference cycles per region.

> +		}
> +	}
> +}

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