* [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() [not found] <20261007154851.45368-1-suhaas@s-joshi.in> @ 2026-10-07 15:48 ` Suhaas Joshi 2026-10-08 10:40 ` Liew Rui Yan 2026-10-08 10:44 ` SJ Park 2026-10-07 15:48 ` [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() Suhaas Joshi 1 sibling, 2 replies; 6+ messages in thread From: Suhaas Joshi @ 2026-10-07 15:48 UTC (permalink / raw) To: sj, akpm; +Cc: damon, linux-mm, linux-kernel, suhaas While merging 2 regions, we iterate over the entire probe_hits[] array, whose size is determined by the DAMON_MAX_PROBES macro. It is possible, however, that we have fewer probes installed than DAMON_MAX_PROBES. In such cases, we end up making redundant iterations. Therefore, to remedy this, iterate over the list of installed probes instead of iterating over the entire array. For doing this, start accepting a struct damon_ctx in damon_merge_two_regions(), and update calling functions to pass this argument. Update the damon_test_merge_two() test to use this new signature for damon_merge_two_regions() as well. Signed-off-by: Suhaas Joshi <suhaas@s-joshi.in> --- mm/damon/core.c | 14 +++++++++----- mm/damon/tests/core-kunit.h | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/mm/damon/core.c b/mm/damon/core.c index 733025b36745..a3af5ee685e1 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -3499,20 +3499,24 @@ static void damon_verify_merge_two_regions( /* * Merge two adjacent regions into one region */ -static void damon_merge_two_regions(struct damon_target *t, - struct damon_region *l, struct damon_region *r) +static void damon_merge_two_regions(struct damon_ctx *ctx, struct damon_target *t, + struct damon_region *l, struct damon_region *r) { unsigned long sz_l = damon_sz_region(l), sz_r = damon_sz_region(r); int i; + struct damon_probe *p; l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) / (sz_l + sz_r); l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r); l->ar.end = r->ar.end; - /* todo: do this for only installed probes */ - for (i = 0; i < DAMON_MAX_PROBES; i++) + + i = 0; + damon_for_each_probe(p, ctx) { l->probe_hits[i] = (l->probe_hits[i] * sz_l + r->probe_hits[i] * sz_r) / (sz_l + sz_r); + ++i; + } damon_verify_merge_two_regions(l, r); damon_destroy_region(r, t); } @@ -3565,7 +3569,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres, goto set_prev_continue; if (damon_sz_region(prev) + damon_sz_region(r) > sz_limit) goto set_prev_continue; - damon_merge_two_regions(t, prev, r); + damon_merge_two_regions(ctx, t, prev, r); continue; set_prev_continue: prev = r; diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h index df84d9cc7d20..5ad89c1d9fc2 100644 --- a/mm/damon/tests/core-kunit.h +++ b/mm/damon/tests/core-kunit.h @@ -182,13 +182,27 @@ static void damon_test_merge_two(struct kunit *test) { struct damon_target *t; struct damon_region *r, *r2, *r3; + struct damon_probe *p; + struct damon_ctx *ctx; int i; + p = damon_new_probe(); + if (!p) + kunit_skip(test, "probe alloc fail"); + ctx = damon_new_ctx(); + if (!ctx) { + damon_destroy_probe(p); + kunit_skip(test, "context alloc fail"); + } + damon_add_probe(ctx, p); t = damon_new_target(); - if (!t) + if (!t) { + damon_destroy_ctx(ctx); kunit_skip(test, "target alloc fail"); + } r = damon_new_region(0, 100); if (!r) { + damon_destroy_ctx(ctx); damon_free_target(t); kunit_skip(test, "region alloc fail"); } @@ -206,7 +220,7 @@ static void damon_test_merge_two(struct kunit *test) r2->age = 21; damon_add_region(r2, t); - damon_merge_two_regions(t, r, r2); + damon_merge_two_regions(ctx, t, r, r2); KUNIT_EXPECT_EQ(test, r->ar.start, 0ul); KUNIT_EXPECT_EQ(test, r->ar.end, 300ul); KUNIT_EXPECT_EQ(test, r->nr_accesses, 16u); @@ -220,6 +234,7 @@ static void damon_test_merge_two(struct kunit *test) } KUNIT_EXPECT_EQ(test, i, 1); + damon_destroy_ctx(ctx); damon_free_target(t); } -- 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() 2026-10-07 15:48 ` [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() Suhaas Joshi @ 2026-10-08 10:40 ` Liew Rui Yan 2026-10-08 10:44 ` SJ Park 1 sibling, 0 replies; 6+ messages in thread From: Liew Rui Yan @ 2026-10-08 10:40 UTC (permalink / raw) To: suhaas Cc: Andrew Morton, damon, linux-kernel, linux-mm, SJ Park, Liew Rui Yan Hi Suhaas, Thank you for your patches series! On Wed, 07 Oct 2026 21:18:45 +0530 Suhaas Joshi <suhaas@s-joshi.in> wrote: > While merging 2 regions, we iterate over the entire probe_hits[] array, > whose size is determined by the DAMON_MAX_PROBES macro. It is possible, > however, that we have fewer probes installed than DAMON_MAX_PROBES. In such I find this a bit hard to read. Could we change it to this? "However, it is possible that we have fewer probes installed than DAMON_MAX_PROBES." > cases, we end up making redundant iterations. Therefore, to remedy this, > iterate over the list of installed probes instead of iterating over the > entire array. For doing this, start accepting a struct damon_ctx in > damon_merge_two_regions(), and update calling functions to pass this > argument. > > Update the damon_test_merge_two() test to use this new signature for > damon_merge_two_regions() as well. > > Signed-off-by: Suhaas Joshi <suhaas@s-joshi.in> > --- > mm/damon/core.c | 14 +++++++++----- > mm/damon/tests/core-kunit.h | 19 +++++++++++++++++-- > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/mm/damon/core.c b/mm/damon/core.c > index 733025b36745..a3af5ee685e1 100644 > --- a/mm/damon/core.c > +++ b/mm/damon/core.c > @@ -3499,20 +3499,24 @@ static void damon_verify_merge_two_regions( > /* > * Merge two adjacent regions into one region > */ > -static void damon_merge_two_regions(struct damon_target *t, > - struct damon_region *l, struct damon_region *r) > +static void damon_merge_two_regions(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *l, struct damon_region *r) > { > unsigned long sz_l = damon_sz_region(l), sz_r = damon_sz_region(r); > int i; > + struct damon_probe *p; > > l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) / > (sz_l + sz_r); > l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r); > l->ar.end = r->ar.end; > - /* todo: do this for only installed probes */ > - for (i = 0; i < DAMON_MAX_PROBES; i++) > + > + i = 0; > + damon_for_each_probe(p, ctx) { > l->probe_hits[i] = (l->probe_hits[i] * sz_l + r->probe_hits[i] > * sz_r) / (sz_l + sz_r); > + ++i; > + } > damon_verify_merge_two_regions(l, r); > damon_destroy_region(r, t); > } > @@ -3565,7 +3569,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres, > goto set_prev_continue; > if (damon_sz_region(prev) + damon_sz_region(r) > sz_limit) > goto set_prev_continue; > - damon_merge_two_regions(t, prev, r); > + damon_merge_two_regions(ctx, t, prev, r); > continue; > set_prev_continue: > prev = r; > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h > index df84d9cc7d20..5ad89c1d9fc2 100644 > --- a/mm/damon/tests/core-kunit.h > +++ b/mm/damon/tests/core-kunit.h > @@ -182,13 +182,27 @@ static void damon_test_merge_two(struct kunit *test) > { > struct damon_target *t; > struct damon_region *r, *r2, *r3; > + struct damon_probe *p; > + struct damon_ctx *ctx; > int i; > > + p = damon_new_probe(); > + if (!p) > + kunit_skip(test, "probe alloc fail"); > + ctx = damon_new_ctx(); > + if (!ctx) { > + damon_destroy_probe(p); > + kunit_skip(test, "context alloc fail"); > + } > + damon_add_probe(ctx, p); > t = damon_new_target(); > - if (!t) > + if (!t) { > + damon_destroy_ctx(ctx); > kunit_skip(test, "target alloc fail"); > + } > r = damon_new_region(0, 100); > if (!r) { > + damon_destroy_ctx(ctx); > damon_free_target(t); > kunit_skip(test, "region alloc fail"); > } > @@ -206,7 +220,7 @@ static void damon_test_merge_two(struct kunit *test) > r2->age = 21; > damon_add_region(r2, t); > > - damon_merge_two_regions(t, r, r2); > + damon_merge_two_regions(ctx, t, r, r2); > KUNIT_EXPECT_EQ(test, r->ar.start, 0ul); > KUNIT_EXPECT_EQ(test, r->ar.end, 300ul); > KUNIT_EXPECT_EQ(test, r->nr_accesses, 16u); > @@ -220,6 +234,7 @@ static void damon_test_merge_two(struct kunit *test) > } > KUNIT_EXPECT_EQ(test, i, 1); > > + damon_destroy_ctx(ctx); > damon_free_target(t); > } > > -- > 2.55.0 Other than the commit message I just mentioned, and the issue reported [1] by Sashiko, LGTM. Reviewed-by: Liew Rui Yan <aethernet65535@gmail.com> [1] https://lore.kernel.org/damon/sashiko-outbox-163177@kernel.org Best regards, Rui Yan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() 2026-10-07 15:48 ` [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() Suhaas Joshi 2026-10-08 10:40 ` Liew Rui Yan @ 2026-10-08 10:44 ` SJ Park 1 sibling, 0 replies; 6+ messages in thread From: SJ Park @ 2026-10-08 10:44 UTC (permalink / raw) To: Suhaas Joshi; +Cc: SJ Park, akpm, damon, linux-mm, linux-kernel Hello Suhaas, On Wed, 7 Oct 2026 21:18:45 +0530 Suhaas Joshi <suhaas@s-joshi.in> wrote: > While merging 2 regions, we iterate over the entire probe_hits[] array, > whose size is determined by the DAMON_MAX_PROBES macro. It is possible, > however, that we have fewer probes installed than DAMON_MAX_PROBES. In such > cases, we end up making redundant iterations. Therefore, to remedy this, > iterate over the list of installed probes instead of iterating over the > entire array. For doing this, start accepting a struct damon_ctx in > damon_merge_two_regions(), and update calling functions to pass this > argument. > > Update the damon_test_merge_two() test to use this new signature for > damon_merge_two_regions() as well. Thank you for this patch. [...] > --- a/mm/damon/tests/core-kunit.h > +++ b/mm/damon/tests/core-kunit.h > @@ -182,13 +182,27 @@ static void damon_test_merge_two(struct kunit *test) > { > struct damon_target *t; > struct damon_region *r, *r2, *r3; > + struct damon_probe *p; > + struct damon_ctx *ctx; > int i; > > + p = damon_new_probe(); > + if (!p) > + kunit_skip(test, "probe alloc fail"); > + ctx = damon_new_ctx(); > + if (!ctx) { > + damon_destroy_probe(p); > + kunit_skip(test, "context alloc fail"); > + } > + damon_add_probe(ctx, p); > t = damon_new_target(); > - if (!t) > + if (!t) { > + damon_destroy_ctx(ctx); > kunit_skip(test, "target alloc fail"); > + } > r = damon_new_region(0, 100); > if (!r) { > + damon_destroy_ctx(ctx); > damon_free_target(t); > kunit_skip(test, "region alloc fail"); > } > @@ -206,7 +220,7 @@ static void damon_test_merge_two(struct kunit *test) > r2->age = 21; > damon_add_region(r2, t); As also found [1] by Sashiko, seems this patch mistakenly not calling damon_destroy_ctx() for r2 allocation failure. Could you please add that? [1] https://lore.kernel.org/sashiko-outbox-163177@kernel.org Thanks, SJ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() [not found] <20261007154851.45368-1-suhaas@s-joshi.in> 2026-10-07 15:48 ` [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() Suhaas Joshi @ 2026-10-07 15:48 ` Suhaas Joshi 2026-10-08 10:44 ` Liew Rui Yan 2026-10-08 11:03 ` SJ Park 1 sibling, 2 replies; 6+ messages in thread From: Suhaas Joshi @ 2026-10-07 15:48 UTC (permalink / raw) To: sj, akpm; +Cc: damon, linux-mm, linux-kernel, suhaas In damon_split_region_at(), we copy the entire probe_hits[] and last_probe_hits[] arrays of DAMON_MAX_PROBES size. However it is possible that the number of probes installed is less than DAMON_MAX_PROBES, in which case one or more iterations is/are redundant. To remedy this, iterate over only as many times as the number of installed probes. To do so, accept a struct damon_ctx pointer in damon_split_region_at(), and update all invocations of the function to pass the new argument. Also update damon_test_split_at() and damos_test_filter_out() in core-kunit.h to supply a context to function paths that call damon_split_region_at(). Signed-off-by: Suhaas Joshi <suhaas@s-joshi.in> --- mm/damon/core.c | 42 +++++++++++++++++--------------- mm/damon/tests/core-kunit.h | 48 ++++++++++++++++++++++++++++++------- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/mm/damon/core.c b/mm/damon/core.c index a3af5ee685e1..025df4d19e20 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -2049,8 +2049,8 @@ static unsigned long damon_region_sz_limit(struct damon_ctx *ctx) return sz; } -static int damon_split_region_at(struct damon_target *t, - struct damon_region *r, unsigned long sz_r); +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, + struct damon_region *r, unsigned long sz_r); /* * damon_apply_min_nr_regions() - Make effect of min_nr_regions parameter. @@ -2075,7 +2075,7 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx) damon_for_each_target(t, ctx) { damon_for_each_region_safe(r, next, t) { while (damon_sz_region(r) > max_region_sz) { - if (damon_split_region_at(t, r, max_region_sz)) + if (damon_split_region_at(ctx, t, r, max_region_sz)) goto out; r = damon_next_region(r); } @@ -2494,9 +2494,9 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_region *r, * * Return: true if the region should be skipped, false otherwise. */ -static bool damos_skip_charged_region(struct damon_target *t, - struct damon_region *r, struct damos *s, - unsigned long min_region_sz) +static bool damos_skip_charged_region(struct damon_ctx *ctx, struct damon_target *t, + struct damon_region *r, struct damos *s, + unsigned long min_region_sz) { struct damos_quota *quota = &s->quota; unsigned long sz_to_skip; @@ -2523,7 +2523,7 @@ static bool damos_skip_charged_region(struct damon_target *t, } sz_to_skip = min_region_sz; } - damon_split_region_at(t, r, sz_to_skip); + damon_split_region_at(ctx, t, r, sz_to_skip); skip = true; } } @@ -2581,12 +2581,12 @@ static bool damos_filter_match(struct damon_ctx *ctx, struct damon_target *t, } /* start before the range and overlap */ if (r->ar.start < start) { - damon_split_region_at(t, r, start - r->ar.start); + damon_split_region_at(ctx, t, r, start - r->ar.start); matched = false; break; } /* start inside the range */ - damon_split_region_at(t, r, end - r->ar.start); + damon_split_region_at(ctx, t, r, end - r->ar.start); matched = true; break; case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM: @@ -2779,7 +2779,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t, c->min_region_sz); if (!sz) goto update_stat; - if (damon_split_region_at(t, r, sz)) + if (damon_split_region_at(c, t, r, sz)) goto update_stat; } if (damos_core_filter_out(c, t, r, s)) @@ -2826,7 +2826,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c, if (damos_quota_is_full(quota, c->min_region_sz)) continue; - if (damos_skip_charged_region(t, r, s, c->min_region_sz)) + if (damos_skip_charged_region(c, t, r, s, c->min_region_sz)) continue; if (s->max_nr_snapshots && @@ -3643,10 +3643,12 @@ static void damon_verify_split_region_at(struct damon_region *r, * * Return: 0 on success, negative error code otherwise. */ -static int damon_split_region_at(struct damon_target *t, - struct damon_region *r, unsigned long sz_r) +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, + struct damon_region *r, unsigned long sz_r) { struct damon_region *new; + struct damon_probe *p; + int i; damon_verify_split_region_at(r, sz_r); new = damon_new_region(r->ar.start + sz_r, r->ar.end); @@ -3658,11 +3660,13 @@ static int damon_split_region_at(struct damon_target *t, new->age = r->age; new->last_nr_accesses = r->last_nr_accesses; new->nr_accesses = r->nr_accesses; - /* todo: do this for only installed probes */ - memcpy(new->probe_hits, r->probe_hits, sizeof(r->probe_hits)); - memcpy(new->last_probe_hits, r->last_probe_hits, - sizeof(r->last_probe_hits)); + i = 0; + damon_for_each_probe(p, ctx) { + new->probe_hits[i] = r->probe_hits[i]; + new->last_probe_hits[i] = r->last_probe_hits[i]; + ++i; + } damon_insert_region(new, r, damon_next_region(r), t); return 0; } @@ -3691,7 +3695,7 @@ static void damon_split_regions_of(struct damon_ctx *ctx, if (sz_sub == 0 || sz_sub >= sz_region) continue; - damon_split_region_at(t, r, sz_sub); + damon_split_region_at(ctx, t, r, sz_sub); sz_region = sz_sub; } } @@ -3723,7 +3727,7 @@ static void damon_split_some_regions(struct damon_ctx *ctx, if (sz_sub == 0 || sz_sub >= sz_region) continue; - damon_split_region_at(t, r, sz_sub); + damon_split_region_at(ctx, t, r, sz_sub); } } } diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h index 5ad89c1d9fc2..e02604c7e0bc 100644 --- a/mm/damon/tests/core-kunit.h +++ b/mm/damon/tests/core-kunit.h @@ -141,12 +141,26 @@ static void damon_test_split_at(struct kunit *test) { struct damon_target *t; struct damon_region *r, *r_new; + struct damon_ctx *ctx; + struct damon_probe *p; + p = damon_new_probe(); + if (!p) + kunit_skip(test, "probe alloc fail"); + ctx = damon_new_ctx(); + if (!ctx) { + damon_destroy_probe(p); + kunit_skip(test, "context alloc fail"); + } + damon_add_probe(ctx, p); t = damon_new_target(); - if (!t) + if (!t) { + damon_destroy_ctx(ctx); kunit_skip(test, "target alloc fail"); + } r = damon_new_region(0, 100); if (!r) { + damon_destroy_ctx(ctx); damon_free_target(t); kunit_skip(test, "region alloc fail"); } @@ -156,7 +170,7 @@ static void damon_test_split_at(struct kunit *test) r->last_probe_hits[0] = 3; r->age = 10; damon_add_region(r, t); - damon_split_region_at(t, r, 25); + damon_split_region_at(ctx, t, r, 25); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2); if (damon_nr_regions(t) != 2) goto out; @@ -176,6 +190,7 @@ static void damon_test_split_at(struct kunit *test) out: damon_free_target(t); + damon_destroy_ctx(ctx); } static void damon_test_merge_two(struct kunit *test) @@ -1648,19 +1663,35 @@ static void damos_test_filter_out(struct kunit *test) struct damon_target *t; struct damon_region *r, *r2; struct damos_filter *f; + struct damon_ctx *ctx; + struct damon_probe *p; + + p = damon_new_probe(); + if (!p) + kunit_skip(test, "probe alloc fail"); + ctx = damon_new_ctx(); + if (!ctx) { + damon_destroy_probe(p); + kunit_skip(test, "context alloc fail"); + } + damon_add_probe(ctx, p); f = damos_new_filter(DAMOS_FILTER_TYPE_ADDR, true, false); - if (!f) + if (!f) { + damon_destroy_ctx(ctx); kunit_skip(test, "filter alloc fail"); + } f->addr_range = (struct damon_addr_range){.start = 2, .end = 6}; t = damon_new_target(); if (!t) { + damon_destroy_ctx(ctx); damos_destroy_filter(f); kunit_skip(test, "target alloc fail"); } r = damon_new_region(3, 5); if (!r) { + damon_destroy_ctx(ctx); damos_destroy_filter(f); damon_free_target(t); kunit_skip(test, "region alloc fail"); @@ -1668,27 +1699,27 @@ static void damos_test_filter_out(struct kunit *test) damon_add_region(r, t); /* region in the range */ - KUNIT_EXPECT_TRUE(test, damos_filter_match(NULL, t, r, f, 1)); + KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); /* region before the range */ r->ar.start = 1; r->ar.end = 2; KUNIT_EXPECT_FALSE(test, - damos_filter_match(NULL, t, r, f, 1)); + damos_filter_match(ctx, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); /* region after the range */ r->ar.start = 6; r->ar.end = 8; KUNIT_EXPECT_FALSE(test, - damos_filter_match(NULL, t, r, f, 1)); + damos_filter_match(ctx, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); /* region started before the range */ r->ar.start = 1; r->ar.end = 4; - KUNIT_EXPECT_FALSE(test, damos_filter_match(NULL, t, r, f, 1)); + KUNIT_EXPECT_FALSE(test, damos_filter_match(ctx, t, r, f, 1)); /* filter should have split the region */ KUNIT_EXPECT_EQ(test, r->ar.start, 1); KUNIT_EXPECT_EQ(test, r->ar.end, 2); @@ -1704,7 +1735,7 @@ static void damos_test_filter_out(struct kunit *test) r->ar.start = 2; r->ar.end = 8; KUNIT_EXPECT_TRUE(test, - damos_filter_match(NULL, t, r, f, 1)); + damos_filter_match(ctx, t, r, f, 1)); /* filter should have split the region */ KUNIT_EXPECT_EQ(test, r->ar.start, 2); KUNIT_EXPECT_EQ(test, r->ar.end, 6); @@ -1717,6 +1748,7 @@ static void damos_test_filter_out(struct kunit *test) damon_destroy_region(r2, t); out: + damon_destroy_ctx(ctx); damon_free_target(t); damos_free_filter(f); } -- 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() 2026-10-07 15:48 ` [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() Suhaas Joshi @ 2026-10-08 10:44 ` Liew Rui Yan 2026-10-08 11:03 ` SJ Park 1 sibling, 0 replies; 6+ messages in thread From: Liew Rui Yan @ 2026-10-08 10:44 UTC (permalink / raw) To: suhaas Cc: Andrew Morton, damon, linux-kernel, linux-mm, SJ Park, Liew Rui Yan On Wed, 07 Oct 2026 21:18:46 +0530 Suhaas Joshi <suhaas@s-joshi.in> wrote: > In damon_split_region_at(), we copy the entire probe_hits[] and > last_probe_hits[] arrays of DAMON_MAX_PROBES size. However it is possible > that the number of probes installed is less than DAMON_MAX_PROBES, in which > case one or more iterations is/are redundant. To remedy this, iterate over > only as many times as the number of installed probes. To do so, accept a > struct damon_ctx pointer in damon_split_region_at(), and update all > invocations of the function to pass the new argument. > > Also update damon_test_split_at() and damos_test_filter_out() in > core-kunit.h to supply a context to function paths that call > damon_split_region_at(). > > Signed-off-by: Suhaas Joshi <suhaas@s-joshi.in> > --- > mm/damon/core.c | 42 +++++++++++++++++--------------- > mm/damon/tests/core-kunit.h | 48 ++++++++++++++++++++++++++++++------- > 2 files changed, 63 insertions(+), 27 deletions(-) > > diff --git a/mm/damon/core.c b/mm/damon/core.c > index a3af5ee685e1..025df4d19e20 100644 > --- a/mm/damon/core.c > +++ b/mm/damon/core.c > @@ -2049,8 +2049,8 @@ static unsigned long damon_region_sz_limit(struct damon_ctx *ctx) > return sz; > } > > -static int damon_split_region_at(struct damon_target *t, > - struct damon_region *r, unsigned long sz_r); > +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, unsigned long sz_r); > > /* > * damon_apply_min_nr_regions() - Make effect of min_nr_regions parameter. > @@ -2075,7 +2075,7 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx) > damon_for_each_target(t, ctx) { > damon_for_each_region_safe(r, next, t) { > while (damon_sz_region(r) > max_region_sz) { > - if (damon_split_region_at(t, r, max_region_sz)) > + if (damon_split_region_at(ctx, t, r, max_region_sz)) > goto out; > r = damon_next_region(r); > } > @@ -2494,9 +2494,9 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_region *r, > * > * Return: true if the region should be skipped, false otherwise. > */ > -static bool damos_skip_charged_region(struct damon_target *t, > - struct damon_region *r, struct damos *s, > - unsigned long min_region_sz) > +static bool damos_skip_charged_region(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, struct damos *s, > + unsigned long min_region_sz) > { > struct damos_quota *quota = &s->quota; > unsigned long sz_to_skip; > @@ -2523,7 +2523,7 @@ static bool damos_skip_charged_region(struct damon_target *t, > } > sz_to_skip = min_region_sz; > } > - damon_split_region_at(t, r, sz_to_skip); > + damon_split_region_at(ctx, t, r, sz_to_skip); > skip = true; > } > } > @@ -2581,12 +2581,12 @@ static bool damos_filter_match(struct damon_ctx *ctx, struct damon_target *t, > } > /* start before the range and overlap */ > if (r->ar.start < start) { > - damon_split_region_at(t, r, start - r->ar.start); > + damon_split_region_at(ctx, t, r, start - r->ar.start); > matched = false; > break; > } > /* start inside the range */ > - damon_split_region_at(t, r, end - r->ar.start); > + damon_split_region_at(ctx, t, r, end - r->ar.start); > matched = true; > break; > case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM: > @@ -2779,7 +2779,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t, > c->min_region_sz); > if (!sz) > goto update_stat; > - if (damon_split_region_at(t, r, sz)) > + if (damon_split_region_at(c, t, r, sz)) > goto update_stat; > } > if (damos_core_filter_out(c, t, r, s)) > @@ -2826,7 +2826,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c, > if (damos_quota_is_full(quota, c->min_region_sz)) > continue; > > - if (damos_skip_charged_region(t, r, s, c->min_region_sz)) > + if (damos_skip_charged_region(c, t, r, s, c->min_region_sz)) > continue; > > if (s->max_nr_snapshots && > @@ -3643,10 +3643,12 @@ static void damon_verify_split_region_at(struct damon_region *r, > * > * Return: 0 on success, negative error code otherwise. > */ > -static int damon_split_region_at(struct damon_target *t, > - struct damon_region *r, unsigned long sz_r) > +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, unsigned long sz_r) > { > struct damon_region *new; > + struct damon_probe *p; > + int i; > > damon_verify_split_region_at(r, sz_r); > new = damon_new_region(r->ar.start + sz_r, r->ar.end); > @@ -3658,11 +3660,13 @@ static int damon_split_region_at(struct damon_target *t, > new->age = r->age; > new->last_nr_accesses = r->last_nr_accesses; > new->nr_accesses = r->nr_accesses; > - /* todo: do this for only installed probes */ > - memcpy(new->probe_hits, r->probe_hits, sizeof(r->probe_hits)); > - memcpy(new->last_probe_hits, r->last_probe_hits, > - sizeof(r->last_probe_hits)); > > + i = 0; > + damon_for_each_probe(p, ctx) { > + new->probe_hits[i] = r->probe_hits[i]; > + new->last_probe_hits[i] = r->last_probe_hits[i]; > + ++i; > + } > damon_insert_region(new, r, damon_next_region(r), t); > return 0; > } > @@ -3691,7 +3695,7 @@ static void damon_split_regions_of(struct damon_ctx *ctx, > if (sz_sub == 0 || sz_sub >= sz_region) > continue; > > - damon_split_region_at(t, r, sz_sub); > + damon_split_region_at(ctx, t, r, sz_sub); > sz_region = sz_sub; > } > } > @@ -3723,7 +3727,7 @@ static void damon_split_some_regions(struct damon_ctx *ctx, > if (sz_sub == 0 || sz_sub >= sz_region) > continue; > > - damon_split_region_at(t, r, sz_sub); > + damon_split_region_at(ctx, t, r, sz_sub); > } > } > } > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h > index 5ad89c1d9fc2..e02604c7e0bc 100644 > --- a/mm/damon/tests/core-kunit.h > +++ b/mm/damon/tests/core-kunit.h > @@ -141,12 +141,26 @@ static void damon_test_split_at(struct kunit *test) > { > struct damon_target *t; > struct damon_region *r, *r_new; > + struct damon_ctx *ctx; > + struct damon_probe *p; > > + p = damon_new_probe(); > + if (!p) > + kunit_skip(test, "probe alloc fail"); > + ctx = damon_new_ctx(); > + if (!ctx) { > + damon_destroy_probe(p); > + kunit_skip(test, "context alloc fail"); > + } > + damon_add_probe(ctx, p); > t = damon_new_target(); > - if (!t) > + if (!t) { > + damon_destroy_ctx(ctx); > kunit_skip(test, "target alloc fail"); > + } > r = damon_new_region(0, 100); > if (!r) { > + damon_destroy_ctx(ctx); > damon_free_target(t); > kunit_skip(test, "region alloc fail"); > } > @@ -156,7 +170,7 @@ static void damon_test_split_at(struct kunit *test) > r->last_probe_hits[0] = 3; > r->age = 10; > damon_add_region(r, t); > - damon_split_region_at(t, r, 25); > + damon_split_region_at(ctx, t, r, 25); > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2); > if (damon_nr_regions(t) != 2) > goto out; > @@ -176,6 +190,7 @@ static void damon_test_split_at(struct kunit *test) > > out: > damon_free_target(t); > + damon_destroy_ctx(ctx); > } > > static void damon_test_merge_two(struct kunit *test) > @@ -1648,19 +1663,35 @@ static void damos_test_filter_out(struct kunit *test) > struct damon_target *t; > struct damon_region *r, *r2; > struct damos_filter *f; > + struct damon_ctx *ctx; > + struct damon_probe *p; > + > + p = damon_new_probe(); > + if (!p) > + kunit_skip(test, "probe alloc fail"); > + ctx = damon_new_ctx(); > + if (!ctx) { > + damon_destroy_probe(p); > + kunit_skip(test, "context alloc fail"); > + } > + damon_add_probe(ctx, p); > > f = damos_new_filter(DAMOS_FILTER_TYPE_ADDR, true, false); > - if (!f) > + if (!f) { > + damon_destroy_ctx(ctx); > kunit_skip(test, "filter alloc fail"); > + } > f->addr_range = (struct damon_addr_range){.start = 2, .end = 6}; > > t = damon_new_target(); > if (!t) { > + damon_destroy_ctx(ctx); > damos_destroy_filter(f); > kunit_skip(test, "target alloc fail"); > } > r = damon_new_region(3, 5); > if (!r) { > + damon_destroy_ctx(ctx); > damos_destroy_filter(f); > damon_free_target(t); > kunit_skip(test, "region alloc fail"); > @@ -1668,27 +1699,27 @@ static void damos_test_filter_out(struct kunit *test) > damon_add_region(r, t); > > /* region in the range */ > - KUNIT_EXPECT_TRUE(test, damos_filter_match(NULL, t, r, f, 1)); > + KUNIT_EXPECT_TRUE(test, damos_filter_match(ctx, t, r, f, 1)); > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); > > /* region before the range */ > r->ar.start = 1; > r->ar.end = 2; > KUNIT_EXPECT_FALSE(test, > - damos_filter_match(NULL, t, r, f, 1)); > + damos_filter_match(ctx, t, r, f, 1)); > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); > > /* region after the range */ > r->ar.start = 6; > r->ar.end = 8; > KUNIT_EXPECT_FALSE(test, > - damos_filter_match(NULL, t, r, f, 1)); > + damos_filter_match(ctx, t, r, f, 1)); > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1); > > /* region started before the range */ > r->ar.start = 1; > r->ar.end = 4; > - KUNIT_EXPECT_FALSE(test, damos_filter_match(NULL, t, r, f, 1)); > + KUNIT_EXPECT_FALSE(test, damos_filter_match(ctx, t, r, f, 1)); > /* filter should have split the region */ > KUNIT_EXPECT_EQ(test, r->ar.start, 1); > KUNIT_EXPECT_EQ(test, r->ar.end, 2); > @@ -1704,7 +1735,7 @@ static void damos_test_filter_out(struct kunit *test) > r->ar.start = 2; > r->ar.end = 8; > KUNIT_EXPECT_TRUE(test, > - damos_filter_match(NULL, t, r, f, 1)); > + damos_filter_match(ctx, t, r, f, 1)); > /* filter should have split the region */ > KUNIT_EXPECT_EQ(test, r->ar.start, 2); > KUNIT_EXPECT_EQ(test, r->ar.end, 6); > @@ -1717,6 +1748,7 @@ static void damos_test_filter_out(struct kunit *test) > damon_destroy_region(r2, t); > > out: > + damon_destroy_ctx(ctx); > damon_free_target(t); > damos_free_filter(f); > } > -- > 2.55.0 Thank you for your patches! LGTM. Reviewed-by: Liew Rui Yan <aethernet65535@gmail.com> Best regards, Rui Yan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() 2026-10-07 15:48 ` [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() Suhaas Joshi 2026-10-08 10:44 ` Liew Rui Yan @ 2026-10-08 11:03 ` SJ Park 1 sibling, 0 replies; 6+ messages in thread From: SJ Park @ 2026-10-08 11:03 UTC (permalink / raw) To: Suhaas Joshi; +Cc: SJ Park, akpm, damon, linux-mm, linux-kernel Hello Suhaas, From the next time, please send a patch series as a single thread, with a cover letter. On Wed, 7 Oct 2026 21:18:46 +0530 Suhaas Joshi <suhaas@s-joshi.in> wrote: > In damon_split_region_at(), we copy the entire probe_hits[] and > last_probe_hits[] arrays of DAMON_MAX_PROBES size. However it is possible > that the number of probes installed is less than DAMON_MAX_PROBES, in which > case one or more iterations is/are redundant. To remedy this, iterate over > only as many times as the number of installed probes. To do so, accept a > struct damon_ctx pointer in damon_split_region_at(), and update all > invocations of the function to pass the new argument. > > Also update damon_test_split_at() and damos_test_filter_out() in > core-kunit.h to supply a context to function paths that call > damon_split_region_at(). > > Signed-off-by: Suhaas Joshi <suhaas@s-joshi.in> > --- > mm/damon/core.c | 42 +++++++++++++++++--------------- > mm/damon/tests/core-kunit.h | 48 ++++++++++++++++++++++++++++++------- > 2 files changed, 63 insertions(+), 27 deletions(-) > > diff --git a/mm/damon/core.c b/mm/damon/core.c > index a3af5ee685e1..025df4d19e20 100644 > --- a/mm/damon/core.c > +++ b/mm/damon/core.c > @@ -2049,8 +2049,8 @@ static unsigned long damon_region_sz_limit(struct damon_ctx *ctx) > return sz; > } > > -static int damon_split_region_at(struct damon_target *t, > - struct damon_region *r, unsigned long sz_r); > +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, unsigned long sz_r); Please wrap lines for 80 columns [1] limit per line. Same for below. [...] > @@ -2494,9 +2494,9 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_region *r, > * > * Return: true if the region should be skipped, false otherwise. > */ > -static bool damos_skip_charged_region(struct damon_target *t, > - struct damon_region *r, struct damos *s, > - unsigned long min_region_sz) > +static bool damos_skip_charged_region(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, struct damos *s, > + unsigned long min_region_sz) Please update the comment above to document the new parameter. [...] > @@ -3643,10 +3643,12 @@ static void damon_verify_split_region_at(struct damon_region *r, > * > * Return: 0 on success, negative error code otherwise. > */ > -static int damon_split_region_at(struct damon_target *t, > - struct damon_region *r, unsigned long sz_r) > +static int damon_split_region_at(struct damon_ctx *ctx, struct damon_target *t, > + struct damon_region *r, unsigned long sz_r) Ditto. [...] [1] https://docs.kernel.org/process/coding-style.html#breaking-long-lines-and-strings Thanks, SJ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-08 11:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20261007154851.45368-1-suhaas@s-joshi.in>
2026-10-07 15:48 ` [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions() Suhaas Joshi
2026-10-08 10:40 ` Liew Rui Yan
2026-10-08 10:44 ` SJ Park
2026-10-07 15:48 ` [PATCH 2/2] mm/damon/core: Copy only installed probe in damon_split_region_at() Suhaas Joshi
2026-10-08 10:44 ` Liew Rui Yan
2026-10-08 11:03 ` SJ Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®