[RFC PATCH v1.2 0/7] mm/damon: update probe hits for runtime parameter commits

[RFC PATCH v1.2 0/7] mm/damon: update probe hits for runtime parameter commits

[SJ Park]

DAMON users can update DAMON parameters such as sampling and aggregation
intervals at runtime.  For such changes, monitoring results that depend
on the intervals should be properly updated for better accuracy.  For
example, the access frequency counter (nr_accesses) is updated.  The
data attribute monitoring counter (probe_hits) is not being updated,
though.  Do the updates for new parameters.

Patches Sequence
================

Patch 1 removes obsolete comments and test code for a function that this
series will touch.  Patches 2-5 rename functions that are being used for
nr_accesses update, to be able to be used for probe_hits without
confusion.  Patch 6 does the probe_hits update.  Patch 7 update
damon_probe_hits_mvsum() to cover a corner case from the update for
better accuracy.

Changelog
=========

Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/20260707135038.90068-1-sj@kernel.org
- Remove obsolete divide-by-zero test with comments as a separate patch.
- Remove obsolete comment removal part in a rename commit message.
- Rename name-changed function parameters.
Changes from RFC
- RFC: https://lore.kernel.org/20260707043828.97900-1-sj@kernel.org
- Remove obsolete comment and code on the test.

SJ Park (7):
  mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero
  mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/
  mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/
  mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/
  mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/
  mm/damon/core: update probe hits for new parameter commit
  mm/damon/core: handle unreset probe_hits in probe_hits_mvsum()

 include/linux/damon.h       | 10 +++---
 mm/damon/core.c             | 65 +++++++++++++++++++++++--------------
 mm/damon/lru_sort.c         |  2 +-
 mm/damon/ops-common.c       |  2 +-
 mm/damon/tests/core-kunit.h | 38 +++++++++++-----------
 5 files changed, 68 insertions(+), 49 deletions(-)


base-commit: 29da8f7d1b66215addd212b9b7191a203e322b8c
-- 
2.47.3

[RFC PATCH v1.2 1/7] mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero

[SJ Park]

The comments on damon_nr_accesses_to_accesses_bp() and its unit test
warn it can trigger division-by-zero when the aggregation interval is
zero.  Commit 35d4a3cf70a8 ("mm/damon/ops-common: handle extreme
intervals in damon_hot_score()") modified damon_nr_samples_per_aggr() to
always return non-zero.  Hence no division-by-zero of the note can
happen.  Remove the obsolete comment on the function.  The test code was
written to test the division-by-zero case, which cannot happen anymore.
Having it makes no sense.  Entirely remove the test code and its
comment.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c             |  8 +-------
 mm/damon/tests/core-kunit.h | 25 +++++++++----------------
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 390e00b3685ef..a15005db82899 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -883,13 +883,7 @@ static unsigned int damon_accesses_bp_to_nr_accesses(
 	return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
 }
 
-/*
- * Convert nr_accesses to access ratio in bp (per 10,000).
- *
- * Callers should ensure attrs.aggr_interval is not zero, like
- * damon_update_monitoring_results() does .  Otherwise, divide-by-zero would
- * happen.
- */
+/* Convert nr_accesses to access ratio in bp (per 10,000) */
 static unsigned int damon_nr_accesses_to_accesses_bp(
 		unsigned int nr_accesses, struct damon_attrs *attrs)
 {
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 0124f83b39b83..b2a280556b841 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -575,26 +575,20 @@ static void damon_test_set_regions(struct kunit *test)
 			}, 3);
 }
 
