mm/damon: cleanup code, add test cases, and update guidances in docs

kdamond_merge_regions() open-codes max region merge threshold calculation. What it does is fundamentally the same as damon_nr_samples_per_aggr() but missing a few corner cases. The unhandled corner cases should be rare and make only a negligible level of monitoring results degradation. But having the inconsistency could increase future maintenance burden. Use the dedicated function.

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

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

mm/damon/core.c +0 -0

index f748acd6bbd5b..846da6df5b552 100644

mm/damon/core.c +0 -0

 	while (true) {
 		nr_regions = 0;
 		damon_for_each_target(t, c) {

Signature · 1 line

2.47.3

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch removed the last usage of str_high_low() but did not remove the now-unused #include <linux/string_choices.h>. --

commit 43d661e4f50bbdfb8a2cca56bf761ac52256d5cd
Author: SJ Park <[email protected]>

mm/damon/core: remove debug messages

This patch removes several unused debug messages from the DAMON core to clean up the code.

Quoted 6 lines

mm/damon/core.c +0 -0

index bda8e23c10560..3e42f370ea67f 100644
@@ -3729,10 +3729,6 @@ static unsigned long damos_wmark_wait_us(struct damos *scheme)
 

mm/damon/core.c static unsigned long damos_wmark_wait_us(struct damos *scheme) · line 3738

Quoted 6 lines

mm/damon/core.c +0 -4

 	/* higher than high watermark or lower than low watermark */
 	if (metric > scheme->wmarks.high || scheme->wmarks.low > metric) {
-		if (scheme->wmarks.activated)
-			pr_debug("deactivate a scheme (%d) for %s wmark\n",
-				 scheme->action,
-				 str_high_low(metric > scheme->wmarks.high));

[Severity: Low]
Since this removes the last usage of str_high_low() in mm/damon/core.c, should the include for <linux/string_choices.h> near line 18 also be removed?

Quoted 4 lines

mm/damon/core.c +0 -0

 		scheme->wmarks.activated = false;
 		return scheme->wmarks.interval;
 	}

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

There are a few debug messages in DAMON core. Those have not really been used in a meaningful way for the last few years, though. Remove those.

There is a debug message in the DAMON virtual address space operation set. It has not really been used in a meaningful way for the last few years, though. Remove it.

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

---
 mm/damon/vaddr.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

mm/damon/vaddr.c +0 -0

index c8c32b2ae0402..f884d3f78f30a 100644
@@ -189,22 +189,12 @@ static int damon_va_three_regions(struct damon_target *t,
  *   <BIG UNMAPPED REGION 2>
  *   <stack>
  */

mm/damon/vaddr.c +2 -7

-			if (ti == t)
-				break;
-			tidx++;
-		}
-		pr_debug("Failed to get three regions of %dth target\n", tidx);
+	if (damon_va_three_regions(t, regions))
 		return;
-	}
 
 	damon_set_regions(t, regions, 3, DAMON_MIN_REGION_SZ);
 }

@@ -217,7 +207,7 @@ static void damon_va_init(struct damon_ctx *ctx)
 	damon_for_each_target(t, ctx) {
 		/* the user may set the target regions as they want */
 		if (!damon_nr_regions(t))
-			__damon_va_init_regions(ctx, t);
+			__damon_va_init_regions(t);
 	}
 }
 

Signature · 1 line

2.47.3

Each DAMON context is allowed to have only up to DAMON_MAX_PROBES probes. The central place for validating DAMON probe parameters, damon_valid_probe_params(), is not validating the upper limit, though. Do the validation.

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

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

mm/damon/core.c +0 -0

index 4d28eb033fc4a..c010f686d033e 100644

mm/damon/core.c +0 -0

 	if (!damon_has_probe_weights(ctx))
 		return true;

Signature · 1 line

2.47.3

DAMON sysfs interface is disallowing >DAMON_MAX_PROBES nr_probes input, since DAMON_MAX_PROBES is the upper limit of probes per DAMON context. The core layer is validating the upper limit again, though. It is preferred to let DAMON API callers such as sysfs interface to set parameters in flexible ways, and do parameters validation in the core layer. Drop the duplicated validation in the sysfs interface.

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

---
 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/damon/sysfs.c +0 -0

index 7ec14f48d157a..b576e97cbfdb8 100644

mm/damon/sysfs.c +0 -0

 		return -EINVAL;
 
 	probes = container_of(kobj, struct damon_sysfs_probes, kobj);

Signature · 1 line

2.47.3

On Tue, 1 Sep 2026 22:47:38 -0700 SJ Park <[email protected]> wrote:

I was wondering whether DAMON_MAX_PROBES is purely a core invariant or also an interface constraint.

With this change, sysfs allows nr_probes larger than DAMON_MAX_PROBES and damon_sysfs_probes_add_dirs() will start creating probe objects before the configuration is later rejected by damon_valid_probe_params().

Since nr_probes directly controls the number of sysfs objects created, do we still want to keep an early check here?

I agree that the core validation is required for non-sysfs callers, but I am not sure whether this particular limit should be duplicated at the sysfs layer.

Quoted 1 line · On Wed, 2 Sep 2026 22:07:13 +0800 Kunwu Chan <[email protected]> wrote:

On Tue, 1 Sep 2026 22:47:38 -0700 SJ Park <[email protected]> wrote:

[...]

Quoted 2 lines

I was wondering whether DAMON_MAX_PROBES is purely a core invariant or also an interface constraint.

Thank you for reviewing my patch and raising this question!

Quoted 6 lines

With this change, sysfs allows nr_probes larger than DAMON_MAX_PROBES and damon_sysfs_probes_add_dirs() will start creating probe objects before the configuration is later rejected by damon_valid_probe_params().

Quoted 6 lines

Since nr_probes directly controls the number of sysfs objects created, do we still want to keep an early check here?

I agree that the core validation is required for non-sysfs callers, but I am not sure whether this particular limit should be duplicated at the sysfs layer.

I agree the user experience may be not that good.

In my humble opinion, however, keeping code simplicity is more important than the user experience here. After all, DAMON_SYSFS is recommended to be used by another high level tools like DAMON user-space tool (damo) rather than human fingers. The user-space tools like damo can do the early check.

Please feel free to let me know if you have any other opinions or questions.

Thanks,
SJ

[...]

Commit 954157679ec3 ("mm/damon/core: disallow overlapping input ranges for damon_set_regions()") disallowed passing overlapping input ranges to damon_set_regions(). Add a kunit test case for the overlapping input.

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

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

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

index 3cbbbcbfbef8f..a4164ac489a61 100644
@@ -612,6 +612,17 @@ static void damon_test_set_regions(struct kunit *test)
 			{.start = 42, .end = 21},
 			}, 1, 1,
 			(struct damon_addr_range[]){}, 0, -EINVAL);
+	/*
+	 * Regions resulting in same region after alignment should return
+	 * -EINVAL.
+	 */
+	damon_test_set_regions_for(test,
+			(struct damon_addr_range[]){}, 0,
+			(struct damon_addr_range[]){
+			{.start = 10, .end = 20},
+			{.start = 20, .end = 30},
+			}, 2, 4096,
+			(struct damon_addr_range[]){}, 0, -EINVAL);
 }
 
 static void damon_test_update_monitoring_result(struct kunit *test)

Signature · 1 line

2.47.3

damon_max_nr_accesses(), which is a previous version of damon_nr_samples_per_aggr() before the renaming, was wrongly returning zero or random overflowed values for extreme intervals setup. Commit 35d4a3cf70a8 ("mm/damon/ops-common: handle extreme intervals in damon_hot_score()") updated the function to return correct or more valid values. Add a kunit test to ensure it is working as expected.

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

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

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

index a4164ac489a61..d0bed01a5b7fe 100644

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

