From: sashiko-bot@kernel.org
To: "Liew Rui Yan" <aethernet65535@gmail.com>
Cc: damon@lists.linux.dev
Subject: Re: [RESEND PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region
Date: Sat, 19 Sep 2026 08:12:39 +0000 [thread overview]
Message-ID: <20260919081239.8A3D91F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260919080453.628679-1-aethernet65535@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mm: invalid non-power-of-two alignment in damon test
--
commit a99d6a5362b525870f71ce4be82705b420aea7ef
Author: Liew Rui Yan <aethernet65535@gmail.com>
mm/damon/tests/core-kunit: add test for unconditionally skipping the last region
This commit adds a kunit test to ensure the last region is not skipped
before it is processed.
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 5ff0436c58441..84e50f08f417d 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
[ ... ]
> @@ -1929,6 +1929,48 @@ static void damon_test_rand(struct kunit *test)
> }
> }
>
> +static void damon_test_last_region_quota_reset(struct kunit *test)
> +{
> + struct damos s = {0};
> + struct damon_target *t;
> + struct damon_region *r1, *r2;
> + unsigned long min_region_sz = 10;
[Severity: Low]
Is it safe to initialize min_region_sz to a non-power-of-two value?
The ALIGN_DOWN() macro inside damos_skip_charged_region() relies on bitwise
masking and mathematically guarantees correct alignment arithmetic only when
the alignment boundary is a strict power of two. Passing 10 yields incorrect
values (e.g., ALIGN_DOWN(120, 10) incorrectly evaluates to 112).
> + bool skipped;
> +
> + t = damon_new_target();
[ ... ]
> + s.quota.charge_target_from = t;
> + s.quota.charge_addr_from = r1->ar.end;
> +
> + skipped = damos_skip_charged_region(t, r2, &s, min_region_sz);
[Severity: Low]
Can this call silently corrupt the region skip calculation in the future?
While the current test parameters bypass the ALIGN_DOWN() logic inside
damos_skip_charged_region(), any future expansion or variation of the test
that hits the branch will encounter the incorrect alignment arithmetic
caused by min_region_sz being set to 10.
> +
> + /* 'r2' is not processed, it should not skip */
> + KUNIT_EXPECT_EQ(test, skipped, false);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919080453.628679-1-aethernet65535@gmail.com?part=1
next prev parent reply other threads:[~2026-09-19 8:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-19 8:01 [RESEND PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region Liew Rui Yan
2026-09-19 8:12 ` sashiko-bot [this message]
2026-09-19 16:07 ` SJ Park
2026-09-19 19:09 ` Liew Rui Yan
2026-09-24 7:57 ` Liew Rui Yan
2026-09-24 8:41 ` 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=20260919081239.8A3D91F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=aethernet65535@gmail.com \
--cc=damon@lists.linux.dev \
--cc=sashiko-reviews@lists.linux.dev \
/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