Skip to contentSkip to conversation

mm/damon: improve readability, clarity and test coverage

PATCH Applied SJ Park 5 participants 31 messages Sep 17 – Sep 20

Subsystems (5)
lore
Share & tools
lore

SJ Park

author maintainer
#0 lore raw

cover mm/damon: improve readability, clarity and test coverage

Yet another batch of miscellaneous DAMON minor improvements. Mostly focused on readability and clarity of code and document, and unit/self test coverage. No user-visible behavioral change is intended.

Changes from RFC v2
- RFC v2: https://lore.kernel.org/[email protected]
- Drop RFC tag.
- Rebase to the latest mm-new.
Changes from RFC
- RFC: https://lore.kernel.org/[email protected]
- Fix a typo in the selftest warning message.
- Collect A-b: from Randy.
- Split out damos_adjust_quota() fix to an individual hotfix patch.
- Add a patch for initializing walk_control_obsolete in damon_new_ctx().
- Add a patch for probe_hits_wsum damos filter commit kunit test.
- Trivial patch sequence update.
- Rebase to the latest mm-new.

SJ Park (10):
mm/damon/api: remove NR_DAMOS_FILTER_TYPES
mm/damon/core: use abs_diff() in damon_feed_loop_next_input() mm/damon/core: use mult_frac() in damon_feed_loop_next_input() mm/damon/core: set damon_ctx->walk_control_obsolete in damon_new_ctx() mm/damon/core: document damon_call()/damon_start() race hang issue mm/damon/paddr: remove pa parameter from damon_pa_filter_pass() mm/damon/tests/core-kunit: test eligible_mem_bp commitment mm/damon/tests/core-kunit: add probe_hits_wsum damos filter commit
test
selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak Docs/mm/damon/design: clarify bp is basis point

 Documentation/mm/damon/design.rst             |  5 ++--
 include/linux/damon.h                         |  2 --
 mm/damon/core.c                               | 19 ++++--------
 mm/damon/paddr.c                              |  5 ++--
 mm/damon/tests/core-kunit.h                   | 30 +++++++++++++++++++
 .../selftests/damon/sysfs_memcg_path_leak.sh  |  7 +++++
 6 files changed, 48 insertions(+), 20 deletions(-)

base-commit: 942682790e570c9422a389bf34097e74c4777fe9

Signature · 1 line
2.47.3
author maintainer
#1 lore raw

PATCH 1/10 mm/damon/api: remove NR_DAMOS_FILTER_TYPES

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

---
 include/linux/damon.h | 2 --
 1 file changed, 2 deletions(-)
