[PATCH v3 0/2] mm/damon/core: preserve quota state when constructing schemes

[SJ Park]

From: Karl Mehltretter <kmehlt...@gmail.com>

damon_commit_ctx() first commits the running context's parameters to a
temporary context for validating proposed updates. Constructing the
temporary schemes clears the running schemes' quota state because
damon_new_scheme() initializes the quota passed as a parameter before
copying it to the new scheme. Even an update later rejected with -EINVAL
loses the running quota state.

Initialize the new scheme's copy instead, and add KUnit tests for the
constructor and for accepted and rejected context updates.

Note: below are test results and changelogs that could be removed from
the final commit log.

In the v1 live test with damo, a scheme with a plain 64 KiB size quota and
a 60-second reset interval uses its quota, and a full "damo tune" with
unchanged parameters then lets it try another 64 KiB within the same
window. With the fix, sz_tried stays at 64 KiB.

KUnit was rerun after the rebase on x86-64 and i386. With only patch 2
applied, the two new tests fail. With the fix, all 46 DAMON KUnit tests
pass on both architectures.

The v1 DAMON selftests showed no new failures (QEMU TCG guest; the
wss_estimation test missed its accuracy bounds with and without the fix).

Changes from v2
- v2: https://lore.kernel.org/20260924201615.44...@gmail.com
- Collect R-b: from SJ for patch 1.
- Rebase to latest mm-new.
- Add a note for starting point of test results and changelogs.
- Fix typos in patch 1: s/detault/default/ s/occationally/occasionally/.
- Update series subject prefix.
Changes from v1
- v1: https://lore.kernel.org/r/20260921003047.12...@gmail.com/
- Collect SJ's Reviewed-by for patch 2.
- Explain the running-context validation in the opening paragraph.
- Clarify which quota is passed as a parameter and use "commit" for the
context operation.
- Point Fixes to commit 60bd24f272d0 ("mm/damon/sysfs: test commit input
against realistic destination"). Drop the redundant stable version
comment.
- Rebase onto mm-new. No changes to the fix or test logic.

Karl Mehltretter (2):
mm/damon/core: preserve the quota passed to damon_new_scheme()
mm/damon/tests/core-kunit: test preservation of quota state

mm/damon/core.c | 6 +-
mm/damon/tests/core-kunit.h | 107 ++++++++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+), 3 deletions(-)


base-commit: 0fa0454423ab5bbe40db5783aab8ae29b40adf89
--
2.47.3

[SJ Park]

From: Karl Mehltretter <kmehlt...@gmail.com>

Check that damon_new_scheme() initializes the new scheme's quota without
changing the quota passed as a parameter. Cover all eight fields
initialized by damos_quota_init().

Also check that damon_commit_ctx() preserves those fields in the
destination scheme for both accepted and rejected parameter updates.
Use an invalid min_region_sz for the rejected update and confirm that
returning -EINVAL leaves the running quota state unchanged.

Without the preceding fix, all eight fields are cleared in the
constructor test and in both context update cases.

Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Bijan Tabatabai <bija...@gmail.com>
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehlt...@gmail.com>
Reviewed-by: SJ Park <s...@kernel.org>
Signed-off-by: SJ Park <s...@kernel.org>
---
mm/damon/tests/core-kunit.h | 107 ++++++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index caf582882f5f..2111faa58153 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -812,6 +812,48 @@ static void damos_test_new_filter(struct kunit *test)
damos_destroy_filter(filter);
}

+static void damos_test_new_scheme_keeps_src_quota(struct kunit *test)
+{
+ struct damos_access_pattern pattern = {};
+ struct damon_target target = {};
+ struct damos_quota quota = {
+ .sz = SZ_64K,
+ .esz = 123,
+ .esz_bp = 456,
+ .total_charged_sz = 789,
+ .total_charged_ns = 1011,
+ .charged_sz = 12,
+ .charged_from = 13,
+ .charge_target_from = &target,
+ .charge_addr_from = 14,
+ };
+ struct damos_watermarks wmarks = {};
+ struct damos *s;
+
+ s = damon_new_scheme(&pattern, DAMOS_STAT, 0, &quota, &wmarks,
+ NUMA_NO_NODE);
+ if (!s)
+ kunit_skip(test, "scheme alloc fail");
+ KUNIT_EXPECT_EQ(test, s->quota.sz, (unsigned long)SZ_64K);
+ KUNIT_EXPECT_EQ(test, s->quota.esz, 0ul);
+ KUNIT_EXPECT_EQ(test, s->quota.esz_bp, 0ul);
+ KUNIT_EXPECT_EQ(test, s->quota.total_charged_sz, 0ul);
+ KUNIT_EXPECT_EQ(test, s->quota.total_charged_ns, 0ul);
+ KUNIT_EXPECT_EQ(test, s->quota.charged_sz, 0ul);
+ KUNIT_EXPECT_EQ(test, s->quota.charged_from, 0ul);
+ KUNIT_EXPECT_PTR_EQ(test, s->quota.charge_target_from, NULL);
+ KUNIT_EXPECT_EQ(test, s->quota.charge_addr_from, 0ul);
+ KUNIT_EXPECT_EQ(test, quota.esz, 123ul);
+ KUNIT_EXPECT_EQ(test, quota.esz_bp, 456ul);
+ KUNIT_EXPECT_EQ(test, quota.total_charged_sz, 789ul);
+ KUNIT_EXPECT_EQ(test, quota.total_charged_ns, 1011ul);
+ KUNIT_EXPECT_EQ(test, quota.charged_sz, 12ul);
+ KUNIT_EXPECT_EQ(test, quota.charged_from, 13ul);
+ KUNIT_EXPECT_PTR_EQ(test, quota.charge_target_from, &target);
+ KUNIT_EXPECT_EQ(test, quota.charge_addr_from, 14ul);
+ damon_destroy_scheme(s);
+}
+
static void damos_test_commit_quota_goal_for(struct kunit *test,
struct damos_quota_goal *dst,
struct damos_quota_goal *src)
@@ -1614,6 +1656,69 @@ static void damon_test_commit_ctx(struct kunit *test)
damon_destroy_ctx(dst);
}

