DAMON development mailing list
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: Jason Angelov <jasonangelov@ucla.edu>
Cc: SJ Park <sj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] mm/damon/tests/core-kunit: test DAMOS_FILTER_TYPE_AGE_MS interval changes
Date: Fri, 13 Nov 2026 18:20:03 -0800	[thread overview]
Message-ID: <20261114022004.110044-1-sj@kernel.org> (raw)
In-Reply-To: <20261113-test-age-ms-filter-v1-3-b99eab2f71bb@ucla.edu>

On Fri, 13 Nov 2026 07:58:57 -0800 Jason Angelov <jasonangelov@ucla.edu> wrote:

> DAMOS_FILTER_TYPE_AGE_MS defines an inclusive range for age in
> milliseconds to filter for target regions.
> Aggregation interval can be changed by users or intervals
> auto-tuning features.

Let's put the second sentence in the line of the end of the first sentence.  In
the way, we can save one line.

> 
> Add a kunit test for each:
> 
> - convert region age to milliseconds, correct behavior at boundaries
> for defined filter range

Let's add two spaces indentation, to make the bullet point lists easier to
read.

> - DAMOS_FILTER_TYPE_AGE_MS filter keeps correct behavior even when
> aggregation interval changes

Ditto.

> 
> Signed-off-by: Jason Angelov <jasonangelov@ucla.edu>
> ---
>  mm/damon/tests/core-kunit.h | 81 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index fb84d30a712d..6de7c2531702 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -1837,6 +1837,86 @@ static void damos_test_age_ms_filter_is_core(struct kunit *test)
>  	KUNIT_EXPECT_FALSE(test, damos_filter_for_ops(DAMOS_FILTER_TYPE_AGE_MS));
>  }
>  
> +static void damos_test_age_ms_filter_interval_changes(struct kunit *test)
> +{
> +	/* Filter window: 200-400 ms.  Start: 100 ms interval, age 0. */

This looks like not a complete sentence for readers but a personal note.
Please make the comment more human-readable or just drop.

> +	struct damos_filter filter = {
> +		.type = DAMOS_FILTER_TYPE_AGE_MS,
> +		.matching = true,
> +		.range_min = 200,
> +		.range_max = 400,
> +	};
> +	struct damon_attrs attrs = {
> +		.sample_interval = 5000,
> +		.aggr_interval = 100000,
> +		.min_nr_regions = 3,
> +		.max_nr_regions = 3,
> +	};
> +	struct damon_ctx *ctx;
> +	struct damon_target *t;
> +	struct damon_region *r;
> +	int err;
> +
> +	ctx = damon_new_ctx();
> +	if (!ctx)
> +		kunit_skip(test, "ctx alloc fail");
> +	t = damon_new_target();
> +	if (!t) {
> +		damon_destroy_ctx(ctx);
> +		kunit_skip(test, "target alloc fail");
> +	}
> +	damon_add_target(ctx, t);
> +	r = damon_new_region(0, DAMON_MIN_REGION_SZ);
> +	if (!r) {
> +		damon_destroy_ctx(ctx);
> +		kunit_skip(test, "region alloc fail");
> +	}
> +	damon_add_region(r, t);
> +	KUNIT_EXPECT_EQ(test, damon_set_attrs(ctx, &attrs), 0);

Why test damon_set_attrs() in this test?

> +
> +	/* below window: age 1 = 100 ms */

What the "window" means?  Filter's range_{min,max}?  This again feels like a
personal note rather than somewhat useful for other code readers.  Please make
the comment more human readable or just drop.

> +	r->age = 1;
> +	KUNIT_EXPECT_FALSE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	/* inside window: age 2 = 200 ms */

Ditto.  Same for all comments in this function.

> +	r->age = 2;
> +	KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	/*
> +	 * change aggregation interval 100 -> 200 ms:
> +	 * inside window: age rescaled 2 -> 1, still 200 ms
> +	 */
> +	attrs.aggr_interval = 200000;
> +	KUNIT_EXPECT_EQ(test, damon_set_attrs(ctx, &attrs), 0);

Why test damon_set_attrs() in this test?

> +	KUNIT_EXPECT_EQ(test, r->age, 1u);
> +	KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	/* inside window: age 2 = 400 ms. */
> +	r->age = 2;
> +	KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	/*
> +	 * change aggregation interval 200 -> 50 ms:
> +	 * inside window: age rescaled 2 -> 8, still 400 ms.
> +	 */
> +	attrs.aggr_interval = 50000;
> +	KUNIT_EXPECT_EQ(test, damon_set_attrs(ctx, &attrs), 0);
> +	KUNIT_EXPECT_EQ(test, r->age, 8u);
> +	KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	/* above window: age 9 = 450 ms. */
> +	r->age = 9;
> +	KUNIT_EXPECT_FALSE(test, damos_filter_match(ctx, t, r, &filter,
> +			ctx->min_region_sz, &err));
> +
> +	damon_destroy_ctx(ctx);
> +}
> +
>  static void damos_test_filter_out(struct kunit *test)
>  {
>  	struct damon_target *t;
> @@ -2599,6 +2679,7 @@ static struct kunit_case damon_test_cases[] = {
>  	KUNIT_CASE(damon_test_commit_ctx_keeps_quota),
>  	KUNIT_CASE(damon_test_valid_probe_params),
>  	KUNIT_CASE(damos_test_age_ms_filter_is_core),
> +	KUNIT_CASE(damos_test_age_ms_filter_interval_changes),
>  	KUNIT_CASE(damos_test_filter_out),
>  	KUNIT_CASE(damos_test_apply_scheme_filtered_sz),
>  	KUNIT_CASE(damos_test_apply_scheme_filter_sz_unchanged),
> 
> -- 
> 2.53.0
> 
> 


Thanks,
SJ

  parent reply	other threads:[~2026-11-14  2:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 15:58 [PATCH 0/3] mm/damon/tests: add KUnit coverage for age_ms filters Jason Angelov
2026-11-13 15:58 ` [PATCH 1/3] mm/damon/tests/core-kunit: test DAMOS_FILTER_TYPE_AGE_MS in damos_test_commit_filter() Jason Angelov
2026-11-13 16:05   ` sashiko-bot
2026-11-14  2:00   ` SJ Park
2026-11-13 15:58 ` [PATCH 2/3] mm/damon/tests/core-kunit: test DAMOS_FILTER_TYPE_AGE_MS in damos_filter_for_ops() Jason Angelov
2026-11-13 16:04   ` sashiko-bot
2026-11-14  2:03   ` SJ Park
2026-11-13 15:58 ` [PATCH 3/3] mm/damon/tests/core-kunit: test DAMOS_FILTER_TYPE_AGE_MS interval changes Jason Angelov
2026-11-13 16:02   ` sashiko-bot
2026-11-14  2:20   ` SJ Park [this message]
2026-11-14  2:21 ` [PATCH 0/3] mm/damon/tests: add KUnit coverage for age_ms filters 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=20261114022004.110044-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=jasonangelov@ucla.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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