include/linux/damon.h +0 -2
index bbb190b474015..836353c4ab9aa 100644
@@ -400,7 +400,6 @@ struct damos_stat {
  * @DAMOS_FILTER_TYPE_ADDR:	Address range.
  * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
  * @DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:	probe_hits weighted sum range.
- * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
  *
  * All types except &DAMOS_FILTER_TYPE_ADDR, &DAMOS_FILTER_TYPE_TARGET and
  * &DAMOS_FILTER_TYPE_PROBE_HITS_WSUM are handled by the underlying &struct
@@ -422,7 +421,6 @@ enum damos_filter_type {
 	DAMOS_FILTER_TYPE_ADDR,
 	DAMOS_FILTER_TYPE_TARGET,
 	DAMOS_FILTER_TYPE_PROBE_HITS_WSUM,
-	NR_DAMOS_FILTER_TYPES,
 };
 
 /**
Signature · 1 line
2.47.3
#2 lore raw

Re: [PATCH 01/10] mm/damon/api: remove NR_DAMOS_FILTER_TYPES

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 3 lines

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Reviewed-by on v1 1/10
#3 lore raw

Re: [PATCH 01/10] mm/damon/api: remove NR_DAMOS_FILTER_TYPES

In reply to SJ Park

Hi SJ,

Quoted 20 lines · SJ Park

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

---
 include/linux/damon.h | 2 --
 1 file changed, 2 deletions(-)
include/linux/damon.h +0 -1
index bbb190b474015..836353c4ab9aa 100644
@@ -400,7 +400,6 @@ struct damos_stat {
  * @DAMOS_FILTER_TYPE_ADDR:	Address range.
  * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
  * @DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:	probe_hits weighted sum range.
- * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
  *
  * All types except &DAMOS_FILTER_TYPE_ADDR, &DAMOS_FILTER_TYPE_TARGET and
  * &DAMOS_FILTER_TYPE_PROBE_HITS_WSUM are handled by the underlying &struct

include/linux/damon.h enum damos_filter_type { · line 426

Quoted 7 lines
include/linux/damon.h +0 -1
@@ -422,7 +421,6 @@ enum damos_filter_type {
 	DAMOS_FILTER_TYPE_ADDR,
 	DAMOS_FILTER_TYPE_TARGET,
 	DAMOS_FILTER_TYPE_PROBE_HITS_WSUM,
-	NR_DAMOS_FILTER_TYPES,
 };
 
 /**
Signature · 2 lines
Asier Gutierrez
Huawei
#4 lore raw

Re: [PATCH 01/10] mm/damon/api: remove NR_DAMOS_FILTER_TYPES

In reply to SJ Park
Quoted 20 lines · SJ Park

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

---
 include/linux/damon.h | 2 --
 1 file changed, 2 deletions(-)
include/linux/damon.h +0 -1
index bbb190b474015..836353c4ab9aa 100644
@@ -400,7 +400,6 @@ struct damos_stat {
  * @DAMOS_FILTER_TYPE_ADDR:	Address range.
  * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
  * @DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:	probe_hits weighted sum range.
- * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
  *
  * All types except &DAMOS_FILTER_TYPE_ADDR, &DAMOS_FILTER_TYPE_TARGET and
  * &DAMOS_FILTER_TYPE_PROBE_HITS_WSUM are handled by the underlying &struct

include/linux/damon.h enum damos_filter_type { · line 426

Quoted 7 lines
include/linux/damon.h +0 -1
@@ -422,7 +421,6 @@ enum damos_filter_type {
 	DAMOS_FILTER_TYPE_ADDR,
 	DAMOS_FILTER_TYPE_TARGET,
 	DAMOS_FILTER_TYPE_PROBE_HITS_WSUM,
-	NR_DAMOS_FILTER_TYPES,
 };
 
 /**

Similarly we have more definitions of NR_foo in damon.h:

* @NR_DAMOS_ACTIONS: Total number of DAMOS actions
* @NR_DAMOS_QUOTA_GOAL_METRICS: Number of DAMOS quota goal metrics.
* @NR_DAMOS_WMARK_METRICS: Total number of DAMOS watermark metrics
* @NR_DAMON_OPS: Number of monitoring operations implementations

and only NR_DAMON_OPS is actually used (I checked mm-new). If the preference is removing all unused macros, we should probably clean them up as well.

Thanks,
Zenghui

author maintainer
#5 lore raw

Re: [PATCH 01/10] mm/damon/api: remove NR_DAMOS_FILTER_TYPES

Quoted 2 lines · On Sat, 19 Sep 2026 21:02:30 +0800 Zenghui Yu <[email protected]> wrote:
Quoted 1 line · SJ Park

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

[...]

Quoted 10 lines

Similarly we have more definitions of NR_foo in damon.h:

* @NR_DAMOS_ACTIONS: Total number of DAMOS actions
* @NR_DAMOS_QUOTA_GOAL_METRICS: Number of DAMOS quota goal metrics.
* @NR_DAMOS_WMARK_METRICS: Total number of DAMOS watermark metrics
* @NR_DAMON_OPS: Number of monitoring operations implementations

and only NR_DAMON_OPS is actually used (I checked mm-new). If the preference is removing all unused macros, we should probably clean them up as well.

Nice findings! Please feel free to post a patch for cleaning those up if you don't mind!

Thanks,
SJ

[...]

#6 lore raw

Re: [PATCH 01/10] mm/damon/api: remove NR_DAMOS_FILTER_TYPES

Quoted 12 lines · SJ Park
Quoted 2 lines · On Sat, 19 Sep 2026 21:02:30 +0800 Zenghui Yu <[email protected]> wrote:
Quoted 1 line · SJ Park

Nobody uses NR_DAMOS_FILTER_TYPES. Remove it.

[...]

Quoted 7 lines

Similarly we have more definitions of NR_foo in damon.h:

* @NR_DAMOS_ACTIONS: Total number of DAMOS actions
* @NR_DAMOS_QUOTA_GOAL_METRICS: Number of DAMOS quota goal metrics.
* @NR_DAMOS_WMARK_METRICS: Total number of DAMOS watermark metrics
* @NR_DAMON_OPS: Number of monitoring operations implementations

Quoted 6 lines
Quoted 3 lines

and only NR_DAMON_OPS is actually used (I checked mm-new). If the preference is removing all unused macros, we should probably clean them up as well.

Nice findings! Please feel free to post a patch for cleaning those up if you don't mind!

Sent out now.

https://lore.kernel.org/damon/[email protected]

Thanks,
Zenghui

author maintainer
#7 lore raw

PATCH 2/10 mm/damon/core: use abs_diff() in damon_feed_loop_next_input()

In reply to SJ Park

damon_feed_loop_next_input() is open-coding absolute diff calculation instead of the dedicated helper, abs_diff(), for no good reason. Use the dedicated helper.

---
 mm/damon/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
mm/damon/core.c +1 -4
index 444c97c184027..de46072dc1a2f 100644
@@ -2926,10 +2926,7 @@ static unsigned long damon_feed_loop_next_input(unsigned long last_input,
 	if (score >= goal * 2)
 		return min_input;
 
-	if (over_achieving)
-		score_goal_diff = score - goal;
-	else
-		score_goal_diff = goal - score;
+	score_goal_diff = abs_diff(score, goal);
 
 	if (last_input < ULONG_MAX / score_goal_diff)
 		compensation = last_input * score_goal_diff / goal;
Signature · 1 line
2.47.3
#8 lore raw

Re: [PATCH 02/10] mm/damon/core: use abs_diff() in damon_feed_loop_next_input()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 5 lines

damon_feed_loop_next_input() is open-coding absolute diff calculation instead of the dedicated helper, abs_diff(), for no good reason. Use the dedicated helper.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Reviewed-by on v1 2/10
#9 lore raw

Re: [PATCH 02/10] mm/damon/core: use abs_diff() in damon_feed_loop_next_input()

In reply to SJ Park
Quoted 15 lines · SJ Park

damon_feed_loop_next_input() is open-coding absolute diff calculation instead of the dedicated helper, abs_diff(), for no good reason. Use the dedicated helper.

---
 mm/damon/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
mm/damon/core.c +0 -0
index 444c97c184027..de46072dc1a2f 100644
@@ -2926,10 +2926,7 @@ static unsigned long damon_feed_loop_next_input(unsigned long last_input,
 	if (score >= goal * 2)

mm/damon/core.c static unsigned long damon_feed_loop_next_input(unsigned long last_input, · line 2932

Quoted 10 lines
mm/damon/core.c +1 -4
 		return min_input;
 
-	if (over_achieving)
-		score_goal_diff = score - goal;
-	else
-		score_goal_diff = goal - score;
+	score_goal_diff = abs_diff(score, goal);
 
 	if (last_input < ULONG_MAX / score_goal_diff)
 		compensation = last_input * score_goal_diff / goal;

LGTM

Signature · 2 lines
Asier Gutierrez
Huawei
author maintainer
#10 lore raw

PATCH 3/10 mm/damon/core: use mult_frac() in damon_feed_loop_next_input()

In reply to SJ Park

damon_feed_loop_next_input() does its best effort overflow protection. score_goal_diff is always smaller than goal (10,000). Hence the calculation can be replaced to use mult_frac() without concerning the overflow. Use mult_frac().

---
 mm/damon/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
mm/damon/core.c +1 -5
index de46072dc1a2f..bc41ac7ecca43 100644
@@ -2927,11 +2927,7 @@ static unsigned long damon_feed_loop_next_input(unsigned long last_input,
 		return min_input;
 
 	score_goal_diff = abs_diff(score, goal);
-
-	if (last_input < ULONG_MAX / score_goal_diff)
-		compensation = last_input * score_goal_diff / goal;
-	else
-		compensation = last_input / goal * score_goal_diff;
+	compensation = mult_frac(last_input, score_goal_diff, goal);
 
 	if (over_achieving)
 		return max(last_input - compensation, min_input);
Signature · 1 line
2.47.3
#11 lore raw

Re: [PATCH 03/10] mm/damon/core: use mult_frac() in damon_feed_loop_next_input()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 6 lines

damon_feed_loop_next_input() does its best effort overflow protection. score_goal_diff is always smaller than goal (10,000). Hence the calculation can be replaced to use mult_frac() without concerning the overflow. Use mult_frac().

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Reviewed-by on v1 3/10
#12 lore raw

Re: [PATCH 03/10] mm/damon/core: use mult_frac() in damon_feed_loop_next_input()

In reply to SJ Park
Quoted 17 lines · SJ Park

damon_feed_loop_next_input() does its best effort overflow protection. score_goal_diff is always smaller than goal (10,000). Hence the calculation can be replaced to use mult_frac() without concerning the overflow. Use mult_frac().

---
 mm/damon/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
mm/damon/core.c +0 -0
index de46072dc1a2f..bc41ac7ecca43 100644
@@ -2927,11 +2927,7 @@ static unsigned long damon_feed_loop_next_input(unsigned long last_input,
 		return min_input;
 

mm/damon/core.c static unsigned long damon_feed_loop_next_input(unsigned long last_input, · line 2933

Quoted 10 lines
mm/damon/core.c +1 -5
 	score_goal_diff = abs_diff(score, goal);
-
-	if (last_input < ULONG_MAX / score_goal_diff)
-		compensation = last_input * score_goal_diff / goal;
-	else
-		compensation = last_input / goal * score_goal_diff;
+	compensation = mult_frac(last_input, score_goal_diff, goal);
 
 	if (over_achieving)
 		return max(last_input - compensation, min_input);

LGTM

Signature · 2 lines
Asier Gutierrez
Huawei
author maintainer
#13 lore raw

PATCH 4/10 mm/damon/core: set damon_ctx->walk_control_obsolete in damon_new_ctx()

In reply to SJ Park

damos_walk() should be called for a damon_ctx context that has successfully started at least once. That's because damon_ctx->walk_control_obsolete is initialized when kdamond starts. If the rule is violated, an indefinite wait can happen. There is no existing violation of the rule.

damon_call() had a similar rule, and it turned out keeping the rule is not easy for damon_call()'s case. Hence, commit 8023b5f47e09 ("mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()") added the initialization in damon_new_ctx() and removed the rule.

Keeping the rule for damos_walk() is relatively easier. But having slightly different rules for similar functions could be confusing. Sashiko, for example, repeatedly asked questions about this. Do the initialization of walk_control_obsolete in damon_new_ctx() for consistency.

[1] https://lore.kernel.org/[email protected]

---
 mm/damon/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
mm/damon/core.c +1 -4
index bc41ac7ecca43..c63b09d4a2d0d 100644
@@ -939,6 +939,7 @@ struct damon_ctx *damon_new_ctx(void)
 	INIT_LIST_HEAD(&ctx->schemes);
 
 	ctx->call_controls_obsolete = true;
+	ctx->walk_control_obsolete = true;
 	prandom_seed_state(&ctx->rnd_state, get_random_u64());
 
 	return ctx;
@@ -2308,10 +2309,6 @@ int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
  * passed at least one &damos->apply_interval_us, kdamond marks the request as
  * completed so that damos_walk() can wakeup and return.
  *
- * Note that this function should be called only after damon_start() with the
- * @ctx has succeeded.  Otherwise, this function could fall into an indefinite
- * wait.
- *
  * Return: 0 on success, negative error code otherwise.
  */
 int damos_walk(struct damon_ctx *ctx, struct damos_walk_control *control)
Signature · 1 line
2.47.3
#14 lore raw

Re: [PATCH 04/10] mm/damon/core: set damon_ctx->walk_control_obsolete in damon_new_ctx()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 17 lines

damos_walk() should be called for a damon_ctx context that has successfully started at least once. That's because damon_ctx->walk_control_obsolete is initialized when kdamond starts. If the rule is violated, an indefinite wait can happen. There is no existing violation of the rule.

damon_call() had a similar rule, and it turned out keeping the rule is not easy for damon_call()'s case. Hence, commit 8023b5f47e09 ("mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()") added the initialization in damon_new_ctx() and removed the rule.

Keeping the rule for damos_walk() is relatively easier. But having slightly different rules for similar functions could be confusing. Sashiko, for example, repeatedly asked questions about this. Do the initialization of walk_control_obsolete in damon_new_ctx() for consistency.

Quoted 3 lines

[1] https://lore.kernel.org/[email protected]

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
author maintainer
#15 lore raw

PATCH 5/10 mm/damon/core: document damon_call()/damon_start() race hang issue

In reply to SJ Park

Let's suppose damon_start() and damon_call() are executed in parallel for the same DAMON context. Then, damon_call() could show ctx->damon_calls_obsolete set while ctx->kdamond is unset. If damon_start() sets ctx->kdamond before damon_call() starts the cancelling, damon_call() can indefinitely hang. No DAMON API caller does such parallel execution of damon_start() and damon_call(), so the issue doesn't exist. But who knows what will happen in future. Add a clarification comment for caution.

---
 mm/damon/core.c | 3 +++
 1 file changed, 3 insertions(+)
mm/damon/core.c +3 -0
index c63b09d4a2d0d..2258b72da7a78 100644
@@ -2258,6 +2258,9 @@ int damon_kdamond_pid(struct damon_ctx *ctx)
  *
  * When this function is failed, the @ctx is guaranteed to be stopped.
  *
+ * This function should not be called in parallel to damon_start() for the
+ * @ctx.  In the case, this function could indefinitely hang.
+ *
  * Return: 0 on success, negative error code otherwise.
  */
 int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
Signature · 1 line
2.47.3
#16 lore raw

Re: [PATCH 05/10] mm/damon/core: document damon_call()/damon_start() race hang issue

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 10 lines

Let's suppose damon_start() and damon_call() are executed in parallel for the same DAMON context. Then, damon_call() could show ctx->damon_calls_obsolete set while ctx->kdamond is unset. If damon_start() sets ctx->kdamond before damon_call() starts the cancelling, damon_call() can indefinitely hang. No DAMON API caller does such parallel execution of damon_start() and damon_call(), so the issue doesn't exist. But who knows what will happen in future. Add a clarification comment for caution.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
author maintainer
#17 lore raw

PATCH 6/10 mm/damon/paddr: remove pa parameter from damon_pa_filter_pass()

In reply to SJ Park

damon_pa_filter_pass() receives the 'pa' parameter, but doesn't use it. Remove the parameter from the function signature.

---
 mm/damon/paddr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/damon/paddr.c +2 -3
index 5abfabaa339e0..2cfdc356b4157 100644
@@ -163,8 +163,7 @@ static bool damon_pa_filter_match(struct damon_filter *filter,
 	return matched == filter->matching;
 }
 
-static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
-		struct damon_probe *p)
+static bool damon_pa_filter_pass(struct folio *folio, struct damon_probe *p)
 {
 	struct damon_filter *f;
 	bool pass = true;
@@ -200,7 +199,7 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
 					ctx->addr_unit);
 			folio = damon_get_folio(PHYS_PFN(pa));
 			damon_for_each_probe(p, ctx) {
-				if (damon_pa_filter_pass(pa, folio, p))
+				if (damon_pa_filter_pass(folio, p))
 					r->probe_hits[i]++;
 				i++;
 			}
