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

[RFC PATCH 0/6] 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
================

Patches 1-4 rename functions that are being used for nr_accesses update,
to be able to be used for probe_hits without confusion.  Patch 5 does
the probe_hits update.  Patch 6 update damon_probe_hits_mvsum() to cover
a corner case from the update for better accuracy.

SJ Park (6):
  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             | 64 ++++++++++++++++++++++++-------------
 mm/damon/lru_sort.c         |  2 +-
 mm/damon/ops-common.c       |  2 +-
 mm/damon/tests/core-kunit.h | 21 ++++++++----
 5 files changed, 63 insertions(+), 36 deletions(-)


base-commit: 9765c7b820c4f11a416d8bc9d402e1914138f64c
-- 
2.47.3

[RFC PATCH 1/6] 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 390e00b3685ef..e710e2789085e 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -880,7 +880,7 @@ 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;
 }
 
 /*
@@ -893,7 +893,7 @@ static unsigned int damon_accesses_bp_to_nr_accesses(
 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 2/6] 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 e710e2789085e..ca530faaa4f83 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;
 }
 
 /*
@@ -899,7 +899,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 3/6] 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.

Also remove the obsolete comment about division-by-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.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index ca530faaa4f83..718268df27fa7 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -883,25 +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).
- *
- * Callers should ensure attrs.aggr_interval is not zero, like
- * damon_update_monitoring_results() does .  Otherwise, divide-by-zero would
- * happen.
- */
-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);
 }
 
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 0124f83b39b83..86eae3a590354 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -575,7 +575,7 @@ 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_sample_count_to_bp(struct kunit *test)
 {
 	struct damon_attrs attrs = {
 		.sample_interval = 10,
@@ -585,7 +585,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
 	/*
 	 * 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
+	 * damon_sample_count_to_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
@@ -594,7 +594,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
 	if (!attrs.aggr_interval)
 		kunit_skip(test, "aggr_interval is zero.");
 
-	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);
+	KUNIT_EXPECT_EQ(test, damon_sample_count_to_bp(123, &attrs), 0);
 }
 
 static void damon_test_update_monitoring_result(struct kunit *test)
@@ -1572,7 +1572,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_accesses_to_accesses_bp),
+	KUNIT_CASE(damon_test_sample_count_to_bp),
 	KUNIT_CASE(damon_test_update_monitoring_result),
 	KUNIT_CASE(damon_test_set_attrs),
 	KUNIT_CASE(damon_test_mvsum),
-- 
2.47.3

[RFC PATCH 4/6] 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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 718268df27fa7..cda318806b65b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -890,7 +890,7 @@ 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_accesses,
 		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
 {
 	return damon_sample_bp_to_count(
@@ -902,10 +902,10 @@ 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 5/6] 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 cda318806b65b..9a3ff0fe55a61 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -898,10 +898,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr_accesses,
 			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)
@@ -941,8 +963,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 86eae3a590354..8e01437eb5ea8 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -603,32 +603,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 6/6] 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 9a3ff0fe55a61..093da5c0b3693 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 3/6] 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:
- [Medium] Obsolete divide-by-zero comment and skip logic were not removed in the KUnit test, contradicting the commit message and core code.
--

--- Patch [3]: [RFC PATCH 3/6] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ ---
commit aaafc980a7ea0e3c63783a3dbd0d43aab65ff523
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 functionality. It also
mentions removing obsolete comments regarding a division-by-zero bug that
was previously fixed.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 0124f83b39b83..86eae3a590354 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
[ ... ]
> @@ -585,7 +585,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>  	/*
>  	 * 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
> +	 * damon_sample_count_to_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
> @@ -594,7 +594,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>  	if (!attrs.aggr_interval)
>  		kunit_skip(test, "aggr_interval is zero.");

[Severity: Medium]
The commit message mentions that the obsolete comment about division-by-zero
is removed because damon_nr_samples_per_aggr() now always returns non-zero.

Should this comment and the associated kunit_skip logic also be removed
from damon_test_sample_count_to_bp()? 

It appears to still reference the caution comment that was removed from the
core code, and it continues to guard against a divide-by-zero that can no
longer happen.

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

Re: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[sashiko-bot]

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

Pre-existing issues:
- [High] Multiplication overflow in DAMON access rate calculations due to 32-bit arithmetic.
--

--- Patch [1]: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ ---
commit 928ef68b58766df0da13e507ad449f7ba9d5c79d
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 that it returns the number of samples DAMON checks for
each region per aggregation interval.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 390e00b3685ef..e710e2789085e 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -880,7 +880,7 @@ 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 isn't a bug introduced by this patch, but can this multiplication in
damon_accesses_bp_to_nr_accesses() overflow 32-bit arithmetic?

If the sample interval is configured to a small value and the aggregation
interval to a large value, damon_nr_samples_per_aggr() can return a large
number. Multiplying accesses_bp by this large number wraps around modulo
2^32 since both operands are 32-bit unsigned integers, resulting in an
incorrectly reduced access count.

>  }
>  
>  /*
> @@ -893,7 +893,7 @@ static unsigned int damon_accesses_bp_to_nr_accesses(
>  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));

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

When nr_accesses is sufficiently large, the internal calculation in
mult_frac() overflows 32-bit arithmetic.

>  }
>  

[ ... ]

> 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 isn't a bug introduced by this patch, but does this calculation in
damon_lru_sort_apply_parameters() overflow on 32-bit architectures?

Multiplying the sample count by hot_thres_access_freq overflows 32-bit
arithmetic on 32-bit systems when the sample count is large.

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

[Severity: High]
This is a pre-existing issue, but could this calculation in
damon_hot_score() result in a 32-bit overflow?

Multiplying r->nr_accesses by DAMON_MAX_SUBSCORE (which is 100) overflows
32-bit arithmetic if the number of samples is near UINT_MAX. This might
cause hot memory to be incorrectly identified as cold.

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

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

Re: [RFC PATCH 2/6] 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] 32-bit integer overflow in conversions between basis points (bp) and sample counts due to large number of samples per aggregation interval.
--

--- Patch [2]: [RFC PATCH 2/6] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/ ---
commit 26f76c0115cf94c7177c6588b38ff64dabf63ab2
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
damon_sample_bp_to_count() to better reflect that it converts a samples ratio
(in basis points) to a count. It also updates the callsites and comments to
match the new name.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index e710e2789085e..ca530faaa4f83 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;

[Severity: High]
This isn't a bug introduced by this patch, but can the product of bp and
damon_nr_samples_per_aggr() overflow 32 bits?

Since both bp and the return value of damon_nr_samples_per_aggr() are 32-bit
unsigned integers, their product can exceed 32 bits if the number of samples
per aggregation interval is large (greater than ~429,496).

For example, a 1ms sample interval and a 10-minute aggregation interval would
result in 600,000 samples. If bp is 10,000 (100%), the multiplication
10,000 * 600,000 overflows 32 bits to 1,705,032,704, resulting in a truncated
count of 170,503 instead of 600,000.

A similar overflow can occur in damon_nr_accesses_to_accesses_bp() when
evaluating (nr_accesses % nr_samples) * 10000 inside mult_frac() for large
nr_accesses values, falsely marking hot memory as cold.

>  }

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

Re: [RFC PATCH 4/6] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] 32-bit integer overflow in damon_sample_bp_to_count() and damon_sample_count_to_bp() corrupts memory access tracking.
--

--- Patch [4]: [RFC PATCH 4/6] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit cb8a54b34f772f70862ab31ccf30eea4a5cb2263
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 its general use for any
positive sample count.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 718268df27fa7..cda318806b65b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -890,7 +890,7 @@ 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 can this math silently overflow when the
sample count is large?

In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac() macro inherits
the 32-bit precision of 'count'. If the user configures sysfs such that max
samples exceeds ~429,496, the intermediate remainder multiplication can wrap
around UINT_MAX.

For instance, 450,000 accesses out of 500,000 maximum samples will silently
overflow during mult_frac() calculation, resulting in a basis point of 410
(4.1% hot) instead of 9000 (90% hot).

Similarly, in mm/damon/core.c:damon_sample_bp_to_count():

    static unsigned int damon_sample_bp_to_count(
            unsigned int bp, struct damon_attrs *attrs)
    {
        return bp * damon_nr_samples_per_aggr(attrs) / 10000;
    }

The multiplication occurs in 32-bit precision before the division by 10000.
If damon_nr_samples_per_aggr() exceeds ~429,496, this will also overflow.

This drastic corruption could cause DAMOS to incorrectly evaluate region
hotness, potentially swapping out highly accessed memory. Would it make sense
to cast these to a 64-bit type before multiplication to avoid the overflow?

> -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_accesses,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
>  {
>  	return damon_sample_bp_to_count(

[ ... ]

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

Re: [RFC PATCH 5/6] 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:

Pre-existing issues:
- [High] 32-bit arithmetic overflow in interval scaling functions corrupts core metrics.
--

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

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

Users can update DAMON parameters at runtime. If the samples and/or
aggregation intervals are updated, monitoring results depending on the
intervals should also be updated. This commit ensures probe_hits are
properly updated to reflect new intervals, similar to age and nr_accesses.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index cda318806b65b..9a3ff0fe55a61 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -898,10 +898,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr_accesses,
>  			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);

[Severity: High]
This is a pre-existing issue, but can the scaling logic called here
overflow and corrupt the core metrics?

Looking at the underlying functions called by
damon_nr_samples_for_new_attrs():

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

and

mm/damon/core.c:damon_sample_count_to_bp() {
    return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
}

If a user configures a large aggregation to sampling interval ratio
(e.g., 1 second aggregation and 1 microsecond sampling), the ratio can
exceed 429K. 

When damon_sample_count_to_bp() scales a count of 500,000, the internal
multiplication in mult_frac() (count * 10000) evaluates to 5,000,000,000,
which overflows the 32-bit unsigned integer maximum and yields an incorrect
value.

Similarly, damon_sample_bp_to_count() performs 32-bit multiplication
which can overflow if bp * samples exceeds the 32-bit limit.

Should these scaling calculations cast to 64-bit to prevent silent
wrap-around and wildly inaccurate statistics?

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

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

Re: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[SJ Park]

On Tue, 07 Jul 2026 04:48:40 +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] Multiplication overflow in DAMON access rate calculations due to 32-bit arithmetic.
> --
> 
> --- Patch [1]: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ ---
> commit 928ef68b58766df0da13e507ad449f7ba9d5c79d
> 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 that it returns the number of samples DAMON checks for
> each region per aggregation interval.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 390e00b3685ef..e710e2789085e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -880,7 +880,7 @@ 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 isn't a bug introduced by this patch, but can this multiplication in
> damon_accesses_bp_to_nr_accesses() overflow 32-bit arithmetic?
> 
> If the sample interval is configured to a small value and the aggregation
> interval to a large value, damon_nr_samples_per_aggr() can return a large
> number. Multiplying accesses_bp by this large number wraps around modulo
> 2^32 since both operands are 32-bit unsigned integers, resulting in an
> incorrectly reduced access count.

Known low priority issue.

> 
> >  }
> >  
> >  /*
> > @@ -893,7 +893,7 @@ static unsigned int damon_accesses_bp_to_nr_accesses(
> >  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));
> 
> [Severity: High]
> This is a pre-existing issue, but does the use of mult_frac() in
> damon_nr_accesses_to_accesses_bp() also suffer from a 32-bit overflow?
> 
> When nr_accesses is sufficiently large, the internal calculation in
> mult_frac() overflows 32-bit arithmetic.

Known low priority issue.

> 
> >  }
> >  
> 
> [ ... ]
> 
> > 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 isn't a bug introduced by this patch, but does this calculation in
> damon_lru_sort_apply_parameters() overflow on 32-bit architectures?
> 
> Multiplying the sample count by hot_thres_access_freq overflows 32-bit
> arithmetic on 32-bit systems when the sample count is large.

Known low priority issue.

> 
> >  	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);
> 
> [Severity: High]
> This is a pre-existing issue, but could this calculation in
> damon_hot_score() result in a 32-bit overflow?
> 
> Multiplying r->nr_accesses by DAMON_MAX_SUBSCORE (which is 100) overflows
> 32-bit arithmetic if the number of samples is near UINT_MAX. This might
> cause hot memory to be incorrectly identified as cold.

Known low priority issue.

> 
> >  
> >  	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
> >  	if (age_in_sec)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260707043828.97900-1-sj@kernel.org?part=1


Thanks,
SJ

[...]

Re: [RFC PATCH 2/6] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/

[SJ Park]

On Tue, 07 Jul 2026 04:49:06 +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] 32-bit integer overflow in conversions between basis points (bp) and sample counts due to large number of samples per aggregation interval.
> --
> 
> --- Patch [2]: [RFC PATCH 2/6] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/ ---
> commit 26f76c0115cf94c7177c6588b38ff64dabf63ab2
> 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
> damon_sample_bp_to_count() to better reflect that it converts a samples ratio
> (in basis points) to a count. It also updates the callsites and comments to
> match the new name.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index e710e2789085e..ca530faaa4f83 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;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can the product of bp and
> damon_nr_samples_per_aggr() overflow 32 bits?
> 
> Since both bp and the return value of damon_nr_samples_per_aggr() are 32-bit
> unsigned integers, their product can exceed 32 bits if the number of samples
> per aggregation interval is large (greater than ~429,496).
> 
> For example, a 1ms sample interval and a 10-minute aggregation interval would
> result in 600,000 samples. If bp is 10,000 (100%), the multiplication
> 10,000 * 600,000 overflows 32 bits to 1,705,032,704, resulting in a truncated
> count of 170,503 instead of 600,000.
> 
> A similar overflow can occur in damon_nr_accesses_to_accesses_bp() when
> evaluating (nr_accesses % nr_samples) * 10000 inside mult_frac() for large
> nr_accesses values, falsely marking hot memory as cold.

Known low priority issue.

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


Thanks,
SJ

[...]

Re: [RFC PATCH 3/6] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

[SJ Park]

On Tue, 07 Jul 2026 04:45:43 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Obsolete divide-by-zero comment and skip logic were not removed in the KUnit test, contradicting the commit message and core code.
> --
> 
> --- Patch [3]: [RFC PATCH 3/6] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ ---
> commit aaafc980a7ea0e3c63783a3dbd0d43aab65ff523
> 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 functionality. It also
> mentions removing obsolete comments regarding a division-by-zero bug that
> was previously fixed.
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 0124f83b39b83..86eae3a590354 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> [ ... ]
> > @@ -585,7 +585,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
> >  	/*
> >  	 * 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
> > +	 * damon_sample_count_to_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
> > @@ -594,7 +594,7 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
> >  	if (!attrs.aggr_interval)
> >  		kunit_skip(test, "aggr_interval is zero.");
> 
> [Severity: Medium]
> The commit message mentions that the obsolete comment about division-by-zero
> is removed because damon_nr_samples_per_aggr() now always returns non-zero.
> 
> Should this comment and the associated kunit_skip logic also be removed
> from damon_test_sample_count_to_bp()? 
> 
> It appears to still reference the caution comment that was removed from the
> core code, and it continues to guard against a divide-by-zero that can no
> longer happen.

Good finding.  I will do so in the next revision.

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


Thanks,
SJ

[...]

Re: [RFC PATCH 4/6] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[SJ Park]

On Tue, 07 Jul 2026 04:50:18 +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] 32-bit integer overflow in damon_sample_bp_to_count() and damon_sample_count_to_bp() corrupts memory access tracking.
> --
> 
> --- Patch [4]: [RFC PATCH 4/6] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
> commit cb8a54b34f772f70862ab31ccf30eea4a5cb2263
> 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 its general use for any
> positive sample count.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 718268df27fa7..cda318806b65b 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -890,7 +890,7 @@ 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 can this math silently overflow when the
> sample count is large?
> 
> In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac() macro inherits
> the 32-bit precision of 'count'. If the user configures sysfs such that max
> samples exceeds ~429,496, the intermediate remainder multiplication can wrap
> around UINT_MAX.
> 
> For instance, 450,000 accesses out of 500,000 maximum samples will silently
> overflow during mult_frac() calculation, resulting in a basis point of 410
> (4.1% hot) instead of 9000 (90% hot).
> 
> Similarly, in mm/damon/core.c:damon_sample_bp_to_count():
> 
>     static unsigned int damon_sample_bp_to_count(
>             unsigned int bp, struct damon_attrs *attrs)
>     {
>         return bp * damon_nr_samples_per_aggr(attrs) / 10000;
>     }
> 
> The multiplication occurs in 32-bit precision before the division by 10000.
> If damon_nr_samples_per_aggr() exceeds ~429,496, this will also overflow.
> 
> This drastic corruption could cause DAMOS to incorrectly evaluate region
> hotness, potentially swapping out highly accessed memory. Would it make sense
> to cast these to a 64-bit type before multiplication to avoid the overflow?

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_accesses,
> >  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
> >  {
> >  	return damon_sample_bp_to_count(
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260707043828.97900-1-sj@kernel.org?part=4


Thanks,
SJ

[...]

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

[SJ Park]

On Tue, 07 Jul 2026 04:51:51 +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] 32-bit arithmetic overflow in interval scaling functions corrupts core metrics.
> --
> 
> --- Patch [5]: [RFC PATCH 5/6] mm/damon/core: update probe hits for new parameter commit ---
> commit 7e26d9c01ec3d36d75d519b11f48ef36f78ec868
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: update probe hits for new parameter commit
> 
> Users can update DAMON parameters at runtime. If the samples and/or
> aggregation intervals are updated, monitoring results depending on the
> intervals should also be updated. This commit ensures probe_hits are
> properly updated to reflect new intervals, similar to age and nr_accesses.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index cda318806b65b..9a3ff0fe55a61 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -898,10 +898,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr_accesses,
> >  			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);
> 
> [Severity: High]
> This is a pre-existing issue, but can the scaling logic called here
> overflow and corrupt the core metrics?
> 
> Looking at the underlying functions called by
> damon_nr_samples_for_new_attrs():
> 
> mm/damon/core.c:damon_sample_bp_to_count() {
>     return bp * damon_nr_samples_per_aggr(attrs) / 10000;
> }
> 
> and
> 
> mm/damon/core.c:damon_sample_count_to_bp() {
>     return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
> }
> 
> If a user configures a large aggregation to sampling interval ratio
> (e.g., 1 second aggregation and 1 microsecond sampling), the ratio can
> exceed 429K. 
> 
> When damon_sample_count_to_bp() scales a count of 500,000, the internal
> multiplication in mult_frac() (count * 10000) evaluates to 5,000,000,000,
> which overflows the 32-bit unsigned integer maximum and yields an incorrect
> value.
> 
> Similarly, damon_sample_bp_to_count() performs 32-bit multiplication
> which can overflow if bp * samples exceeds the 32-bit limit.
> 
> Should these scaling calculations cast to 64-bit to prevent silent
> wrap-around and wildly inaccurate statistics?

Known low priority issue.

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


Thanks,
SJ

[...]