DAMON development mailing list
 help / color / mirror / Atom feed
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

  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