Signature · 1 line
2.47.3
#18 lore raw

Re: [PATCH 06/10] mm/damon/paddr: remove pa parameter from damon_pa_filter_pass()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 4 lines

damon_pa_filter_pass() receives the 'pa' parameter, but doesn't use it. Remove the parameter from the function signature.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
author maintainer
#19 lore raw

PATCH 7/10 mm/damon/tests/core-kunit: test eligible_mem_bp commitment

In reply to SJ Park

There was a DAMOS quota goal commit bug [1] that doesn't update the nid field for DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP metric goal. Add a kunit test case for confirming nid commitment.

[1] https://lore.kkernel.org/[email protected]

---
 mm/damon/tests/core-kunit.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
mm/damon/tests/core-kunit.h +10 -0
index 5da84caf4124d..1f19fefdd98c3 100644
@@ -835,6 +835,9 @@ static void damos_test_commit_quota_goal_for(struct kunit *test,
 		KUNIT_EXPECT_EQ(test, dst->nid, src->nid);
 		KUNIT_EXPECT_EQ(test, dst->memcg_id, src->memcg_id);
 		break;
+	case DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP:
+		KUNIT_EXPECT_EQ(test, dst->nid, src->nid);
+		break;
 	default:
 		break;
 	}
@@ -898,6 +901,13 @@ static void damos_test_commit_quota_goal(struct kunit *test)
 			.current_value = 345,
 			.last_psi_total = 567,
 			});
