Yet another batch of misc cleanups and refactoring for DAMON code, tests, and documents.
First two patches (1and 2) rename DAMOS core filters related code for readability.
Three following patches (3-5) refactor page table walk callback functions in DAMON, as suggested by Hugh and David, and I promised.
Next two patches (6 and 7) refactor DAMON core layer kunit test and sysfs interface selftest to be simple and deduplicated.
Final two patches (8 and 9) fix up sphinx and grammatical errors on documents.
SeongJae Park (9): mm/damon: rename damos core filter helpers to have word core mm/damon: rename damos->filters to damos->core_filters mm/damon/vaddr: cleanup using pmd_trans_huge_lock() mm/damon/vaddr: use vm_normal_folio{,_pmd}() instead of damon_get_folio() mm/damon/vaddr: consistently use only pmd_entry for damos_migrate mm/damon/tests/core-kunit: remove DAMON_MIN_REGION redefinition selftests/damon/sysfs.py: merge DAMON status dumping into commitment assertion Docs/mm/damon/maintainer-profile: fix a typo on mm-untable link Docs/mm/damon/maintainer-profile: fix grammartical errors
.clang-format | 4 +- Documentation/mm/damon/maintainer-profile.rst | 10 +- include/linux/damon.h | 14 +- mm/damon/core.c | 25 ++- mm/damon/tests/core-kunit.h | 59 ++++---- mm/damon/vaddr.c | 143 +++++++----------- .../selftests/damon/drgn_dump_damon_status.py | 8 +- tools/testing/selftests/damon/sysfs.py | 45 ++---- 8 files changed, 121 insertions(+), 187 deletions(-)
base-commit: 4e9ec347bc14de636aec3014dee3b5d279ca33bf
DAMOS filters that are handled by the ops layer are linked to damos->ops_filters. Owing to the ops_ prefix on the name, it is easy to understand it is for ops layer handled filters. The other types of filters, which are handled by the core layer, are linked to damos->filters. Because of the name, it is easy to confuse the list is there for not only core layer handled ones but all filters. Avoid such confusions by renaming the field to core_filters.
Signed-off-by: SeongJae Park sj@kernel.org --- include/linux/damon.h | 10 +++++----- mm/damon/core.c | 6 +++--- mm/damon/tests/core-kunit.h | 4 ++-- .../testing/selftests/damon/drgn_dump_damon_status.py | 8 ++++---- tools/testing/selftests/damon/sysfs.py | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h index 6e3db165fe60..3813373a9200 100644 --- a/include/linux/damon.h +++ b/include/linux/damon.h @@ -492,7 +492,7 @@ struct damos_migrate_dests { * @wmarks: Watermarks for automated (in)activation of this scheme. * @migrate_dests: Destination nodes if @action is "migrate_{hot,cold}". * @target_nid: Destination node if @action is "migrate_{hot,cold}". - * @filters: Additional set of &struct damos_filter for &action. + * @core_filters: Additional set of &struct damos_filter for &action. * @ops_filters: ops layer handling &struct damos_filter objects list. * @last_applied: Last @action applied ops-managing entity. * @stat: Statistics of this scheme. @@ -518,7 +518,7 @@ struct damos_migrate_dests { * * Before applying the &action to a memory region, &struct damon_operations * implementation could check pages of the region and skip &action to respect - * &filters + * &core_filters * * The minimum entity that @action can be applied depends on the underlying * &struct damon_operations. Since it may not be aligned with the core layer @@ -562,7 +562,7 @@ struct damos { struct damos_migrate_dests migrate_dests; }; }; - struct list_head filters; + struct list_head core_filters; struct list_head ops_filters; void *last_applied; struct damos_stat stat; @@ -872,10 +872,10 @@ static inline unsigned long damon_sz_region(struct damon_region *r) list_for_each_entry_safe(goal, next, &(quota)->goals, list)
#define damos_for_each_core_filter(f, scheme) \ - list_for_each_entry(f, &(scheme)->filters, list) + list_for_each_entry(f, &(scheme)->core_filters, list)
#define damos_for_each_core_filter_safe(f, next, scheme) \ - list_for_each_entry_safe(f, next, &(scheme)->filters, list) + list_for_each_entry_safe(f, next, &(scheme)->core_filters, list)
#define damos_for_each_ops_filter(f, scheme) \ list_for_each_entry(f, &(scheme)->ops_filters, list) diff --git a/mm/damon/core.c b/mm/damon/core.c index d4cb11ced13f..aedb315b075a 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -306,7 +306,7 @@ void damos_add_filter(struct damos *s, struct damos_filter *f) if (damos_filter_for_ops(f->type)) list_add_tail(&f->list, &s->ops_filters); else - list_add_tail(&f->list, &s->filters); + list_add_tail(&f->list, &s->core_filters); }
static void damos_del_filter(struct damos_filter *f) @@ -397,7 +397,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern, */ scheme->next_apply_sis = 0; scheme->walk_completed = false; - INIT_LIST_HEAD(&scheme->filters); + INIT_LIST_HEAD(&scheme->core_filters); INIT_LIST_HEAD(&scheme->ops_filters); scheme->stat = (struct damos_stat){}; INIT_LIST_HEAD(&scheme->list); @@ -995,7 +995,7 @@ static void damos_set_filters_default_reject(struct damos *s) s->core_filters_default_reject = false; else s->core_filters_default_reject = - damos_filters_default_reject(&s->filters); + damos_filters_default_reject(&s->core_filters); s->ops_filters_default_reject = damos_filters_default_reject(&s->ops_filters); } diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h index 0d2d8cda8631..4380d0312d24 100644 --- a/mm/damon/tests/core-kunit.h +++ b/mm/damon/tests/core-kunit.h @@ -876,7 +876,7 @@ static void damos_test_commit_filter(struct kunit *test) static void damos_test_help_initailize_scheme(struct damos *scheme) { INIT_LIST_HEAD(&scheme->quota.goals); - INIT_LIST_HEAD(&scheme->filters); + INIT_LIST_HEAD(&scheme->core_filters); INIT_LIST_HEAD(&scheme->ops_filters); }
@@ -1140,7 +1140,7 @@ static void damon_test_set_filters_default_reject(struct kunit *test) struct damos scheme; struct damos_filter *target_filter, *anon_filter;
- INIT_LIST_HEAD(&scheme.filters); + INIT_LIST_HEAD(&scheme.core_filters); INIT_LIST_HEAD(&scheme.ops_filters);
damos_set_filters_default_reject(&scheme); diff --git a/tools/testing/selftests/damon/drgn_dump_damon_status.py b/tools/testing/selftests/damon/drgn_dump_damon_status.py index cb4fdbe68acb..5374d18d1fa8 100755 --- a/tools/testing/selftests/damon/drgn_dump_damon_status.py +++ b/tools/testing/selftests/damon/drgn_dump_damon_status.py @@ -175,11 +175,11 @@ def scheme_to_dict(scheme): ['target_nid', int], ['migrate_dests', damos_migrate_dests_to_dict], ]) - filters = [] + core_filters = [] for f in list_for_each_entry( - 'struct damos_filter', scheme.filters.address_of_(), 'list'): - filters.append(damos_filter_to_dict(f)) - dict_['filters'] = filters + 'struct damos_filter', scheme.core_filters.address_of_(), 'list'): + core_filters.append(damos_filter_to_dict(f)) + dict_['core_filters'] = core_filters ops_filters = [] for f in list_for_each_entry( 'struct damos_filter', scheme.ops_filters.address_of_(), 'list'): diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py index b34aea0a6775..b4c5ef5c4d69 100755 --- a/tools/testing/selftests/damon/sysfs.py +++ b/tools/testing/selftests/damon/sysfs.py @@ -132,7 +132,7 @@ def assert_scheme_committed(scheme, dump): assert_watermarks_committed(scheme.watermarks, dump['wmarks']) # TODO: test filters directory for idx, f in enumerate(scheme.core_filters.filters): - assert_filter_committed(f, dump['filters'][idx]) + assert_filter_committed(f, dump['core_filters'][idx]) for idx, f in enumerate(scheme.ops_filters.filters): assert_filter_committed(f, dump['ops_filters'][idx])
A few DAMON core functions including damon_set_regions() were hard-coded to use DAMON_MIN_REGION as their regions management granularity. For simple and human-readable unit tests' expectations, DAMON core layer kunit test re-defines DAMON_MIN_REGION to '1'.
A previous patch series [1] has removed the hard-coded part but kept the redefinition and updated related function calls to explicitly use DAMON_MIN_REGION. Remove the unnecessary redefinition and update relevant function calls to pass literals (number '1') instead of the DAMON_MIN_REGION.
[1] https://lore.kernel.org/20250828171242.59810-1-sj@kernel.org
Signed-off-by: SeongJae Park sj@kernel.org --- mm/damon/core.c | 5 ---- mm/damon/tests/core-kunit.h | 55 ++++++++++++++++++------------------- 2 files changed, 26 insertions(+), 34 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c index aedb315b075a..f9fc0375890a 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -20,11 +20,6 @@ #define CREATE_TRACE_POINTS #include <trace/events/damon.h>
-#ifdef CONFIG_DAMON_KUNIT_TEST -#undef DAMON_MIN_REGION -#define DAMON_MIN_REGION 1 -#endif - static DEFINE_MUTEX(damon_lock); static int nr_running_ctxs; static bool running_exclusive_ctxs; diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h index 4380d0312d24..a1eff023e928 100644 --- a/mm/damon/tests/core-kunit.h +++ b/mm/damon/tests/core-kunit.h @@ -279,7 +279,7 @@ static void damon_test_split_regions_of(struct kunit *test) kunit_skip(test, "region alloc fail"); } damon_add_region(r, t); - damon_split_regions_of(t, 2, DAMON_MIN_REGION); + damon_split_regions_of(t, 2, 1); KUNIT_EXPECT_LE(test, damon_nr_regions(t), 2u); damon_free_target(t);
@@ -292,7 +292,7 @@ static void damon_test_split_regions_of(struct kunit *test) kunit_skip(test, "second region alloc fail"); } damon_add_region(r, t); - damon_split_regions_of(t, 4, DAMON_MIN_REGION); + damon_split_regions_of(t, 4, 1); KUNIT_EXPECT_LE(test, damon_nr_regions(t), 4u); damon_free_target(t); } @@ -373,7 +373,7 @@ static void damon_test_set_regions(struct kunit *test)
damon_add_region(r1, t); damon_add_region(r2, t); - damon_set_regions(t, &range, 1, DAMON_MIN_REGION); + damon_set_regions(t, &range, 1, 1);
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 3); damon_for_each_region(r, t) { @@ -1037,15 +1037,14 @@ static void damos_test_filter_out(struct kunit *test) f = damos_new_filter(DAMOS_FILTER_TYPE_ADDR, true, false); if (!f) kunit_skip(test, "filter alloc fail"); - f->addr_range = (struct damon_addr_range){ - .start = DAMON_MIN_REGION * 2, .end = DAMON_MIN_REGION * 6}; + f->addr_range = (struct damon_addr_range){.start = 2, .end = 6};
t = damon_new_target(); if (!t) { damos_destroy_filter(f); kunit_skip(test, "target alloc fail"); } - r = damon_new_region(DAMON_MIN_REGION * 3, DAMON_MIN_REGION * 5); + r = damon_new_region(3, 5); if (!r) { damos_destroy_filter(f); damon_free_target(t); @@ -1054,50 +1053,48 @@ 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, DAMON_MIN_REGION)); + KUNIT_EXPECT_TRUE(test, damos_filter_match(NULL, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1);
/* region before the range */ - r->ar.start = DAMON_MIN_REGION * 1; - r->ar.end = DAMON_MIN_REGION * 2; + r->ar.start = 1; + r->ar.end = 2; KUNIT_EXPECT_FALSE(test, - damos_filter_match(NULL, t, r, f, DAMON_MIN_REGION)); + damos_filter_match(NULL, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1);
/* region after the range */ - r->ar.start = DAMON_MIN_REGION * 6; - r->ar.end = DAMON_MIN_REGION * 8; + r->ar.start = 6; + r->ar.end = 8; KUNIT_EXPECT_FALSE(test, - damos_filter_match(NULL, t, r, f, DAMON_MIN_REGION)); + damos_filter_match(NULL, t, r, f, 1)); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 1);
/* region started before the range */ - r->ar.start = DAMON_MIN_REGION * 1; - r->ar.end = DAMON_MIN_REGION * 4; - KUNIT_EXPECT_FALSE(test, - damos_filter_match(NULL, t, r, f, DAMON_MIN_REGION)); + r->ar.start = 1; + r->ar.end = 4; + KUNIT_EXPECT_FALSE(test, damos_filter_match(NULL, t, r, f, 1)); /* filter should have split the region */ - KUNIT_EXPECT_EQ(test, r->ar.start, DAMON_MIN_REGION * 1); - KUNIT_EXPECT_EQ(test, r->ar.end, DAMON_MIN_REGION * 2); + KUNIT_EXPECT_EQ(test, r->ar.start, 1); + KUNIT_EXPECT_EQ(test, r->ar.end, 2); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2); r2 = damon_next_region(r); - KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 2); - KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 4); + KUNIT_EXPECT_EQ(test, r2->ar.start, 2); + KUNIT_EXPECT_EQ(test, r2->ar.end, 4); damon_destroy_region(r2, t);
/* region started in the range */ - r->ar.start = DAMON_MIN_REGION * 2; - r->ar.end = DAMON_MIN_REGION * 8; + r->ar.start = 2; + r->ar.end = 8; KUNIT_EXPECT_TRUE(test, - damos_filter_match(NULL, t, r, f, DAMON_MIN_REGION)); + damos_filter_match(NULL, t, r, f, 1)); /* filter should have split the region */ - KUNIT_EXPECT_EQ(test, r->ar.start, DAMON_MIN_REGION * 2); - KUNIT_EXPECT_EQ(test, r->ar.end, DAMON_MIN_REGION * 6); + KUNIT_EXPECT_EQ(test, r->ar.start, 2); + KUNIT_EXPECT_EQ(test, r->ar.end, 6); KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2); r2 = damon_next_region(r); - KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 6); - KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 8); + KUNIT_EXPECT_EQ(test, r2->ar.start, 6); + KUNIT_EXPECT_EQ(test, r2->ar.end, 8); damon_destroy_region(r2, t);
damon_free_target(t);
For each test case, sysfs.py makes changes to DAMON, dumps DAMON internal status and asserts the expectation is met. The dumping part should be the same for all cases, so it is duplicated for each test case. Which means it is easy to make mistakes. Actually a few of those duplicates are not turning DAMON off in case of the dumping failure. It makes following selftests that need to turn DAMON on fails with -EBUSY. Merge the status dumping into commitment assertion with proper dumping failure handling, to deduplicate and avoid the unnecessary following tests failures.
Signed-off-by: SeongJae Park sj@kernel.org --- tools/testing/selftests/damon/sysfs.py | 43 ++++++++------------------ 1 file changed, 13 insertions(+), 30 deletions(-)
diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py index b4c5ef5c4d69..9cca71eb0325 100755 --- a/tools/testing/selftests/damon/sysfs.py +++ b/tools/testing/selftests/damon/sysfs.py @@ -185,7 +185,15 @@ def assert_ctx_committed(ctx, dump): assert_monitoring_targets_committed(ctx.targets, dump['adaptive_targets']) assert_schemes_committed(ctx.schemes, dump['schemes'])
-def assert_ctxs_committed(ctxs, dump): +def assert_ctxs_committed(kdamonds): + status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid) + if err is not None: + print(err) + kdamonds.stop() + exit(1) + + ctxs = kdamonds.kdamonds[0].contexts + dump = status['contexts'] assert_true(len(ctxs) == len(dump), 'ctxs length', dump) for idx, ctx in enumerate(ctxs): assert_ctx_committed(ctx, dump[idx]) @@ -202,13 +210,7 @@ def main(): print('kdamond start failed: %s' % err) exit(1)
- status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid) - if err is not None: - print(err) - kdamonds.stop() - exit(1) - - assert_ctxs_committed(kdamonds.kdamonds[0].contexts, status['contexts']) + assert_ctxs_committed(kdamonds)
context = _damon_sysfs.DamonCtx( monitoring_attrs=_damon_sysfs.DamonAttrs( @@ -256,12 +258,7 @@ def main(): kdamonds.kdamonds[0].contexts = [context] kdamonds.kdamonds[0].commit()
- status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid) - if err is not None: - print(err) - exit(1) - - assert_ctxs_committed(kdamonds.kdamonds[0].contexts, status['contexts']) + assert_ctxs_committed(kdamonds)
# test online commitment of minimum context. context = _damon_sysfs.DamonCtx() @@ -270,12 +267,7 @@ def main(): kdamonds.kdamonds[0].contexts = [context] kdamonds.kdamonds[0].commit()
- status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid) - if err is not None: - print(err) - exit(1) - - assert_ctxs_committed(kdamonds.kdamonds[0].contexts, status['contexts']) + assert_ctxs_committed(kdamonds)
kdamonds.stop()
@@ -303,17 +295,8 @@ def main(): exit(1) kdamonds.kdamonds[0].contexts[0].targets[1].obsolete = True kdamonds.kdamonds[0].commit() - - status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid) - if err is not None: - print(err) - kdamonds.stop() - exit(1) - del kdamonds.kdamonds[0].contexts[0].targets[1] - - assert_ctxs_committed(kdamonds.kdamonds[0].contexts, status['contexts']) - + assert_ctxs_committed(kdamonds) kdamonds.stop()
if __name__ == '__main__':
linux-kselftest-mirror@lists.linaro.org
-
SeongJae Park