-static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
+static void damon_test_max_nr_accesses(struct kunit *test)
 {
 	struct damon_attrs attrs = {
-		.sample_interval = 10,
-		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
+		.sample_interval = 0,
+		.aggr_interval = 0,
 	};
 
-	/*
-	 * In some cases such as 32bit architectures where UINT_MAX is
-	 * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
-	 * damon_nr_accesses_to_accesses_bp() in the case will cause
-	 * divide-by-zero.  Such case is prohibited in normal execution since
-	 * the caution is documented on the comment for the function, and
-	 * damon_update_monitoring_results() does the check.  Skip the test in
-	 * the case.
-	 */
-	if (!attrs.aggr_interval)
-		kunit_skip(test, "aggr_interval is zero.");
+	/* Zero aggregation interval doesn't cause division by zero */
+	KUNIT_EXPECT_EQ(test, damon_max_nr_accesses(attrs), 1);
 
-	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);
+	/* Too large aggregation interval doesn't cause overflow */
+	attrs.aggr_interval = (unsigned long)UINT_MAX + 1;
+	KUNIT_EXPECT_GE(test, damon_max_nr_accesses(attrs), 0);
+	KUNIT_EXPECT_LE(test, damon_max_nr_accesses(attrs), UINT_MAX);
 }
 
 static void damon_test_update_monitoring_result(struct kunit *test)
@@ -1572,7 +1566,6 @@ 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_accesses_to_accesses_bp),
 	KUNIT_CASE(damon_test_update_monitoring_result),
 	KUNIT_CASE(damon_test_set_attrs),
 	KUNIT_CASE(damon_test_mvsum),
-- 
2.47.3

[RFC PATCH v1.2 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[SJ Park]

damon_max_nr_accesses() actually returns the number of samples DAMON
checks for each region per each aggregation interval.  Rename it to
better describe what it really does and not confusing for more general
uses.

Signed-off-by: SJ Park <sj@kernel.org>
---
 include/linux/damon.h | 10 ++++++----
 mm/damon/core.c       |  4 ++--
 mm/damon/lru_sort.c   |  2 +-
 mm/damon/ops-common.c |  2 +-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 19b7e839bde0b..63f596957c1f1 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1056,15 +1056,17 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
 	return ctx->ops.id == DAMON_OPS_VADDR || ctx->ops.id == DAMON_OPS_FVADDR;
 }
 
-static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
+/* Returns number of samples per aggregation interval */
+static inline unsigned int damon_nr_samples_per_aggr(
+		const struct damon_attrs *attrs)
 {
 	unsigned long sample_interval;
-	unsigned long max_nr_accesses;
+	unsigned long nr_samples;
 
 	sample_interval = attrs->sample_interval ? : 1;
-	max_nr_accesses = min(attrs->aggr_interval / sample_interval,
+	nr_samples = min(attrs->aggr_interval / sample_interval,
 			(unsigned long)UINT_MAX);
-	return max_nr_accesses ? : 1;
+	return nr_samples ? : 1;
 }
 
 
diff --git a/mm/damon/core.c b/mm/damon/core.c
index a15005db82899..177e5abc981ef 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -880,14 +880,14 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,
 static unsigned int damon_accesses_bp_to_nr_accesses(
 		unsigned int accesses_bp, struct damon_attrs *attrs)
 {
-	return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
+	return accesses_bp * damon_nr_samples_per_aggr(attrs) / 10000;
 }
 
 /* Convert nr_accesses to access ratio in bp (per 10,000) */
 static unsigned int damon_nr_accesses_to_accesses_bp(
 		unsigned int nr_accesses, struct damon_attrs *attrs)
 {
-	return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
+	return mult_frac(nr_accesses, 10000, damon_nr_samples_per_aggr(attrs));
 }
 
 static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index e8c389ad3226f..7e077084cb03a 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -303,7 +303,7 @@ static int damon_lru_sort_apply_parameters(void)
 		goto out;
 
 	err = -ENOMEM;
-	hot_thres = damon_max_nr_accesses(&attrs) *
+	hot_thres = damon_nr_samples_per_aggr(&attrs) *
 		hot_thres_access_freq / 1000;
 	hot_scheme = damon_lru_sort_new_hot_scheme(hot_thres);
 	if (!hot_scheme)
diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index 6bdd1cfd3863a..89c694f6e4fa8 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -112,7 +112,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
 	int hotness;
 
 	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
-		damon_max_nr_accesses(&c->attrs);
+		damon_nr_samples_per_aggr(&c->attrs);
 
 	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
 	if (age_in_sec)
-- 
2.47.3

[RFC PATCH v1.2 3/7] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/

[SJ Park]

accesses_bp_to_nr_accesses() actually converts a positive samples ratio
to the count.  Rename it to better describe what it really does and not
confusing for more general uses.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 177e5abc981ef..dea63083917b0 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -876,11 +876,11 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,
 	return age * old_attrs->aggr_interval / new_attrs->aggr_interval;
 }
 