+	damos_test_commit_quota_goal_for(test, &dst,
+			&(struct damos_quota_goal){
+			.metric = DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP,
+			.target_value = 12,
+			.current_value = 345,
+			.nid = 6,
+			});
 }
 
 static void damos_test_commit_quota_goals_for(struct kunit *test,
Signature · 1 line
2.47.3
#20 lore raw

Re: [PATCH 07/10] mm/damon/tests/core-kunit: test eligible_mem_bp commitment

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 7 lines

There was a DAMOS quota goal commit bug [1] that doesn't update the nid field for DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP metric goal. Add a kunit test case for confirming nid commitment.

[1] https://lore.kkernel.org/[email protected]

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
author maintainer
#21 lore raw

PATCH 8/10 mm/damon/tests/core-kunit: add probe_hits_wsum damos filter commit test

In reply to SJ Park

DAMOS filter commit kunit test lacks test cases for probe_hits_wsum filter type. Add test cases for probe_hits_wsum type DAMOS filter commit.

---
 mm/damon/tests/core-kunit.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
mm/damon/tests/core-kunit.h +20 -0
index 1f19fefdd98c3..5ff0436c58441 100644
@@ -1166,6 +1166,10 @@ static void damos_test_commit_filter_for(struct kunit *test,
 		KUNIT_EXPECT_EQ(test, dst->sz_range.min, src->sz_range.min);
 		KUNIT_EXPECT_EQ(test, dst->sz_range.max, src->sz_range.max);
 		break;
+	case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
+		KUNIT_EXPECT_EQ(test, dst->range_min, src->range_min);
+		KUNIT_EXPECT_EQ(test, dst->range_max, src->range_max);
+		break;
 	default:
 		break;
 	}
@@ -1239,6 +1243,22 @@ static void damos_test_commit_filter(struct kunit *test)
 			.allow = true,
 			.target_idx = 6,
 			}, false);
+	damos_test_commit_filter_for(test, &dst,
+			&(struct damos_filter){
+			.type = DAMOS_FILTER_TYPE_PROBE_HITS_WSUM,
+			.matching = false,
+			.allow = true,
+			.range_min = 12,
+			.range_max = 34,
+			}, false);
+	damos_test_commit_filter_for(test, &dst,
+			&(struct damos_filter){
+			.type = DAMOS_FILTER_TYPE_PROBE_HITS_WSUM,
+			.matching = false,
+			.allow = true,
+			.range_min = 34,
+			.range_max = 12,
+			}, true);
 }
 
 static void damos_test_help_initailize_scheme(struct damos *scheme)
Signature · 1 line
2.47.3
#22 lore raw

Re: [PATCH 08/10] mm/damon/tests/core-kunit: add probe_hits_wsum damos filter commit test

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 5 lines

DAMOS filter commit kunit test lacks test cases for probe_hits_wsum filter type. Add test cases for probe_hits_wsum type DAMOS filter commit.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
author maintainer
#23 lore raw

PATCH 9/10 selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak

In reply to SJ Park

The selftest can fail for any leak if it happens while the test is running. Remove the false positive test failures by further checking if the expected leaking function is called out on the report.

---
 tools/testing/selftests/damon/sysfs_memcg_path_leak.sh | 7 +++++++
 1 file changed, 7 insertions(+)
tools/testing/selftests/damon/sysfs_memcg_path_leak.sh +7 -0
index 33a7ff43ed6cc..34c37129c49fe 100755
@@ -41,5 +41,12 @@ if [ "$kmemleak_report" = "" ]
 then
 	exit 0
 fi
+if ! echo "$kmemleak_report" | grep "memcg_path_store" --quiet
+then
+	echo "[WARN] memleak found; apparently not from DAMON, though"
+	echo "$kmemleak_report"
+	exit 0
+fi
+
 echo "$kmemleak_report"
 exit 1
Signature · 1 line
2.47.3
#24 lore raw

Re: [PATCH 09/10] selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/damon: test falsely passes on minimal grep due to --quiet flag --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/damon: test falsely passes on minimal grep due to --quiet flag --

commit 3a025795f6efb7b8edc7858f005049159a83efbe
Author: SJ Park <[email protected]>

selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak

This commit updates the DAMON sysfs_memcg_path_leak selftest to explicitly check the kmemleak report for "memcg_path_store". It exits with success if the leak doesn't appear to be related to DAMON, reducing false positive failures.

tools/testing/selftests/damon/sysfs_memcg_path_leak.sh if [ "$kmemleak_report" = "" ] · line 44

Quoted 8 lines
tools/testing/selftests/damon/sysfs_memcg_path_leak.sh +1 -0
@@ -41,5 +41,12 @@ if [ "$kmemleak_report" = "" ]
 then
 	exit 0
 fi
+if ! echo "$kmemleak_report" | grep "memcg_path_store" --quiet

[Severity: Medium]
Will this script falsely pass in environments with a minimal grep implementation?

If this selftest is executed in an environment (like some BusyBox builds or Alpine Linux) where grep does not support the GNU --quiet extension, grep will exit with an error code.

The ! operator will negate this non-zero exit code, causing the condition to evaluate to true. The script will then enter the then block and exit successfully (exit 0), potentially masking real kernel memory leaks.

Could this use -q instead of --quiet to be more portable?

Quoted 8 lines
tools/testing/selftests/damon/sysfs_memcg_path_leak.sh +6 -0
+then
+	echo "[WARN] memleak found; apparently not from DAMON, though"
+	echo "$kmemleak_report"
+	exit 0
+fi
+
 echo "$kmemleak_report"
 exit 1
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
author maintainer
#25 lore raw

Re: [PATCH 09/10] selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak

Quoted 30 lines · On Thu, 17 Sep 2026 14:31:54 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/damon: test falsely passes on minimal grep due to --quiet flag --

commit 3a025795f6efb7b8edc7858f005049159a83efbe
Author: SJ Park <[email protected]>

selftests/damon/sysfs_memcg_path_leak: fail only for real DAMON leak

This commit updates the DAMON sysfs_memcg_path_leak selftest to explicitly check the kmemleak report for "memcg_path_store". It exits with success if the leak doesn't appear to be related to DAMON, reducing false positive failures.

tools/testing/selftests/damon/sysfs_memcg_path_leak.sh if [ "$kmemleak_report" = "" ] · line 44

Quoted 8 lines
tools/testing/selftests/damon/sysfs_memcg_path_leak.sh +1 -0
@@ -41,5 +41,12 @@ if [ "$kmemleak_report" = "" ]
 then
 	exit 0
 fi
+if ! echo "$kmemleak_report" | grep "memcg_path_store" --quiet

[Severity: Medium]
Will this script falsely pass in environments with a minimal grep implementation?

If this selftest is executed in an environment (like some BusyBox builds or Alpine Linux) where grep does not support the GNU --quiet extension, grep will exit with an error code.

Quoted 5 lines

The ! operator will negate this non-zero exit code, causing the condition to evaluate to true. The script will then enter the then block and exit successfully (exit 0), potentially masking real kernel memory leaks.