+static void damon_test_commit_ctx_keeps_quota_for(struct kunit *test,
+ unsigned long min_region_sz, int expected_err)
+{
+ struct damos_access_pattern pattern = {};
+ struct damos_quota quota = {.sz = SZ_64K};
+ struct damos_watermarks wmarks = {};
+ struct damon_ctx *src, *dst;
+ struct damon_target *target;
+ struct damos *s;
+
+ dst = damon_new_ctx();
+ if (!dst)
+ kunit_skip(test, "dst alloc fail");
+ target = damon_new_target();
+ if (!target) {
+ damon_destroy_ctx(dst);
+ kunit_skip(test, "target alloc fail");
+ }
+ damon_add_target(dst, target);
+ s = damon_new_scheme(&pattern, DAMOS_STAT, 0, &quota, &wmarks,
+ NUMA_NO_NODE);
+ if (!s) {
+ damon_destroy_ctx(dst);
+ kunit_skip(test, "scheme alloc fail");
+ }
+ damon_add_scheme(dst, s);
+
+ /* Copy the parameters before populating dst's runtime quota state. */
+ src = damon_new_test_ctx(dst);
+ if (!src) {
+ damon_destroy_ctx(dst);
+ kunit_skip(test, "src alloc fail");
+ }
+ src->min_region_sz = min_region_sz;
+ s->quota.esz = 123;
+ s->quota.esz_bp = 456;
+ s->quota.total_charged_sz = 789;
+ s->quota.total_charged_ns = 1011;
+ s->quota.charged_sz = 12;
+ s->quota.charged_from = 13;
+ s->quota.charge_target_from = target;
+ s->quota.charge_addr_from = 14;
+
+ KUNIT_EXPECT_EQ(test, damon_commit_ctx(dst, src), expected_err);
+ KUNIT_EXPECT_EQ(test, s->quota.esz, 123ul);
+ KUNIT_EXPECT_EQ(test, s->quota.esz_bp, 456ul);
+ KUNIT_EXPECT_EQ(test, s->quota.total_charged_sz, 789ul);
+ KUNIT_EXPECT_EQ(test, s->quota.total_charged_ns, 1011ul);
+ KUNIT_EXPECT_EQ(test, s->quota.charged_sz, 12ul);
+ KUNIT_EXPECT_EQ(test, s->quota.charged_from, 13ul);
+ KUNIT_EXPECT_PTR_EQ(test, s->quota.charge_target_from, target);
+ KUNIT_EXPECT_EQ(test, s->quota.charge_addr_from, 14ul);
+ damon_destroy_ctx(src);
+ damon_destroy_ctx(dst);
+}
+
+static void damon_test_commit_ctx_keeps_quota(struct kunit *test)
+{
+ /* Only power of two min_region_sz is allowed. */
+ damon_test_commit_ctx_keeps_quota_for(test, 4096, 0);
+ damon_test_commit_ctx_keeps_quota_for(test, 4095, -EINVAL);
+}
+
static void damon_test_valid_probe_params(struct kunit *test)
{
struct damon_ctx *ctx;
@@ -2348,6 +2453,7 @@ static struct kunit_case damon_test_cases[] = {
KUNIT_CASE(damon_test_mvsum),
KUNIT_CASE(damon_test_nr_accesses_mvsum),
KUNIT_CASE(damos_test_new_filter),
+ KUNIT_CASE(damos_test_new_scheme_keeps_src_quota),
KUNIT_CASE(damos_test_commit_quota_goal),
KUNIT_CASE(damos_test_set_psi_current_val),
KUNIT_CASE(damos_test_commit_quota_goals),
@@ -2360,6 +2466,7 @@ static struct kunit_case damon_test_cases[] = {
KUNIT_CASE(damon_test_commit_filter),
KUNIT_CASE(damon_test_commit_probes),
KUNIT_CASE(damon_test_commit_ctx),
+ KUNIT_CASE(damon_test_commit_ctx_keeps_quota),
KUNIT_CASE(damon_test_valid_probe_params),
KUNIT_CASE(damos_test_filter_out),
KUNIT_CASE(damos_test_apply_scheme_filtered_sz),
--
2.47.3