+	/* Zero aggregation interval doesn't cause division by zero */
+	KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs), 1);
+
+	/*
+	 * Too large aggregation interval on 64 bit system doesn't cause
+	 * overflow
+	 */
+	if (ULONG_MAX > UINT_MAX) {
+		attrs.aggr_interval = (unsigned long)UINT_MAX + 1;
+		KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs),
+				UINT_MAX);
+	}
+}
+
 static void damon_test_update_monitoring_result(struct kunit *test)
 {
 	struct damon_attrs old_attrs = {

@@ -1736,6 +1757,7 @@ static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_split_above_half_progresses),
 	KUNIT_CASE(damon_test_ops_registration),
 	KUNIT_CASE(damon_test_set_regions),
+	KUNIT_CASE(damon_test_nr_samples_per_aggr),
 	KUNIT_CASE(damon_test_update_monitoring_result),
 	KUNIT_CASE(damon_test_set_attrs),
 	KUNIT_CASE(damon_test_mvsum),

Signature · 1 line

2.47.3

On Tue, 1 Sep 2026 22:47:42 -0700 SJ Park <[email protected]> wrote:

Both sample_interval and aggr_interval are zero here. Since sample_interval is the denominator in damon_nr_samples_per_aggr(), would it be better to keep aggr_interval non-zero when testing the zero sample_interval case?

This would make the test explicitly cover the divide-by-zero protection. If the aggr_interval == 0 behavior is also worth covering, perhaps it could be tested separately.

Quoted 1 line · On Wed, 2 Sep 2026 23:08:41 +0800 Kunwu Chan <[email protected]> wrote:

On Tue, 1 Sep 2026 22:47:42 -0700 SJ Park <[email protected]> wrote:

[...]

Quoted 8 lines

Both sample_interval and aggr_interval are zero here. Since sample_interval is the denominator in damon_nr_samples_per_aggr(), would it be better to keep aggr_interval non-zero when testing the zero sample_interval case?

This would make the test explicitly cover the divide-by-zero protection. If the aggr_interval == 0 behavior is also worth covering, perhaps it could be tested separately.

Thank you for your review and question, Kunwu!

Yes, that kind of additional test case would be nice. Please feel free to post a patch if you'd like to! :) And no pressure, no rush. I will also consider doing it myself if it seems you are not interested.

Thanks,
SJ

[...]

Quoted 15 lines · On Wed, 2 Sep 2026 08:19:54 -0700 SJ Park <[email protected]> wrote:

Quoted 1 line · On Wed, 2 Sep 2026 23:08:41 +0800 Kunwu Chan <[email protected]> wrote:

On Tue, 1 Sep 2026 22:47:42 -0700 SJ Park <[email protected]> wrote:

[...]

Quoted 8 lines

Both sample_interval and aggr_interval are zero here. Since sample_interval is the denominator in damon_nr_samples_per_aggr(), would it be better to keep aggr_interval non-zero when testing the zero sample_interval case?

This would make the test explicitly cover the divide-by-zero protection. If the aggr_interval == 0 behavior is also worth covering, perhaps it could be tested separately.

Thank you for your review and question, Kunwu!

Quoted 3 lines

Yes, that kind of additional test case would be nice. Please feel free to post a patch if you'd like to! :) And no pressure, no rush. I will also consider doing it myself if it seems you are not interested.

Thanks for the feedback, SJ!

I will prepare a small follow-up patch for this test clarity improvement.

Reviewed-by Kunwu Chan <[email protected]>

DAMON sysfs quota goal target_metric file now accepts 'hugepage_mem_bp' input. Test it is accepted in fundamental DAMON sysfs file operation selftest.

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

---
 tools/testing/selftests/damon/sysfs.sh | 1 +
 1 file changed, 1 insertion(+)

tools/testing/selftests/damon/sysfs.sh +1 -0

index ddebde6edabe4..b66593c9ac471 100755
@@ -210,6 +210,7 @@ test_goal()
 	ensure_write_succ "$fpath" "active_mem_bp" "valid input"
 	ensure_write_succ "$fpath" "inactive_mem_bp" "valid input"
 	ensure_write_succ "$fpath" "node_eligible_mem_bp" "valid input"
+	ensure_write_succ "$fpath" "hugepage_mem_bp" "valid input"
 	ensure_write_fail "$fpath" "foo" "invalid input"
 	ensure_file "$goal_dir/nid" "exist" "600"
 	ensure_file "$goal_dir/path" "exist" "600"

Signature · 1 line

2.47.3

DAMON ABI doc is recommending DAMON admin-guide for people who are willing to know further about DAMON. Nowadays the subsystem doc (Documentation/mm/damon) is a more recommended place for even beginners. Recommend the subsystem doc over admin-guide.

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

---
 Documentation/ABI/testing/sysfs-kernel-mm-damon | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/ABI/testing/sysfs-kernel-mm-damon +1 -1

index f8d2601e82904..ad21f58f3c912 100644
@@ -3,7 +3,7 @@ Date:		Mar 2022
 Contact:	SJ Park <[email protected]>
 Description:	Interface for Data Access MONitoring (DAMON).  Contains files
 		for controlling DAMON.  For more details on DAMON itself,
-		please refer to Documentation/admin-guide/mm/damon/index.rst.
+		please refer to Documentation/mm/damon/index.rst.
 
 What:		/sys/kernel/mm/damon/admin/
 Date:		Mar 2022

Signature · 1 line

2.47.3

Misc cleanup, improvements and updates of code, test, and documents.

Misc cleanup, improvements and updates of code, test, and documents.

Patches 1-5 cleanup DAMON code. Patches 6-10 adds kunit and selftest test cases for recently fixed bugs and a new feature. Patches 11 and 12 update guidelines for AI review and what document to read, on DAMON documents.