Could this use -q instead of --quiet to be more portable?

I believe modern test infrastructures support --quiet. So no blocker. We can revisit this if there is any report.

Quoted 12 lines
Quoted 9 lines
tools/testing/selftests/damon/sysfs_memcg_path_leak.sh +6 -0
+then
+	echo "[WARN] memleak found; apparently not from DAMON, though"
+	echo "$kmemleak_report"
+	exit 0
+fi
+
 echo "$kmemleak_report"
 exit 1
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Thanks,
SJ

author maintainer
#26 lore raw

PATCH 10/10 Docs/mm/damon/design: clarify bp is basis point

In reply to SJ Park

DAMON design document uses "bp" for "basis point" in multiple places. Because it is not clearly mentioned, it is difficult to understand what "bp" stands for. Add the clarification.

---
 Documentation/mm/damon/design.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Documentation/mm/damon/design.rst +3 -2
index 0a86792f90a18..e82390e77a70a 100644
@@ -445,7 +445,7 @@ users to set the aimed amount of access events to observe via DAMON within
 given time interval.  The target can be specified by the user as a ratio of
 DAMON-observed access events to the theoretical maximum amount of the events
 (``access_bp``) that measured within a given number of aggregations
-(``aggrs``).
+(``aggrs``).  The ratio is in basis point (bp or 1/10,000).
 
 The DAMON-observed access events are calculated in byte granularity based on
 DAMON :ref:`region assumption <damon_design_region_based_sampling>`.  For
@@ -717,7 +717,8 @@ mechanism tries to make ``current_value`` of ``target_metric`` be same to
   in microseconds that measured from last quota reset to next quota reset.
   DAMOS does the measurement on its own, so only ``target_value`` need to be
   set by users at the initial time.  In other words, DAMOS does self-feedback.
-- ``node_mem_used_bp``: Specific NUMA node's used memory ratio in bp (1/10,000).
+- ``node_mem_used_bp``: Specific NUMA node's used memory ratio in basis point
+  (bp or 1/10,000).
 - ``node_mem_free_bp``: Specific NUMA node's free memory ratio in bp (1/10,000).
 - ``node_memcg_used_bp``: Specific cgroup's node used memory ratio for a
   specific NUMA node, in bp (1/10,000).
Signature · 1 line
2.47.3
#27 lore raw

Re: [PATCH 10/10] Docs/mm/damon/design: clarify bp is basis point

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 8 lines

DAMON design document uses "bp" for "basis point" in multiple places. Because it is not clearly mentioned, it is difficult to understand what "bp" stands for. Add the clarification.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
Acked-by on v1 10/10
#28 lore raw

Re: [PATCH 10/10] Docs/mm/damon/design: clarify bp is basis point

In reply to SJ Park
Quoted 3 lines · SJ Park

DAMON design document uses "bp" for "basis point" in multiple places. Because it is not clearly mentioned, it is difficult to understand what "bp" stands for. Add the clarification.

Exactly ;-) I had such confusion when I was reading the doc.

Quoted 5 lines

Thanks,
Zenghui

author maintainer
#29 lore raw
In reply to SJ Park
Quoted 3 lines · On Thu, 17 Sep 2026 07:21:58 -0700 SJ Park <[email protected]> wrote:

Yet another batch of miscellaneous DAMON minor improvements. Mostly focused on readability and clarity of code and document, and unit/self test coverage. No user-visible behavioral change is intended.

Sashiko found no blocker for this series. Sashiko send its findings to damon@ mailing list [1], and I reply those if anything needs to be clarified. Please refer to those for details.

[1] https://lore.kernel.org/damon/

Thanks,
SJ

maintainer
#30 lore raw
In reply to SJ Park
Quoted 3 lines · On Thu, 17 Sep 2026 07:21:58 -0700 SJ Park <[email protected]> wrote:

Yet another batch of miscellaneous DAMON minor improvements. Mostly focused on readability and clarity of code and document, and unit/self test coverage. No user-visible behavioral change is intended.

Thanks, all queued up in mm-new.