DAMON development mailing list
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [PATCH 03/11] mm/damon/tests/core-kunit: test damon_rand()
Date: Tue, 30 Jun 2026 14:25:02 +0000	[thread overview]
Message-ID: <20260630142503.7100A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260630141726.92246-4-sj@kernel.org>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The lower bound check `KUNIT_EXPECT_GE(test, rnd, 0)` is a tautology and fails to test the lower bound offset logic.
- [Low] The test completely misses the 64-bit slow path of `damon_rand()`.
--

commit 3514b0c5edd38e0cdd6bf2d3e8b3ba10d7e617d2
Author: SJ Park <sj@kernel.org>

mm/damon/tests/core-kunit: test damon_rand()

This commit introduces a kunit test for the damon_rand() PRNG to ensure that
the generated output respects the expected bounds.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index a00168730445d..0ec7d14d354e7 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h

[ ... ]

> +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 fallback path in damon_rand()?

Looking at the implementation of damon_rand(), it contains specific logic for
spans exceeding U32_MAX. By hardcoding a span of 10, the test exclusively
exercises the 32-bit fast path and leaves the 64-bit arithmetic
(mul_u64_u64_shr) completely untested.

Could we add cases that test spans larger than U32_MAX?

> +
> +		KUNIT_EXPECT_GE(test, rnd, 0);

[Severity: Medium]
Is this comparison inherently tautological?

Since rnd is an unsigned long, rnd >= 0 is always true, which renders this
assertion ineffective and might trigger compiler warnings for tautological
comparisons.

Additionally, because the test uses 0 for the lower bound, does it properly
verify that damon_rand() correctly applies the lower bound offset to its
output? If the implementation failed to add the lower bound, this test would
not catch it.

> +		KUNIT_EXPECT_LE(test, rnd, 9);
> +	}
> +}

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

  reply	other threads:[~2026-06-30 14:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 14:17 [PATCH 00/11] mm/damon: update, optimize, and clean up doc, tests, and code SJ Park
2026-06-30 14:17 ` [PATCH 01/11] Docs/mm/damon/design: update for DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP SJ Park
2026-06-30 14:17 ` [PATCH 02/11] Docs/ABI/damon: document probe files SJ Park
2026-06-30 14:17 ` [PATCH 03/11] mm/damon/tests/core-kunit: test damon_rand() SJ Park
2026-06-30 14:25   ` sashiko-bot [this message]
2026-06-30 14:54     ` SJ Park
2026-06-30 14:17 ` [PATCH 04/11] selftests/damon/sysfs.sh: test multiple probe dirs creation SJ Park
2026-06-30 14:17 ` [PATCH 05/11] selftests/damon/sysfs.sh: test {core,ops}_filters/ directories SJ Park
2026-06-30 14:17 ` [PATCH 06/11] selftests/damon/sysfs.sh: test dests dir SJ Park
2026-06-30 14:17 ` [PATCH 07/11] selftests/damon/sysfs.sh: test all files in quota goal dir SJ Park
2026-06-30 14:17 ` [PATCH 08/11] mm/damon/core: reduce range setup in damon_commit_target_regions() SJ Park
2026-06-30 14:17 ` [PATCH 09/11] mm/damon/sysfs: split probe setup function out SJ Park
2026-06-30 14:17 ` [PATCH 10/11] mm/damon/sysfs: split out filters setup function SJ Park
2026-06-30 14:17 ` [PATCH 11/11] mm/damon/sysfs: fix typos in probe_{add,rm}_dirs: s/attr/probe/ SJ Park
2026-06-30 14:57 ` [PATCH 00/11] mm/damon: update, optimize, and clean up doc, tests, and code SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260630142503.7100A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox