Re: [RFC PATCH v1.3 03/11] mm/damon/tests/core-kunit: test damon_rand()

[SeongJae Park <sj@kernel.org>]

On Fri, 26 Jun 2026 00:25:19 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Incomplete test coverage: the test misses the 64-bit execution branch of damon_rand().
> --
> 
> commit e5514cf11386fbde6faace5ed2dc4b93fedc4bf8
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/tests/core-kunit: test damon_rand()
> 
> This commit adds a KUnit test for the recently introduced lockless PRNG in
> damon_rand(). It validates that the generated numbers stay within the
> requested bounds.
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 1cfb8c176b873..282670b0fa908 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> 
> [ ... ]
> 
> > @@ -1460,6 +1460,20 @@ static void damon_test_is_last_region(struct kunit *test)
> >  	damon_free_target(t);
> >  }
> >  
> > +static void damon_test_rand(struct kunit *test)
> > +{
> > +	struct damon_ctx ctx;
> > +	int i;
> > +
> > +	prandom_seed_state(&ctx.rnd_state, get_random_u64());
> > +	for (i = 0; i < 10000; i++) {
> > +		unsigned long rnd = damon_rand(&ctx, 0, 10);
> 
> [Severity: Low]
> Does this test miss the 64-bit execution branch of damon_rand()?
> 
> The damon_rand() function implements two distinct calculation paths: one for
> spans <= U32_MAX and a 64-bit fallback for larger spans using
> mul_u64_u64_shr().
> 
> Because this test hardcodes the bounds 0 and 10, the span evaluates to
> <= U32_MAX. This guarantees that only the 32-bit path is executed, leaving
> the 64-bit logic entirely untested on 64-bit architectures.
> 
> Would it be helpful to add another iteration or separate test case with a
> span greater than U32_MAX to ensure full coverage of damon_rand()?

I agree testing it together could be a good idea.  But not necessarily a
blocker of this patch.  I will consider adding more test cases including that
in future.


Thanks,
SJ

[...]