mm/damon/core: preserve quota state when constructing schemes

PATCH Applied SJ Park 1 participant 3 messages Sep 28

Subsystems (4)

Stable branch Memory management Kselftest DAMON

lore

Share & tools

lore

SJ Park

author maintainer

2026-09-28 08:58 UTC #0 lore raw

v3 cover mm/damon/core: preserve quota state when constructing schemes

From: Karl Mehltretter <[email protected]>

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/[email protected]
- 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/[email protected]/
- 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

Signature · 1 line

2.47.3

SJ Park

author maintainer

2026-09-28 08:58 UTC #1 lore raw

PATCH v3 1/2 mm/damon/core: preserve the quota passed to damon_new_scheme()

From: Karl Mehltretter <[email protected]>

damon_commit_ctx() first commits the running context's parameters to a temporary context for validating proposed updates. When damon_commit_schemes() creates the temporary schemes, it passes the running scheme's quota as the quota parameter of damon_new_scheme().

damon_new_scheme() calls damos_quota_init() on that quota before copying it to the new scheme. This clears the running scheme's effective quota, feedback input and charging state. Even an update rejected with -EINVAL loses the running quota state.

For a size quota, this discards the bytes already charged and allows the scheme to use a fresh quota before the reset interval has elapsed. For a goal-driven quota, the consist tuner loses its accumulated input and restarts from its minimum input. A time quota loses its throughput estimate and falls back to the initial estimate.

The end users will show DAMOS works more or less aggressively than expected for online-commit updates of quotas. DAMON provides best efforts by default. DAMON parameters online commit is supposed to be executed only occasionally. Hence, the issue wouldn't be critical on sane setups. For user_input type quota goals, online commit of the user input score is expected to be frequent. But, for the case commit_schemes_quota_goals command is recommended for optimal execution, and it doesn't have this bug.

Commit 60bd24f272d0 ("mm/damon/sysfs: test commit input against realistic destination") introduced this problem in v6.19 when sysfs validation began committing the running context's parameters to a temporary context. Commit b90408ef1163 ("mm/damon/core: safely validate src on damon_commit_ctx()") later moved that validation into the core API, exposing other callers including DAMON_RECLAIM and DAMON_LRU_SORT.

Sashiko reported the same side effect [1] on the RFC of the core API change.

Copy the quota to the new scheme first, then initialize that copy. Make damos_quota_init() return void, since its return value is no longer needed.

Link https://lore.kernel.org/r/[email protected]/ [1] open here

Fixes 60bd24f272d0 ("mm/damon/sysfs: test commit input against realistic destination")

Cc <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Bijan Tabatabai <[email protected]>

Assisted-by LLM

Signed-off-by Karl Mehltretter <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

mm/damon/core.c +3 -3

index 60e4233ed23c..5ecbea5d71e1 100644
@@ -734,7 +734,7 @@ static bool damos_quota_goals_empty(struct damos_quota *q)
 }
 
 /* initialize fields of @quota that normally API users wouldn't set */
-static struct damos_quota *damos_quota_init(struct damos_quota *quota)
+static void damos_quota_init(struct damos_quota *quota)
 {
 	quota->esz = 0;
 	quota->total_charged_sz = 0;

@@ -744,7 +744,6 @@ static struct damos_quota *damos_quota_init(struct damos_quota *quota)
 	quota->charge_target_from = NULL;
 	quota->charge_addr_from = 0;
 	quota->esz_bp = 0;
-	return quota;
 }
 
 struct damos *damon_new_scheme(struct damos_access_pattern *pattern,

@@ -776,7 +775,8 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 	scheme->last_applied = NULL;
 	INIT_LIST_HEAD(&scheme->list);
 
-	scheme->quota = *(damos_quota_init(quota));
+	scheme->quota = *quota;
+	damos_quota_init(&scheme->quota);
 	/* quota.goals should be separately set by caller */
 	INIT_LIST_HEAD(&scheme->quota.goals);

Signature · 1 line

2.47.3

SJ Park

author maintainer

2026-09-28 08:58 UTC #2 lore raw

PATCH v3 2/2 mm/damon/tests/core-kunit: test preservation of quota state

In reply to SJ Park

From: Karl Mehltretter <[email protected]>

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 <[email protected]>

Cc Bijan Tabatabai <[email protected]>

Assisted-by LLM

Signed-off-by Karl Mehltretter <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/tests/core-kunit.h | 107 ++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

mm/damon/tests/core-kunit.h +107 -0

index caf582882f5f..2111faa58153 100644
@@ -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),

Signature · 1 line

2.47.3