Re: [PATCH 08/14] mm/damon/core: add kdamond_call() debug_sanity check

[SeongJae Park <sj@kernel.org>]

On Fri, 22 May 2026 17:55:14 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Calling damon_nr_regions() instead of accessing t->nr_regions directly causes completely redundant O(N) list traversals and duplicate warnings.

No blocker.  Read below for more details.

> --
> 
> commit 97b7c0b9ade3ed697ef1eba39c981f58994bd0bd
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: add kdamond_call() debug_sanity check
> 
> This commit introduces a new debugging check, damon_verify_ctx(), which
> runs under CONFIG_DAMON_DEBUG_SANITY. It is called from kdamond_call() to
> ensure the DAMON context state is valid before proceeding.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index d816679dd7020..00e2997524ece 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3329,6 +3329,37 @@ static void kdamond_usleep(unsigned long usecs)
> >  		usleep_range_idle(usecs, usecs + 1);
> >  }
> >  
> > +#ifdef CONFIG_DAMON_DEBUG_SANITY
> > +static void damon_verify_ctx(struct damon_ctx *c)
> > +{
> > +	struct damon_target *t;
> > +	struct damon_region *r;
> > +
> > +	damon_for_each_target(t, c) {
> > +		struct damon_region *prev_r = NULL;
> > +		unsigned int nr_regions = 0;
> > +
> > +		damon_for_each_region(r, t) {
> > +			WARN_ONCE(r->ar.start >= r->ar.end,
> > +					"region start (%lu) >= end (%lu)\n",
> > +					r->ar.start, r->ar.end);
> > +			WARN_ONCE(prev_r && prev_r->ar.end > r->ar.start,
> > +					"region overlap (%lu > %lu)\n",
> > +					prev_r->ar.end, r->ar.start);
> > +			prev_r = r;
> > +			nr_regions++;
> > +		}
> > +		WARN_ONCE(damon_nr_regions(t) != nr_regions,
> > +				"nr_regions mismatch: %u != %u\n",
> > +				damon_nr_regions(t), nr_regions);
> 
> [Severity: Low]
> Does this code perform unnecessary list traversals and cause duplicate warnings?
> 
> Since damon_nr_regions() internally calls damon_verify_nr_regions() which
> performs its own O(N) list traversal and issues its own WARN_ONCE on mismatch,
> using it here results in traversing the list multiple times.
> 
> It also causes multiple warnings for the exact same mismatch.

Correct.  And therefore the next patch is removing damon_verify_nr_regions().

> 
> Would it be better to compare against t->nr_regions directly to avoid the
> redundant traversals and duplicate warnings?

For the above reason, no.


Thanks,
SJ

[...]