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

[PATCH 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.3
- RFC v1.3: https://lore.kernel.org/20260708024855.116614-1-sj@kernel.org
- Drop RFC tag.
Changes from RFC v1.2
- RFC v1.2: https://lore.kernel.org/20260708013252.95888-1-sj@kernel.org
- Fix rebase failure-caused kunit break in patch 1.
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 | 36 ++++++--------------
 5 files changed, 59 insertions(+), 56 deletions(-)


base-commit: aa8ba979c42a7bf2d344a0f3b192c4dc61000096
-- 
2.47.3

[PATCH 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 | 23 -----------------------
 2 files changed, 1 insertion(+), 30 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..278d46632eff4 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -575,28 +575,6 @@ static void damon_test_set_regions(struct kunit *test)
 			}, 3);
 }
 
-static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
-{
-	struct damon_attrs attrs = {
-		.sample_interval = 10,
-		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
-	};
-
-	/*
-	 * 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.");
-
-	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);
-}
-
 static void damon_test_update_monitoring_result(struct kunit *test)
 {
 	struct damon_attrs old_attrs = {
@@ -1572,7 +1550,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

[PATCH 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

[PATCH 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

[PATCH 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

[PATCH 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

[PATCH 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 278d46632eff4..d97280270c27b 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -581,32 +581,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

[PATCH 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: [PATCH 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The functions `damon_sample_bp_to_count` and `damon_sample_count_to_bp` are susceptible to 32-bit integer overflows when evaluating large aggregation intervals.
--

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

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

This commit renames damon_nr_accesses_to_accesses_bp to
damon_sample_count_to_bp to better describe its actual behavior of
converting a positive sample count to a ratio.

> 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 is a pre-existing issue, but does this multiplication overflow if the
total samples in an aggregation interval is large?

If an administrator configures a large aggr_interval compared to
sample_interval, making the sample ratio exceed 429,496, multiplying bp
(up to 10,000) by the number of samples could exceed the 32-bit limit of
unsigned int, causing incorrect bound calculations.

>  
> -/* 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 also a pre-existing issue, but can this use of mult_frac result in
an integer overflow?

The mult_frac macro evaluates (count % samples) * 10000 in 32-bit
arithmetic, which might overflow and produce an incorrect basis point
ratio when the sample count is very large.

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

Re: [PATCH 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:
- [High] Scaling of `probe_hits` during parameter update produces mathematically incorrect results and silent integer truncation due to `unsigned char` type constraints.
--

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

Users can dynamically update subsystem parameters at runtime. This patch adds
logic to appropriately scale probe_hits and last_probe_hits based on the
updated aggregation and sample intervals to maintain accurate monitoring
snapshots.

> 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: High]
Does this code silently truncate the scaled results?

damon_nr_samples_for_new_attrs() returns an unsigned int, but both
last_probe_hits[] and probe_hits[] in struct damon_region are unsigned char
arrays. If the user updates the parameters such that the new ratio of
aggr_interval to sample_interval scales the sample count above 255, the
assignment to unsigned char will silently wrap.

For example, if a hit count is scaled up to 300, the stored value would
become 44. Could this produce mathematically invalid scaling and corrupt the
relative hit proportions that the monitoring results rely on?

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

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

Re: [PATCH 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:

Pre-existing issues:
- [High] 32-bit arithmetic overflow in sample count and basis points conversions leads to incorrect scaling for large interval ratios.
- [High] 32-bit integer overflow in `damon_age_for_new_attrs` when calculating region age during attribute updates on 32-bit architectures.
--

--- Patch [5]: [PATCH 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit 7659397725f64a348908ab982da60d557f63b8c0
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 reflect its general use for any positive
sample count.

> 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 is a pre-existing issue, but does this code overflow 32-bit arithmetic
when the sample count is large?

In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac macro multiplies
the remainder by 10000, which can overflow 32-bit arithmetic.

Similarly, in damon_sample_bp_to_count(), the 32-bit multiplication overflows
since both operands are 32-bit unsigned integers:

mm/damon/core.c:damon_sample_bp_to_count() {
	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
}

This occurs when the number of samples per aggregation interval exceeds
roughly 429,496 (e.g., sample_interval=1us and aggr_interval=1s, yielding
1,000,000 samples).

This could cause the region access tracking variables to be corrupted and
truncated during dynamic attribute updates (damon_set_attrs), rendering DAMOS
memory management schemes dysfunctional.

Both intervals are fully configurable by privileged users via DAMON
sysfs/debugfs interfaces, or dynamically updated by DAMON's auto-tuning
feature, making the overflow reachable in real-world scenarios.

[Severity: High]
This is also a pre-existing issue, but can the region age calculation
overflow on 32-bit architectures during attribute updates?

Looking at damon_age_for_new_attrs(), the multiplication is limited to the
architecture's native unsigned long size:

mm/damon/core.c:damon_age_for_new_attrs() {
	return age * old_attrs->aggr_interval / new_attrs->aggr_interval;
}

On 32-bit architectures, aggr_interval (unsigned long) and age (unsigned int)
are both 32-bit. The product exceeds ULONG_MAX (4,294,967,295) once the
stable time exceeds ~4,294 seconds.

This causes an integer overflow before the division, meaning any memory
region that remains stable for more than ~71.5 minutes will have its age
wrapped around and reset to an incorrectly small value when DAMON
attributes are dynamically updated.

This could break DAMOS schemes that target old pages for reclamation.

The bug is automatically triggered by DAMON's auto-tuning thread
(kdamond_tune_intervals), which periodically calls damon_set_attrs, or via
user-initiated sysfs attribute updates.

> -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);
>  }

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

Re: [PATCH 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 incorrectly references `damon_nr_samples_per_aggr()` instead of `damon_max_nr_accesses()`.
--

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

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

This commit removes an obsolete comment on damon_nr_accesses_to_accesses_bp()
and its associated kunit test. Previous changes guarantee that the interval
calculations will not result in a division-by-zero.

> 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.

[Severity: Medium]
This isn't a bug, but is the function name referenced in the commit
message accurate?

The commit message states that commit 35d4a3cf70a8 modified
damon_nr_samples_per_aggr() to always return non-zero. However, the function
that was modified and used to calculate maximum accesses is
damon_max_nr_accesses().

Could this description be updated to reference damon_max_nr_accesses() to
prevent confusion for future code archaeology and maintenance?

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

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

[SJ Park]

On Wed, 08 Jul 2026 14:13:35 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit message incorrectly references `damon_nr_samples_per_aggr()` instead of `damon_max_nr_accesses()`.
> --
> 
> --- Patch [1]: [PATCH 1/7] mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero ---
> commit f8fbcab0a03e83756cdeec775b4f0bc25f753341
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove comment and test for nr_to_bp() divide-by-zero
> 
> This commit removes an obsolete comment on damon_nr_accesses_to_accesses_bp()
> and its associated kunit test. Previous changes guarantee that the interval
> calculations will not result in a division-by-zero.
> 
> > 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.
> 
> [Severity: Medium]
> This isn't a bug, but is the function name referenced in the commit
> message accurate?
> 
> The commit message states that commit 35d4a3cf70a8 modified
> damon_nr_samples_per_aggr() to always return non-zero. However, the function
> that was modified and used to calculate maximum accesses is
> damon_max_nr_accesses().
> 
> Could this description be updated to reference damon_max_nr_accesses() to
> prevent confusion for future code archaeology and maintenance?

Good finding.  I will ask Andrew to fix when he picks this up, unless I find
more issues that deserve v2 of this series.

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


Thanks,
SJ

Re: [PATCH 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

[SJ Park]

On Wed, 08 Jul 2026 14:03:47 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The functions `damon_sample_bp_to_count` and `damon_sample_count_to_bp` are susceptible to 32-bit integer overflows when evaluating large aggregation intervals.
> --
> 
> --- Patch [4]: [PATCH 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ ---
> commit 75d19025f9846da2b8b1718a4f36d8064a4aa832
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/
> 
> This commit renames damon_nr_accesses_to_accesses_bp to
> damon_sample_count_to_bp to better describe its actual behavior of
> converting a positive sample count to a ratio.
> 
> > 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 is a pre-existing issue, but does this multiplication overflow if the
> total samples in an aggregation interval is large?
> 
> If an administrator configures a large aggr_interval compared to
> sample_interval, making the sample ratio exceed 429,496, multiplying bp
> (up to 10,000) by the number of samples could exceed the 32-bit limit of
> unsigned int, causing incorrect bound calculations.

Known low priority issue.

> 
> >  
> > -/* 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 also a pre-existing issue, but can this use of mult_frac result in
> an integer overflow?
> 
> The mult_frac macro evaluates (count % samples) * 10000 in 32-bit
> arithmetic, which might overflow and produce an incorrect basis point
> ratio when the sample count is very large.

Known low priorrity issue.

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


Thanks,
SJ

Re: [PATCH 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[SJ Park]

On Wed, 08 Jul 2026 14:09:44 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] 32-bit arithmetic overflow in sample count and basis points conversions leads to incorrect scaling for large interval ratios.
> - [High] 32-bit integer overflow in `damon_age_for_new_attrs` when calculating region age during attribute updates on 32-bit architectures.
> --
> 
> --- Patch [5]: [PATCH 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
> commit 7659397725f64a348908ab982da60d557f63b8c0
> 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 reflect its general use for any positive
> sample count.
> 
> > 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 is a pre-existing issue, but does this code overflow 32-bit arithmetic
> when the sample count is large?
> 
> In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac macro multiplies
> the remainder by 10000, which can overflow 32-bit arithmetic.
> 
> Similarly, in damon_sample_bp_to_count(), the 32-bit multiplication overflows
> since both operands are 32-bit unsigned integers:
> 
> mm/damon/core.c:damon_sample_bp_to_count() {
> 	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
> }
> 
> This occurs when the number of samples per aggregation interval exceeds
> roughly 429,496 (e.g., sample_interval=1us and aggr_interval=1s, yielding
> 1,000,000 samples).
> 
> This could cause the region access tracking variables to be corrupted and
> truncated during dynamic attribute updates (damon_set_attrs), rendering DAMOS
> memory management schemes dysfunctional.
> 
> Both intervals are fully configurable by privileged users via DAMON
> sysfs/debugfs interfaces, or dynamically updated by DAMON's auto-tuning
> feature, making the overflow reachable in real-world scenarios.

Known low priority issue.

> 
> [Severity: High]
> This is also a pre-existing issue, but can the region age calculation
> overflow on 32-bit architectures during attribute updates?
> 
> Looking at damon_age_for_new_attrs(), the multiplication is limited to the
> architecture's native unsigned long size:
> 
> mm/damon/core.c:damon_age_for_new_attrs() {
> 	return age * old_attrs->aggr_interval / new_attrs->aggr_interval;
> }
> 
> On 32-bit architectures, aggr_interval (unsigned long) and age (unsigned int)
> are both 32-bit. The product exceeds ULONG_MAX (4,294,967,295) once the
> stable time exceeds ~4,294 seconds.
> 
> This causes an integer overflow before the division, meaning any memory
> region that remains stable for more than ~71.5 minutes will have its age
> wrapped around and reset to an incorrectly small value when DAMON
> attributes are dynamically updated.
> 
> This could break DAMOS schemes that target old pages for reclamation.
> 
> The bug is automatically triggered by DAMON's auto-tuning thread
> (kdamond_tune_intervals), which periodically calls damon_set_attrs, or via
> user-initiated sysfs attribute updates.

Known low priority issue.

> 
> > -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);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708135359.122587-1-sj@kernel.org?part=5


Thanks,
SJ

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

[SJ Park]

On Wed, 08 Jul 2026 14:08:07 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Scaling of `probe_hits` during parameter update produces mathematically incorrect results and silent integer truncation due to `unsigned char` type constraints.
> --
> 
> --- Patch [6]: [PATCH 6/7] mm/damon/core: update probe hits for new parameter commit ---
> commit 6ec1cede99a50d44fcfc3e2fc6a1b5334600120f
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core: update probe hits for new parameter commit
> 
> Users can dynamically update subsystem parameters at runtime. This patch adds
> logic to appropriately scale probe_hits and last_probe_hits based on the
> updated aggregation and sample intervals to maintain accurate monitoring
> snapshots.
> 
> > 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: High]
> Does this code silently truncate the scaled results?
> 
> damon_nr_samples_for_new_attrs() returns an unsigned int, but both
> last_probe_hits[] and probe_hits[] in struct damon_region are unsigned char
> arrays. If the user updates the parameters such that the new ratio of
> aggr_interval to sample_interval scales the sample count above 255, the
> assignment to unsigned char will silently wrap.
> 
> For example, if a hit count is scaled up to 300, the stored value would
> become 44. Could this produce mathematically invalid scaling and corrupt the
> relative hit proportions that the monitoring results rely on?

Known low priority issue.

> 
> > +		else
> > +			r->probe_hits[i] = 0;
> > +		i++;
> > +	}
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708135359.122587-1-sj@kernel.org?part=6


Thanks,
SJ

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

[SJ Park]

On Wed,  8 Jul 2026 06:53:51 -0700 SJ Park <sj@kernel.org> wrote:

> 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

I mistakenly used the future name of damon_max_nr_accesses() on the above
sentence.  Andrew, could you please fix the above sentence by doiing
s/nr_samples_per_aggr/max_nr_accesses/ when you pick this up?

Please let me know if you prefer v2.


Thanks,
SJ

[...]

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

[SJ Park]

On Wed,  8 Jul 2026 06:53:50 -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.

Sashiko found no blocker but a wrong function name [1] on the commit message of
patch 1.  In short, need s/nr_samples_per_aggr/max_nr_accesses/ for the commit
message.  Please read my reply [1] for more detail.

Andrew, as it is a relatively small change on commit message, could you make
the fix when you pick the patch up?  If you prefer v2, please feel free to let
me know.

[1] https://lore.kerrnel.org/20260708143130.124419-1-sj@kernel.org


Thanks,
SJ

[...]