-/* convert access ratio in bp (per 10,000) to nr_accesses */
-static unsigned int damon_accesses_bp_to_nr_accesses(
-		unsigned int accesses_bp, struct damon_attrs *attrs)
+/* convert sample ratio in bp (per 10,000) to count */
+static unsigned int damon_sample_bp_to_count(
+		unsigned int bp, struct damon_attrs *attrs)
 {
-	return accesses_bp * damon_nr_samples_per_aggr(attrs) / 10000;
+	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
 }
 
 /* Convert nr_accesses to access ratio in bp (per 10,000) */
@@ -893,7 +893,7 @@ static unsigned int damon_nr_accesses_to_accesses_bp(
 static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
 {
-	return damon_accesses_bp_to_nr_accesses(
+	return damon_sample_bp_to_count(
 			damon_nr_accesses_to_accesses_bp(
 				nr_accesses, old_attrs),
 			new_attrs);
-- 
2.47.3

[RFC PATCH v1.2 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

[SJ Park]

damon_nr_accesses_to_accesses_bp() actually converts a positive sample
count to the ratio.  Rename it to better describe what it really does
and not confusing for more general uses.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index dea63083917b0..718268df27fa7 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -883,19 +883,18 @@ static unsigned int damon_sample_bp_to_count(
 	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
 }
 
-/* Convert nr_accesses to access ratio in bp (per 10,000) */
-static unsigned int damon_nr_accesses_to_accesses_bp(
-		unsigned int nr_accesses, struct damon_attrs *attrs)
+/* convert sample count to ratio in bp (per 10,000) */
+static unsigned int damon_sample_count_to_bp(
+		unsigned int count, struct damon_attrs *attrs)
 {
-	return mult_frac(nr_accesses, 10000, damon_nr_samples_per_aggr(attrs));
+	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
 }
 
 static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
 {
 	return damon_sample_bp_to_count(
-			damon_nr_accesses_to_accesses_bp(
-				nr_accesses, old_attrs),
+			damon_sample_count_to_bp(nr_accesses, old_attrs),
 			new_attrs);
 }
 
-- 
2.47.3

[RFC PATCH v1.2 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[SJ Park]

damon_nr_accesses_for_new_attrs() can be used for not only nr_accesses
but also any positive sample count, like probe_hits.  Rename to be able
to be used for such general uses without confusion.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 718268df27fa7..800ff6b6d2473 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -890,22 +890,21 @@ static unsigned int damon_sample_count_to_bp(
 	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
 }
 
-static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
+static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
 {
 	return damon_sample_bp_to_count(
-			damon_sample_count_to_bp(nr_accesses, old_attrs),
-			new_attrs);
+			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
 }
 
 static void damon_update_monitoring_result(struct damon_region *r,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
 		bool aggregating)
 {
-	r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
+	r->last_nr_accesses = damon_nr_samples_for_new_attrs(
 			r->last_nr_accesses, old_attrs, new_attrs);
 	if (!aggregating)
-		r->nr_accesses = damon_nr_accesses_for_new_attrs(
+		r->nr_accesses = damon_nr_samples_for_new_attrs(
 				r->nr_accesses, old_attrs, new_attrs);
 	else
 		/*
-- 
2.47.3

[RFC PATCH v1.2 6/7] mm/damon/core: update probe hits for new parameter commit

[SJ Park]

Users can update DAMON parameters at runtime.  If the samples and/or
aggregation intervals are updated in this way, monitoring results
depending on the intervals should also be updated for a more accurate
snapshot.  The age and nr_accesses are properly updated, while
probe_hits are not updated in the way.  Do the update.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c             | 28 +++++++++++++++++++++++++---
 mm/damon/tests/core-kunit.h | 13 ++++++++++---
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 800ff6b6d2473..15d31285515d6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -897,10 +897,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
 			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
 }
 
+static void damon_update_probe_hits(struct damon_region *r,
+		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
+		bool aggregating, struct damon_ctx *ctx)
+{
+	struct damon_probe *p;
+	int i = 0;
+
+	damon_for_each_probe(p, ctx) {
+		r->last_probe_hits[i] = damon_nr_samples_for_new_attrs(
+				r->last_probe_hits[i], old_attrs, new_attrs);
+		if (!aggregating)
+			r->probe_hits[i] = damon_nr_samples_for_new_attrs(
+					r->probe_hits[i], old_attrs,
+					new_attrs);
+		else
+			r->probe_hits[i] = 0;
+		i++;
+	}
+}
+
 static void damon_update_monitoring_result(struct damon_region *r,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
-		bool aggregating)
+		bool aggregating, struct damon_ctx *ctx)
 {
+	damon_update_probe_hits(r, old_attrs, new_attrs, aggregating, ctx);
+
 	r->last_nr_accesses = damon_nr_samples_for_new_attrs(
 			r->last_nr_accesses, old_attrs, new_attrs);
 	if (!aggregating)
@@ -940,8 +962,8 @@ static void damon_update_monitoring_results(struct damon_ctx *ctx,
 
 	damon_for_each_target(t, ctx)
 		damon_for_each_region(r, t)
-			damon_update_monitoring_result(
-					r, old_attrs, new_attrs, aggregating);
+			damon_update_monitoring_result(r, old_attrs, new_attrs,
+					aggregating, ctx);
 }
 
 /*
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index b2a280556b841..219d02c98624a 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -597,32 +597,39 @@ static void damon_test_update_monitoring_result(struct kunit *test)
 		.sample_interval = 10, .aggr_interval = 1000,};
 	struct damon_attrs new_attrs;
 	struct damon_region *r = damon_new_region(3, 7);
+	struct damon_ctx *ctx;
 
 	if (!r)
 		kunit_skip(test, "region alloc fail");
+	ctx = damon_new_ctx();
+	if (!ctx) {
+		damon_free_region(r);
+		kunit_skip(test, "ctx alloc fail");
+	}
 
 	r->nr_accesses = 15;
 	r->age = 20;
 
 	new_attrs = (struct damon_attrs){
 		.sample_interval = 100, .aggr_interval = 10000,};
-	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
+	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false, ctx);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 15);
 	KUNIT_EXPECT_EQ(test, r->age, 2);
 
 	new_attrs = (struct damon_attrs){
 		.sample_interval = 1, .aggr_interval = 1000};
-	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
+	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false, ctx);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
 	KUNIT_EXPECT_EQ(test, r->age, 2);
 
 	new_attrs = (struct damon_attrs){
 		.sample_interval = 1, .aggr_interval = 100};
-	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
+	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false, ctx);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
 	KUNIT_EXPECT_EQ(test, r->age, 20);
 
 	damon_free_region(r);
+	damon_destroy_ctx(ctx);
 }
 
 static void damon_test_set_attrs(struct kunit *test)
-- 
2.47.3

[RFC PATCH v1.2 7/7] mm/damon/core: handle unreset probe_hits in probe_hits_mvsum()

[SJ Park]

If damon_update_monitoring_result() is called at the end of the
aggregation interval, probe_hits is not reset.  That's because the value
will be exposed to the user via damon_region_aggregated trace event.

Meanwhile, damon_probe_hits_mvsum() can be called in this state.  Due to
its logic, it will return a value that is incorrectly high.  This could
happen  if the user requested DAMOS schemes applied regions sysfs files
update exactly in the time sequence.

The impact is minor, but better to avoid.  Check the timing and simply
return the fully aggregated last_probe_hits, like
damon_nr_accesses_mvsum() also does.  It is not 100% accurate since it
is the last interval's aggregation.  But better than the value that is
completely reset.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 15d31285515d6..11473aa1f70f4 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -290,6 +290,9 @@ unsigned char damon_probe_hits_mvsum(int probe_idx, struct damon_region *r,
 			ctx->passed_sample_intervals;
 	left_window_bp = mult_frac(left_window, 10000, window_len);
 
+	if (left_window_bp == 10000)
+		return r->last_probe_hits[probe_idx];
+
 	return damon_mvsum(r->probe_hits[probe_idx],
 			r->last_probe_hits[probe_idx], left_window_bp);
 }
-- 
2.47.3

Re: [RFC PATCH v1.2 3/7] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The calculation of sample counts and ratios in `damon_sample_bp_to_count()` and `damon_nr_accesses_to_accesses_bp()` can suffer from a 32-bit integer overflow under extreme interval configurations, leading to corrupted DAMON metrics.
--

--- Patch [3]: [RFC PATCH v1.2 3/7] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/ ---
commit e8ec17a3e305676513352140eeb31c1b75063618
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/

This commit renames accesses_bp_to_nr_accesses() to sample_bp_to_count() 
to better describe its actual behavior. The function converts a positive 
samples ratio into a count.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -876,11 +876,11 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,
>  	return age * old_attrs->aggr_interval / new_attrs->aggr_interval;
>  }
>  
> -/* convert access ratio in bp (per 10,000) to nr_accesses */
> -static unsigned int damon_accesses_bp_to_nr_accesses(
> -		unsigned int accesses_bp, struct damon_attrs *attrs)
> +/* convert sample ratio in bp (per 10,000) to count */
> +static unsigned int damon_sample_bp_to_count(
> +		unsigned int bp, struct damon_attrs *attrs)
>  {
> -	return accesses_bp * damon_nr_samples_per_aggr(attrs) / 10000;
> +	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
[Severity: High]
This is a pre-existing issue, but can the 32-bit unsigned arithmetic here
silently overflow under extreme configurations?

If an automated tool configures DAMON via sysfs (for example, with
sample_us = 1 and aggr_us = 1000000) such that the ratio
aggr_interval / sample_interval exceeds 429,496, the multiplication
bp * damon_nr_samples_per_aggr(attrs) would exceed 32 bits.

This could return a heavily truncated count, potentially corrupting access
metrics and causing flawed memory management decisions.

A similar overflow appears to happen in the calculation of the ratio in
damon_nr_accesses_to_accesses_bp(). When it calls mult_frac, the parameter
nr_accesses is an unsigned int, causing nr_accesses * 10000 to overflow
and wrap around when evaluating the remainder multiplication.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=3

Re: [RFC PATCH v1.2 1/7] mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to entirely remove the test code, but instead leaves a broken, orphaned test function causing build failures.
--

--- Patch [1]: [RFC PATCH v1.2 1/7] mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero ---
commit d67544b646a2eb5eb3336e5b2555a05dd0f2f18c
Author: SJ Park <sj@kernel.org>

mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero

This patch removes obsolete comments and test code related to division by zero
in DAMON's nr_accesses_to_accesses_bp() function, noting that earlier commits
prevent zero aggregation intervals from reaching this code.

> Commit 35d4a3cf70a8 ("mm/damon/ops-common: handle extreme
> intervals in damon_hot_score()") modified damon_nr_samples_per_aggr() to
> always return non-zero.  Hence no division-by-zero of the note can
> happen.  Remove the obsolete comment on the function.  The test code was
> written to test the division-by-zero case, which cannot happen anymore.
> Having it makes no sense.  Entirely remove the test code and its
> comment.

[Severity: Medium]
The commit message states that the test code was entirely removed, but the
diff shows the test being rewritten to damon_test_max_nr_accesses() instead.

Also, should the commit message reference damon_max_nr_accesses() here
instead of damon_nr_samples_per_aggr(), given the function that is actually
being tested in the diff?

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 0124f83b39b83..b2a280556b841 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -575,26 +575,20 @@ static void damon_test_set_regions(struct kunit *test)
>  			}, 3);
>  }
>  
> -static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
> +static void damon_test_max_nr_accesses(struct kunit *test)
>  {
>  	struct damon_attrs attrs = {
> -		.sample_interval = 10,
> -		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
> +		.sample_interval = 0,
> +		.aggr_interval = 0,
>  	};

[ ... ]

> +	/* Zero aggregation interval doesn't cause division by zero */
> -	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);
> +	KUNIT_EXPECT_EQ(test, damon_max_nr_accesses(attrs), 1);

[Severity: Medium]
Will this cause a build failure?

Looking at damon_max_nr_accesses(), it expects a pointer to struct damon_attrs
rather than a struct passed by value. Passing attrs here directly looks like it
will result in an incompatible type error during compilation.

[ ... ]

> @@ -1572,7 +1566,6 @@ 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_accesses_to_accesses_bp),
>  	KUNIT_CASE(damon_test_update_monitoring_result),
>  	KUNIT_CASE(damon_test_set_attrs),
>  	KUNIT_CASE(damon_test_mvsum),

[Severity: Medium]
Does removing this test case from the array orphan the newly rewritten test
function above?

Since damon_test_max_nr_accesses() is declared as static and is no longer
referenced by damon_test_cases[], it appears this will trigger a
-Wunused-function warning when compiling the tests.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=1

Re: [RFC PATCH v1.2 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The KUnit tests fail to build because they still use the deleted function name `damon_max_nr_accesses()`. Furthermore, the test function uses an incorrect pass-by-value argument and is missing from the test cases array.

Pre-existing issues:
- [High] Multiplication involving `damon_nr_samples_per_aggr()` can easily overflow a 32-bit unsigned integer.
--

--- Patch [2]: [RFC PATCH v1.2 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ ---
commit 41a943f324d729eeccaf0110b19b34cddf7fa203
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

This commit renames damon_max_nr_accesses() to damon_nr_samples_per_aggr()
to better describe its actual behavior returning the number of samples per
aggregation interval.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 19b7e839bde0b..63f596957c1f1 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -1056,15 +1056,17 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>  	return ctx->ops.id == DAMON_OPS_VADDR || ctx->ops.id == DAMON_OPS_FVADDR;
>  }
>  
> -static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> +/* Returns number of samples per aggregation interval */
> +static inline unsigned int damon_nr_samples_per_aggr(
> +		const struct damon_attrs *attrs)

[Severity: Low]
Does this rename break the KUnit tests? 

In mm/damon/tests/core-kunit.h, the damon_test_max_nr_accesses() test still
references the old damon_max_nr_accesses() name and passes attrs by value
instead of as a pointer. 

Furthermore, it appears damon_test_max_nr_accesses() is missing from the
damon_test_cases array.

>  {
>  	unsigned long sample_interval;
> -	unsigned long max_nr_accesses;
> +	unsigned long nr_samples;
>  
>  	sample_interval = attrs->sample_interval ? : 1;
> -	max_nr_accesses = min(attrs->aggr_interval / sample_interval,
> +	nr_samples = min(attrs->aggr_interval / sample_interval,
>  			(unsigned long)UINT_MAX);
> -	return max_nr_accesses ? : 1;
> +	return nr_samples ? : 1;
>  }

[ ... ]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index a15005db82899..177e5abc981ef 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -880,14 +880,14 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,
>  static unsigned int damon_accesses_bp_to_nr_accesses(
>  		unsigned int accesses_bp, struct damon_attrs *attrs)
>  {
> -	return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
> +	return accesses_bp * damon_nr_samples_per_aggr(attrs) / 10000;
>  }

[Severity: High]
This is a pre-existing issue, but can this multiplication overflow a
32-bit unsigned integer?

Since damon_nr_samples_per_aggr() can return a large value (up to UINT_MAX),
multiplying it by accesses_bp (which can be up to 10000) could cause an
intermediate overflow wrap-around before the division.

For example, a 35-minute aggregation interval with a 5ms sampling interval
yields about 420,000 samples. Multiplying this by 10,000 exceeds 4.2
billion, which wraps around a 32-bit unsigned integer.

[ ... ]

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index e8c389ad3226f..7e077084cb03a 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
> @@ -303,7 +303,7 @@ static int damon_lru_sort_apply_parameters(void)
>  		goto out;
>  
>  	err = -ENOMEM;
> -	hot_thres = damon_max_nr_accesses(&attrs) *
> +	hot_thres = damon_nr_samples_per_aggr(&attrs) *
>  		hot_thres_access_freq / 1000;

[Severity: High]
This is also a pre-existing issue, but does this multiplication have the
same 32-bit overflow risk?

If the aggregation interval is sufficiently large, multiplying
damon_nr_samples_per_aggr() by hot_thres_access_freq could similarly wrap
around before the division.

>  	hot_scheme = damon_lru_sort_new_hot_scheme(hot_thres);

[ ... ]

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 6bdd1cfd3863a..89c694f6e4fa8 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -112,7 +112,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
>  	int hotness;
>  
>  	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
> -		damon_max_nr_accesses(&c->attrs);
> +		damon_nr_samples_per_aggr(&c->attrs);

[Severity: High]
This is another pre-existing issue, but could this multiplication also
overflow?

Multiplying r->nr_accesses by DAMON_MAX_SUBSCORE (100) will overflow a
32-bit unsigned integer if the region has accumulated more than
42,949,672 accesses.

>  
>  	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=2

Re: [RFC PATCH v1.2 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch series introduces a build failure in the DAMON KUnit tests by leaving behind a broken, unreferenced test function.

Pre-existing issues:
- [High] Functions `damon_sample_bp_to_count` and `damon_sample_count_to_bp` silently overflow their 32-bit unsigned integer calculations when the number of samples per aggregation interval is large.
--

--- Patch [5]: [RFC PATCH v1.2 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit a57666ff806c37c47b5fa8f1ce4de7e70616ac6d
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/
    
This commit renames damon_nr_accesses_for_new_attrs() to
damon_nr_samples_for_new_attrs() to clarify that it can be used for any
positive sample count, not just nr_accesses.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 718268df27fa7..800ff6b6d2473 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -890,22 +890,21 @@ static unsigned int damon_sample_count_to_bp(
>  	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
>  }

[Severity: High]
This isn't a bug introduced by this patch, but renaming this function
propagates a flawed arithmetic calculation.

Can the 32-bit unsigned integer calculations in damon_sample_count_to_bp()
and damon_sample_bp_to_count() silently overflow?

If a user configures a large aggregation interval and small sample interval,
the total samples per aggregation can exceed 429,496, leading to 32-bit
arithmetic overflow during ratio conversion.

In damon_sample_count_to_bp():

    mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));

The mult_frac macro expands to perform a 32-bit multiplication of count
(or its remainder) by 10000, which overflows for counts exceeding 429,496.

Similarly, in damon_sample_bp_to_count() (just above this context):

    return bp * damon_nr_samples_per_aggr(attrs) / 10000;

The multiplication is performed in 32-bit unsigned integer arithmetic, which
overflows if the total samples per aggregation is large.

>  
> -static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
> +static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
>  {
>  	return damon_sample_bp_to_count(
> -			damon_sample_count_to_bp(nr_accesses, old_attrs),
> -			new_attrs);
> +			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
>  }
>  

[ ... ]

[Severity: Low]
Does this patch series leave behind a broken, unreferenced test function
in the DAMON KUnit tests?

When building the kernel with DAMON KUnit tests enabled, this causes a build
failure due to an implicit function declaration.

In mm/damon/tests/core-kunit.h:damon_test_max_nr_accesses():

    KUNIT_EXPECT_EQ(test, damon_max_nr_accesses(attrs), 1);

The function calls damon_max_nr_accesses(), which was renamed in an earlier
patch in this series but was left unupdated in this abandoned test function.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=5

Re: [RFC PATCH v1.2 6/7] mm/damon/core: update probe hits for new parameter commit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent integer truncation causes scaled `probe_hits` to wrap around, corrupting monitoring data when updating DAMON attributes.
--

--- Patch [6]: [RFC PATCH v1.2 6/7] mm/damon/core: update probe hits for new parameter commit ---
commit 3c348527fa362f1ef3768392d433781b92010ba7
Author: SJ Park <sj@kernel.org>

mm/damon/core: update probe hits for new parameter commit

This commit ensures that DAMON probe hits are appropriately scaled when a user
dynamically updates the sampling or aggregation intervals at runtime. It
applies the existing scaling logic used for other metrics to both probe_hits
and last_probe_hits.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 800ff6b6d2473..15d31285515d6 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -897,10 +897,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
>  			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
>  }
>  
> +static void damon_update_probe_hits(struct damon_region *r,
> +		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
> +		bool aggregating, struct damon_ctx *ctx)
> +{
> +	struct damon_probe *p;
> +	int i = 0;
> +
> +	damon_for_each_probe(p, ctx) {
> +		r->last_probe_hits[i] = damon_nr_samples_for_new_attrs(
> +				r->last_probe_hits[i], old_attrs, new_attrs);
> +		if (!aggregating)
> +			r->probe_hits[i] = damon_nr_samples_for_new_attrs(
> +					r->probe_hits[i], old_attrs,
> +					new_attrs);

[Severity: Medium]
Will this silently truncate the scaled probe hits if they exceed 255?

The function damon_nr_samples_for_new_attrs() returns an unsigned int, but
both last_probe_hits and probe_hits are defined as unsigned char arrays in
struct damon_region. 

If a user dynamically updates intervals via sysfs such that the new
aggr_samples is significantly larger than the old one, the scaled value could
exceed the 8-bit maximum and wrap around modulo 256 instead of saturating.

Should these values be clamped to the maximum value of an unsigned char to
prevent monitoring data corruption when the scaled count overflows?

> +		else
> +			r->probe_hits[i] = 0;
> +		i++;
> +	}
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=6

Re: [RFC PATCH v1.2 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch series introduces a build failure when `CONFIG_DAMON_KUNIT_TEST=y`. An unused test function references a removed function `damon_max_nr_accesses()`.

Pre-existing issues:
- [High] 32-bit integer overflows in `damon_sample_bp_to_count()` and `damon_sample_count_to_bp()` can yield wildly incorrect sample counts when large aggregation intervals are used.
--

--- Patch [4]: [RFC PATCH v1.2 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ ---
commit ea8982f99be13edf95a6b08dabc871567edc1f0c
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

damon_nr_accesses_to_accesses_bp() actually converts a positive sample
count to the ratio.  Rename it to better describe what it really does
and not confusing for more general uses.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index dea63083917b0..718268df27fa7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -883,19 +883,18 @@ static unsigned int damon_sample_bp_to_count(
>  	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but could this calculation
overflow and truncate before division when the product exceeds
4,294,967,295?

If DAMON is configured with a high aggr_interval relative to
sample_interval, nr_samples can be large. If a region is heavily
accessed, the sample count increases. Using 32-bit unsigned integer
arithmetic here might cause silent truncation, leading to hot pages
being misclassified as cold, and causing aggressive memory reclamation
of the active working set.

> -/* Convert nr_accesses to access ratio in bp (per 10,000) */
> -static unsigned int damon_nr_accesses_to_accesses_bp(
> -		unsigned int nr_accesses, struct damon_attrs *attrs)
> +/* convert sample count to ratio in bp (per 10,000) */
> +static unsigned int damon_sample_count_to_bp(
> +		unsigned int count, struct damon_attrs *attrs)
>  {
> -	return mult_frac(nr_accesses, 10000, damon_nr_samples_per_aggr(attrs));
> +	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
>  }

[Severity: High]
This is a pre-existing issue, but does mult_frac() also suffer from a
32-bit integer overflow?

mult_frac() expands to expressions that internally compute the remainder
multiplied by 10000 using 32-bit unsigned arithmetic. If the sample
counts and aggregation intervals are large enough (above 429,496), could
this overflow?


[Severity: Low]
Additionally, it looks like an earlier patch in this series removed
damon_max_nr_accesses(), but left references to it in
mm/damon/tests/core-kunit.h:damon_test_max_nr_accesses().

Does compiling the kernel with CONFIG_DAMON_KUNIT_TEST=y fail due to an
undeclared function error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708013252.95888-1-sj@kernel.org?part=4

Re: [RFC PATCH v1.2 0/7] mm/damon: update probe hits for runtime parameter commits

[SJ Park]

On Tue,  7 Jul 2026 18:32:43 -0700 SJ Park <sj@kernel.org> wrote:

> DAMON users can update DAMON parameters such as sampling and aggregation
> intervals at runtime.  For such changes, monitoring results that depend
> on the intervals should be properly updated for better accuracy.  For
> example, the access frequency counter (nr_accesses) is updated.  The
> data attribute monitoring counter (probe_hits) is not being updated,
> though.  Do the updates for new parameters.

Please ignore this version.  I made silly mistakes while rebasing.  As this
version is not for a review, I will send a correct version very soon.


Thanks,
SJ

[...]