All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.