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

[SeongJae Park <sj@kernel.org>]

On Wed, 20 May 2026 06:52:52 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The sanity check for region bounds in `damon_verify_ctx` uses an inverted condition, causing it to falsely trigger a kernel warning on all valid regions and fail to catch invalid ones.
> --
> 
> commit 7d803208aef5bc4f95dd9c37657de5b62618e8e6
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: add kdamond_call() debug_sanity check
> 
> This commit introduces a debug sanity check for the DAMON context state
> before handling damon_call_control objects in kdamond_call(). The check
> verifies region boundaries, overlaps, and the total region count under
> CONFIG_DAMON_DEBUG_SANITY.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -3326,6 +3326,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);
> 
> [Severity: High]
> Should this condition be inverted? WARN_ONCE triggers when the condition
> evaluates to true. Since a valid memory region typically has a start address
> strictly less than its end address, this evaluates to true for normal
> regions.
> 
> Could this trigger a spurious warning on the very first valid region
> encountered, and because it uses WARN_ONCE, prevent any actual regressions
> where start > end from being reported?

Nice catch.  start == end is also invalid because we disallow empty regions.  I
will update it to start >= end in the next version.

> 
> > +			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++;
> > +		}
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260520062858.167011-1-sj@kernel.org?part=8


Thanks,
SJ