[PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[Liew Rui Yan]

Commit 50585192bc2e ("mm/damon/schemes: skip already charged targets and
regions") introduced a minor issue that causes the last region to be
skipped before it is processed.

Example:

    1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).
    2. Quota is configured to process only 100 bytes per window.
    3. Window 1: Processes R1 (0-100).  Quota is full.  charge_{target,
       addr}_from is saved at (Target, 100).
    4. Window 2: The loop reaches R2.  Because R2 is
       damon_last_region(t), the old code unconditionally returns true,
       skipping R2 entirely and resetting the charge_{target,addr}_from.

    Result: R2 is permanently skipped even though it has never been
    processed.

Add a test to prevent this regression in the future.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 mm/damon/tests/core-kunit.h | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 4a536d41cdb2..90b0fda34fba 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1632,6 +1632,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;
+	bool skipped;
+
+	t = damon_new_target();
+	if (!t) {
+		kunit_skip(test, "target alloc fail");
+	}
+
+	r1 = damon_new_region(0, 100);
+	if (!r1) {
+		damon_free_target(t);
+		kunit_skip(test, "region 1 alloc fail");
+	}
+	damon_add_region(r1, t);
+
+	r2 = damon_new_region(100, 200);
+	if (!r2) {
+		damon_free_target(t);
+		kunit_skip(test, "region 2 alloc fail");
+	}
+	damon_add_region(r2, t);
+
+	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);
+
+	/* 'r2' is not processed, it should not skip */
+	KUNIT_EXPECT_EQ(test, skipped, false);
+
+	/* 'r2' is last region, it should reset charge_{target,addr}_from */
+	KUNIT_EXPECT_PTR_EQ(test, s.quota.charge_target_from, NULL);
+	KUNIT_EXPECT_EQ(test, s.quota.charge_addr_from, 0);
+
+	damon_free_target(t);
+}
+
 static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_target),
 	KUNIT_CASE(damon_test_regions),
@@ -1664,6 +1706,7 @@ static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_is_last_region),
 	KUNIT_CASE(damon_test_walk_control_obsolete),
 	KUNIT_CASE(damon_test_rand),
+	KUNIT_CASE(damon_test_last_region_quota_reset),
 	{},
 };
 
-- 
2.55.0

Re: [PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a KUnit test for a bug in `damos_skip_charged_region()` but completely omits the code fix, causing the test to fail unconditionally and break CI.
--

commit 7f67629ee782b9649498afa3088f50459bfb8f3b
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

This commit introduces a KUnit test to verify that the last region is not
skipped before being processed.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 4a536d41cdb2d..90b0fda34fba2 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
[ ... ]
> @@ -1632,6 +1632,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;
> +	bool skipped;
[ ... ]
> +	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);
> +
> +	/* 'r2' is not processed, it should not skip */
> +	KUNIT_EXPECT_EQ(test, skipped, false);

[Severity: Medium]
Does this patch omit the fix for the core logic?

Without an accompanying fix for damos_skip_charged_region() in
mm/damon/core.c, won't this new test unconditionally fail and break CI?

Because the underlying bug is not fixed in this patch, the old logic still
triggers and returns true when it processes the last region:

mm/damon/core.c:damos_skip_charged_region() {
	...
	if (quota->charge_target_from) {
		if (t != quota->charge_target_from)
			return true;
		if (r == damon_last_region(t)) {
			quota->charge_target_from = NULL;
			quota->charge_addr_from = 0;
			return true;
		}
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831113735.3349-1-aethernet65535@gmail.com?part=1

Re: [PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[Liew Rui Yan]

On Mon, 31 Aug 2026 11:50:14 +0000 sashiko-bot@kernel.org wrote:

> commit 7f67629ee782b9649498afa3088f50459bfb8f3b
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/tests/core-kunit: add test for unconditionally skipping the last region
> 
> This commit introduces a KUnit test to verify that the last region is not
> skipped before being processed.
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 4a536d41cdb2d..90b0fda34fba2 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> [ ... ]
> > @@ -1632,6 +1632,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;
> > +	bool skipped;
> [ ... ]
> > +	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);
> > +
> > +	/* 'r2' is not processed, it should not skip */
> > +	KUNIT_EXPECT_EQ(test, skipped, false);
> 
> [Severity: Medium]
> Does this patch omit the fix for the core logic?
> 
> Without an accompanying fix for damos_skip_charged_region() in
> mm/damon/core.c, won't this new test unconditionally fail and break CI?
> 
> Because the underlying bug is not fixed in this patch, the old logic still
> triggers and returns true when it processes the last region:
> 
> mm/damon/core.c:damos_skip_charged_region() {
> 	...
> 	if (quota->charge_target_from) {
> 		if (t != quota->charge_target_from)
> 			return true;
> 		if (r == damon_last_region(t)) {
> 			quota->charge_target_from = NULL;
> 			quota->charge_addr_from = 0;
> 			return true;
> 		}
> 	...
> }

This test is add for [1].  Therefore, this patch can only be merged
after [1] is merged.

[1] https://lore.kernel.org/damon/20260830063159.6347-1-aethernet65535@gmail.com

Best regards,
Rui Yan

Re: [PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[SJ Park]

On Mon, 31 Aug 2026 20:28:15 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> On Mon, 31 Aug 2026 11:50:14 +0000 sashiko-bot@kernel.org wrote:
[...] 
> > [Severity: Medium]
> > Does this patch omit the fix for the core logic?
> > 
> > Without an accompanying fix for damos_skip_charged_region() in
> > mm/damon/core.c, won't this new test unconditionally fail and break CI?
> > 
> > Because the underlying bug is not fixed in this patch, the old logic still
> > triggers and returns true when it processes the last region:
> > 
> > mm/damon/core.c:damos_skip_charged_region() {
> > 	...
> > 	if (quota->charge_target_from) {
> > 		if (t != quota->charge_target_from)
> > 			return true;
> > 		if (r == damon_last_region(t)) {
> > 			quota->charge_target_from = NULL;
> > 			quota->charge_addr_from = 0;
> > 			return true;
> > 		}
> > 	...
> > }
> 
> This test is add for [1].  Therefore, this patch can only be merged
> after [1] is merged.

Makes sense.

> 
> [1] https://lore.kernel.org/damon/20260830063159.6347-1-aethernet65535@gmail.com

I wonder if you are open to repost this after the dependent patch is merged
into mm-new.  It would be definitely inconvenient for you, but that would help
us getting AI review help.  What do you think?


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/tests/core-kunit: add test for unconditionally skipping the last region

[Liew Rui Yan]

On Mon, 31 Aug 2026 17:25:14 -0700 SJ Park <sj@kernel.org> wrote:

> On Mon, 31 Aug 2026 20:28:15 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> > On Mon, 31 Aug 2026 11:50:14 +0000 sashiko-bot@kernel.org wrote:
> [...] 
> > > [Severity: Medium]
> > > Does this patch omit the fix for the core logic?
> > > 
> > > Without an accompanying fix for damos_skip_charged_region() in
> > > mm/damon/core.c, won't this new test unconditionally fail and break CI?
> > > 
> > > Because the underlying bug is not fixed in this patch, the old logic still
> > > triggers and returns true when it processes the last region:
> > > 
> > > mm/damon/core.c:damos_skip_charged_region() {
> > > 	...
> > > 	if (quota->charge_target_from) {
> > > 		if (t != quota->charge_target_from)
> > > 			return true;
> > > 		if (r == damon_last_region(t)) {
> > > 			quota->charge_target_from = NULL;
> > > 			quota->charge_addr_from = 0;
> > > 			return true;
> > > 		}
> > > 	...
> > > }
> > 
> > This test is add for [1].  Therefore, this patch can only be merged
> > after [1] is merged.
> 
> Makes sense.
> 
> > 
> > [1] https://lore.kernel.org/damon/20260830063159.6347-1-aethernet65535@gmail.com
> 
> I wonder if you are open to repost this after the dependent patch is merged
> into mm-new.  It would be definitely inconvenient for you, but that would help
> us getting AI review help.  What do you think?

Okay!  That will not be inconvenient :>

Best regards,
Rui Yan