[RFC PATCH v1.2 00/17] mm/damon: optimize out nr_accesses_bp

[RFC PATCH v1.2 00/17] mm/damon: optimize out nr_accesses_bp

[SeongJae Park]

TLDR: Replace damon_region->nr_accesses_bp, which is easy to be wrong,
with a simpler on-demand moving sum function, damon_nr_accesses_mvsum().

Background
==========

DAMON's monitoring output (access pattern snapshot, or more technically
speaking, damon_region->nr_accesses) is completed once per aggregation
interval, which is 100 ms by default.  Users can arbitrarily increase
the interval for demand.  Under the suggested intervals auto-tuning
setup, it can span up to 200 seconds.  If the aggregation interval is
too long, the snapshot users cannot use it in reasonable time.  To
mitigate this, we introduced a new field of damon_region, namely
nr_accesses_bp.  It contains a pseudo moving sum of nr_accesses in bp
units and is updated for each sampling interval.

It turned out keeping it correctly updated every sampling interval is
not that easy.  From online parameter update feature development and
more experimental hacks, we found it is easy to be corrupted.  Once it
is corrupted, DAMON's monitoring outputs become quite insane.  Hence we
added a few validation checks.  It is easy  to be corrupted because it
requires every update per sampling interval to be correct.

Solution
========

There is no real reason to keep it updated every sampling interval.  Due
to the simple pseudo-moving sum mechanism and existing helper field
(last_nr_accesses), we can also calculate the pseudo moving sum on
demand in a much simpler way.

Implement a function for getting the pseudo moving sum on demand, and
replace nr_accessses_bp uses with the new function.  Also remove no more
needed tests for nr_accesses_bp and the per-sampling interval update
functions.  Finally, remove the nr_accesses_bp.  The new function is
quite simple.

Discussion
==========

Depending on the use case, multiple nr_accesses readers could be
executed in the same kdamond_fn() main loop iteration, which is executed
once per sampling interval.  Such readers include DAMON region exporting
tracepoints (damon_[region_]aggregated and damos_before_apply), DAMOS,
and DAMON sysfs interface logic for update_schemes_tried_regions
command.  In this case, the new function will be called multiple times
and this could be overhead compared to the old logic, which simply reads
the field without any additional work.  Nonetheless, the new function is
quite simple.  And the new approach does nothing while there is no need
to read.  The old approach had to execute its update function for each
region for every sampling interval.  Hence the new approach is believed
to be even more lightweight in common case, and the overhead is anyway
negligible.

One more advantage of this change is that one field from the
damon_region struct is removed.  On setups that uses a high number of
DAMON regions, this could be a potential memory space benefit.

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

Patch 1 introduces the new function for getting the pseudo moving sum of
nr_accesses on demands.  Patch 2 implements a unit test for the new
function's internal logic.  Patch 3 updates monitoring logic to ready
for safe use of the new function.  Patches 4-6 replace uses of
nr_accesses_bp in DAMOS, tracepoints and DAMON sysfs interface with the
new function, respectively.  Patches 7-9 removes nr_accesses_bp
validation functions in DAMON core, one by one.  Patches 10 and 11
further remove tests and test helper for nr_accesses_bp, respectively.
Patches 12 removes the setups and updates or nr_accesses_bp field.
Patches 13-15 cleans up function parameters that are no more being used
due to the previous patch.  Patch 16 removes the function that was used
for updating nr_accesses_bp field with its unit test, which is the
single remaining caller of the function.  Finally, patch 17 removes
damon_region->nr_accesses_bp field.

Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/20260620172244.90953-1-sj@kernel.org
- Handle next_aggregation_sis < passed_sample_intervals in
  nr_accesses_mvsum().
- Always rescale ->last_nr_accesss for intervals change.
- Remove unused attrs params from damon_update_region_access_rate() and
  its callers.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/20260619193415.73833-1-sj@kernel.org
- Avoid divide-by-zero from zero aggregation interval.
- Call damon_nr_accesses_mvsum() for damos tracing only when it is enabled.
- Remove obsolete mentions of nr_accesses_bp in comments.

SeongJae Park (17):
  mm/damon: introduce damon_nr_accesses_mvsum()
  mm/damon/tests/core-kunit: test damon_mvsum()
  mm/damon/core: always update ->last_nr_accesses for intervals change
  mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()
  mm/damon/core: use damon_nr_accesses_mvsum() for damos region tracing
  mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions
  mm/damon/core: remove damon_warn_fix_nr_accesses_corruption()
  mm/damon/core: remove damon_verify_reset_aggregated()
  mm/damon/core: remove damon_verify_merge_regions_of()
  mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests
  selftests/damon/drgn_dump_damon_status: do not dump nr_accesses_bp
  mm/damon/core: remove nr_accesses_bp setups and updates
  mm/damon/core: remove attrs param from
    damon_update_region_access_rate()
  mm/damonn/paddr: remove attrs param from __damon_pa_check_access()
  mm/damon/vaddr: remove attrs param from __damon_va_check_access()
  mm/damon/core: remove damon_moving_sum() and its unit test
  mm/damon: remove damon_region->nr_accesses_bp

 include/linux/damon.h                         |  15 +-
 include/trace/events/damon.h                  |   8 +-
 mm/damon/core.c                               | 198 +++++++-----------
 mm/damon/paddr.c                              |   9 +-
 mm/damon/sysfs-schemes.c                      |   6 +-
 mm/damon/tests/core-kunit.h                   |  37 ++--
 mm/damon/vaddr.c                              |  12 +-
 .../selftests/damon/drgn_dump_damon_status.py |   1 -
 8 files changed, 116 insertions(+), 170 deletions(-)


base-commit: 6b4f924f70f92679c81959ad6b9234242aab74a7
-- 
2.47.3

[RFC PATCH v1.2 01/17] mm/damon: introduce damon_nr_accesses_mvsum()

[SeongJae Park]

Introduce a new DAMON core function, damon_nr_accesses_mvsum().  It
returns a pseudo moving sum value of a given region's nr_accesses for
the last aggregation interval.  The internal logic is the same to
nr_accesses_bp.  The difference is that nr_accesses_bp is updated for
each sampling interval, while the new function needs to be executed only
when requested.  Hence the function's return value is the same as the
value of nr_accesses_bp.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h |  2 ++
 mm/damon/core.c       | 62 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 02ac34537df9a..55a743d99b567 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1013,6 +1013,8 @@ struct damon_probe *damon_new_probe(void);
 void damon_add_probe(struct damon_ctx *ctx, struct damon_probe *probe);
 
 struct damon_region *damon_new_region(unsigned long start, unsigned long end);
+unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
+		struct damon_ctx *ctx);
 
 int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 		unsigned int nr_ranges, unsigned long min_region_sz);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 5b84b3ce3fcff..2cc911fa221aa 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -210,6 +210,68 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
 	return NULL;
 }
 
+/*
+ * damon_mvsum() - Returns pseudo moving sum value for a time window.
+ * @current_nr:		The value of the current aggregation window.
+ * @last_nr:		The value of the last aggregation window.
+ * @left_window_bp:	Left time of the current aggregation window.
+ *
+ * This function calculates a pseudo moving sum value of a counter that is
+ * aggregated for each time window.  @current_nr is the value of the counter
+ * that aggregated so far (maybe not yet complete), from the beginning of the
+ * current aggregation time window.  @last_nr is the value of the counter that
+ * has completely aggregated in the last aggregation time window.
+ * @left_window_bp represents how much time is left for the current aggregation
+ * time window in bp (1/10,000).  For example, the aggregation time window is
+ * for every 10 seconds and 7 seconds has passed since the beginning of the
+ * current window, this parameter will be 3000 ((10 - 7) / 10 * 10000).
+ *
+ * The logic assumes the aggregation in the last phase was made in a single
+ * speed.  Based on the assumption, the value from the last window that needs
+ * to be added to the current value is calculated as a portion of the last
+ * value based on the remaining time window.
+ */
+static unsigned long damon_mvsum(unsigned long current_nr,
+		unsigned long last_nr, unsigned long left_window_bp)
+{
+	return current_nr + mult_frac(last_nr, left_window_bp, 10000);
+}
+
+/**
+ * damon_nr_accesses_mvsum() - Returns moving sum access frequency score.
+ * @r:		Region to get the access frequency of.
+ * @ctx:	DAMON context of @r.
+ *
+ * This function returns for how many sampling iterations in the last
+ * aggregation interval (&damon_attrs->aggr_interval) the region was found to
+ * be accessed.  Hence the value can be interpreted as the relative access
+ * frequency score of the region (@r).  The value is calculated as a pseudo
+ * moving sum, and hence it is not an exact value but just a best-effort
+ * reasonable estimation.
+ *
+ * Return: the pseudo moving sum access frequency score.
+ */
+unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
+		struct damon_ctx *ctx)
+{
+	unsigned long sample_interval, aggr_interval;
+	unsigned long window_len, left_window, left_window_bp;
+
+	sample_interval = ctx->attrs.sample_interval ? : 1;
+	aggr_interval = ctx->attrs.aggr_interval ? : 1;
+	window_len = aggr_interval / sample_interval;
+	if (time_after_eq(ctx->passed_sample_intervals,
+				ctx->next_aggregation_sis))
+		left_window = 0;
+	else
+		left_window = ctx->next_aggregation_sis -
+			ctx->passed_sample_intervals;
+	left_window_bp = mult_frac(left_window, 10000, window_len);
+
+	return damon_mvsum(r->nr_accesses, r->last_nr_accesses,
+			left_window_bp);
+}
+
 #ifdef CONFIG_DAMON_DEBUG_SANITY
 static void damon_verify_new_region(unsigned long start, unsigned long end)
 {
-- 
2.47.3

[RFC PATCH v1.2 02/17] mm/damon/tests/core-kunit: test damon_mvsum()

[SeongJae Park]

Add a simple unit test for damon_nr_accesses_mvsum()'s internal core
logic, damon_mvsum().  The test contains cases for just-started windows,
partially completed windows, and just-completed windows.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 4e448c08c724a..cdab14396250f 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -623,6 +623,30 @@ static void damon_test_moving_sum(struct kunit *test)
 	}
 }
 
+static void damon_test_mvsum(struct kunit *test)
+{
+	unsigned long input_expects[] = {
+		/* current value, last value, remaining window (bp) */
+		0, 49, 10000, 49,	/* 0 + 49 * 1 */
+		3, 10, 7000, 10,	/* 3 + 10 * 0.7 */
+		3, 10, 5000, 8,		/* 3 + 10 * 0.5 */
+		32, 100, 1000, 42,	/* 32 + 100 * 0.1 */
+		42, 49, 0, 42,		/* 42 + 49 * 0 */
+	};
+
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(input_expects); i += 4) {
+		unsigned long current_nr = input_expects[i];
+		unsigned long last_nr = input_expects[i + 1];
+		unsigned long left_window_bp = input_expects[i + 2];
+		unsigned long expect = input_expects[i + 3];
+
+		KUNIT_EXPECT_EQ(test, damon_mvsum(current_nr, last_nr,
+					left_window_bp), expect);
+	}
+}
+
 static void damos_test_new_filter(struct kunit *test)
 {
 	struct damos_filter *filter;
@@ -1501,6 +1525,7 @@ static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_update_monitoring_result),
 	KUNIT_CASE(damon_test_set_attrs),
 	KUNIT_CASE(damon_test_moving_sum),
+	KUNIT_CASE(damon_test_mvsum),
 	KUNIT_CASE(damos_test_new_filter),
 	KUNIT_CASE(damos_test_commit_quota_goal),
 	KUNIT_CASE(damos_test_commit_quota_goals),
-- 
2.47.3

[RFC PATCH v1.2 03/17] mm/damon/core: always update ->last_nr_accesses for intervals change

[SeongJae Park]

Each iteration of kdamond_fn() main loop caches and use the next
aggregation time (next_aggregation_sis) because it can be updated in the
middle, inside kdamond_call().  If that happens,
damon_update_monitoring_result() is called for scaling the access
frequency information of each region according to the changed intervals.
The function does not update damon_region->last_nr_accesses when it is
at the end of the aggregation, because it will anyway be reset after the
function is executed, in kdamond_reset_aggregated().

Let's suppose damon_nr_accesses_mvsum() is called with the not yet
updated last_nr_accesses.  It will use the fresh next_aggregation_sis in
the context instead of the cached one, unlike kdamond_fn().  As a
result, use of not updated last_nr_acceses with the updated
next_aggregation_sis result in returning wrong value.

There is no such damon_nr_accesses_nvsum() call at the moment, so this
is no problem.  It is planned to add such calls, though.  Prevent the
issue by updating last_nr_accesses always.  This adds overhead, but
that's fine because the overhead is not big, and it is anyway not a fast
path.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 2cc911fa221aa..191533685cf2f 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -873,6 +873,8 @@ 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, old_attrs, new_attrs);
 	if (!aggregating) {
 		r->nr_accesses = damon_nr_accesses_for_new_attrs(
 				r->nr_accesses, old_attrs, new_attrs);
@@ -884,8 +886,6 @@ static void damon_update_monitoring_result(struct damon_region *r,
 		 * interval.  In other words, make the status like
 		 * kdamond_reset_aggregated() is called.
 		 */
-		r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
-				r->last_nr_accesses, old_attrs, new_attrs);
 		r->nr_accesses_bp = r->last_nr_accesses * 10000;
 		r->nr_accesses = 0;
 	}
-- 
2.47.3

[RFC PATCH v1.2 04/17] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

[SeongJae Park]

damon_nr_accesses_mvsum() returns a value same to nr_accesses_bp.  Also
the function is more simple and therefore more tolerant to errors.
Execution of the function would be more expensive than the simple read
of the field, but because the function is quite simple, the overhead
should be negligible.  Use it in __damos_valid_target() instead of the
nr_accesses_bp.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 191533685cf2f..8f845bf698b2d 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2125,10 +2125,11 @@ static noinline_for_stack void kdamond_tune_intervals(struct damon_ctx *c)
 	damon_set_attrs(c, &new_attrs);
 }
 
-static bool __damos_valid_target(struct damon_region *r, struct damos *s)
+static bool __damos_valid_target(struct damon_region *r, struct damos *s,
+		struct damon_ctx *c)
 {
 	unsigned long sz;
-	unsigned int nr_accesses = r->nr_accesses_bp / 10000;
+	unsigned int nr_accesses = damon_nr_accesses_mvsum(r, c);
 
 	sz = damon_sz_region(r);
 	return s->pattern.min_sz_region <= sz &&
@@ -2154,7 +2155,7 @@ static bool damos_quota_is_set(struct damos_quota *quota)
 static bool damos_valid_target(struct damon_ctx *c, struct damon_region *r,
 		struct damos *s)
 {
-	bool ret = __damos_valid_target(r, s);
+	bool ret = __damos_valid_target(r, s, c);
 
 	if (!ret || !damos_quota_is_set(&s->quota) || !c->ops.get_scheme_score)
 		return ret;
@@ -2740,7 +2741,7 @@ static phys_addr_t damos_calc_eligible_bytes(struct damon_ctx *c,
 		damon_for_each_region(r, t) {
 			phys_addr_t addr, end_addr;
 
-			if (!__damos_valid_target(r, s))
+			if (!__damos_valid_target(r, s, c))
 				continue;
 
 			/* Convert from core address units to physical bytes */
@@ -3029,7 +3030,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 			(DAMOS_MAX_SCORE + 1));
 	damon_for_each_target(t, c) {
 		damon_for_each_region(r, t) {
-			if (!__damos_valid_target(r, s))
+			if (!__damos_valid_target(r, s, c))
 				continue;
 			if (damos_core_filter_out(c, t, r, s))
 				continue;
-- 
2.47.3

[RFC PATCH v1.2 05/17] mm/damon/core: use damon_nr_accesses_mvsum() for damos region tracing

[SeongJae Park]

damon_nr_accesses_mvsum() returns a value same to nr_accesses_bp.  Also
the function is more simple and therefore more tolerant to errors.
Execution of the function would be more expensive than the simple read
of the field, but because the function is quite simple, the overhead
should be negligible.  Use it in the DAMON region exporting trace points
instead of the nr_accesses_bp.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/trace/events/damon.h | 8 +++++---
 mm/damon/core.c              | 5 +++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index 78388538acf44..8851727ae1627 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -78,9 +78,11 @@ TRACE_EVENT_CONDITION(damos_before_apply,
 
 	TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
 		unsigned int target_idx, struct damon_region *r,
-		unsigned int nr_regions, bool do_trace),
+		unsigned int nr_accesses, unsigned int nr_regions,
+		bool do_trace),
 
-	TP_ARGS(context_idx, scheme_idx, target_idx, r, nr_regions, do_trace),
+	TP_ARGS(context_idx, scheme_idx, target_idx, r, nr_accesses,
+		nr_regions, do_trace),
 
 	TP_CONDITION(do_trace),
 
@@ -101,7 +103,7 @@ TRACE_EVENT_CONDITION(damos_before_apply,
 		__entry->target_idx = target_idx;
 		__entry->start = r->ar.start;
 		__entry->end = r->ar.end;
-		__entry->nr_accesses = r->nr_accesses_bp / 10000;
+		__entry->nr_accesses = nr_accesses;
 		__entry->age = r->age;
 		__entry->nr_regions = nr_regions;
 	),
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 8f845bf698b2d..91f137901e726 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2439,7 +2439,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 	struct damos *siter;		/* schemes iterator */
 	unsigned int sidx = 0;
 	struct damon_target *titer;	/* targets iterator */
-	unsigned int tidx = 0;
+	unsigned int tidx = 0, nr_accesses = 0;
 	bool do_trace = false;
 
 	/* get indices for trace_damos_before_apply() */
@@ -2454,6 +2454,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 				break;
 			tidx++;
 		}
+		nr_accesses = damon_nr_accesses_mvsum(r, c);
 		do_trace = true;
 	}
 
@@ -2469,7 +2470,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 		if (damos_core_filter_out(c, t, r, s))
 			return;
 		ktime_get_coarse_ts64(&begin);
-		trace_damos_before_apply(cidx, sidx, tidx, r,
+		trace_damos_before_apply(cidx, sidx, tidx, r, nr_accesses,
 				damon_nr_regions(t), do_trace);
 		sz_applied = c->ops.apply_scheme(c, t, r, s,
 				&sz_ops_filter_passed);
-- 
2.47.3

[RFC PATCH v1.2 06/17] mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions

[SeongJae Park]

damon_nr_accesses_mvsum() returns a value same to nr_accesses_bp.  Also
the function is more simple and therefore more tolerant to errors.
Execution of the function would be more expensive than the simple read
of the field, but because the function is quite simple, the overhead
should be negligible.  Use it in the DAMON sysfs interface for
scheme-tried regions, instead of the nr_accesses_bp.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/sysfs-schemes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 9d8fab32b80f7..30a007bcf82f4 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -159,7 +159,7 @@ struct damon_sysfs_scheme_region {
 };
 
 static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
-		struct damon_region *region)
+		struct damon_region *region, struct damon_ctx *ctx)
 {
 	struct damon_sysfs_scheme_region *sysfs_region = kmalloc_obj(*sysfs_region);
 
@@ -167,7 +167,7 @@ static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
 		return NULL;
 	sysfs_region->kobj = (struct kobject){};
 	sysfs_region->ar = region->ar;
-	sysfs_region->nr_accesses = region->nr_accesses_bp / 10000;
+	sysfs_region->nr_accesses = damon_nr_accesses_mvsum(region, ctx);
 	sysfs_region->age = region->age;
 	sysfs_region->probes = NULL;
 	INIT_LIST_HEAD(&sysfs_region->list);
@@ -3124,7 +3124,7 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
 	if (total_bytes_only)
 		return;
 
-	region = damon_sysfs_scheme_region_alloc(r);
+	region = damon_sysfs_scheme_region_alloc(r, ctx);
 	if (!region)
 		return;
 	region->sz_filter_passed = sz_filter_passed;
-- 
2.47.3

[RFC PATCH v1.2 07/17] mm/damon/core: remove damon_warn_fix_nr_accesses_corruption()

[SeongJae Park]

nr_accesses_bp is delicate.  Once it is corrupted, the consequence is
the bad madness of DAMON monitoring results.  From developments of
features of size, we historically found nr_accesses_bp can be corrupted
by complicated bugs that are not easy to debug.  Hence we added a
function for finding the corruption and fixing it right away.

There are no more uses of nr_accesses_bp.  Hence the function for
corruption detection and fix is no more needed.  Rip it out.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 91f137901e726..1a10eb0694aca 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1999,19 +1999,6 @@ int damos_walk(struct damon_ctx *ctx, struct damos_walk_control *control)
 	return 0;
 }
 
-/*
- * Warn and fix corrupted ->nr_accesses[_bp] for investigations and preventing
- * the problem being propagated.
- */
-static void damon_warn_fix_nr_accesses_corruption(struct damon_region *r)
-{
-	if (r->nr_accesses_bp == r->nr_accesses * 10000)
-		return;
-	WARN_ONCE(true, "invalid nr_accesses_bp at reset: %u %u\n",
-			r->nr_accesses_bp, r->nr_accesses);
-	r->nr_accesses_bp = r->nr_accesses * 10000;
-}
-
 #ifdef CONFIG_DAMON_DEBUG_SANITY
 static void damon_verify_reset_aggregated(struct damon_region *r,
 		struct damon_ctx *c)
@@ -2053,7 +2040,6 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 			trace_damon_aggregated(ti, r, damon_nr_regions(t));
 			trace_damon_region_aggregated(ti, r,
 					damon_nr_regions(t), nr_probes);
-			damon_warn_fix_nr_accesses_corruption(r);
 			r->last_nr_accesses = r->nr_accesses;
 			r->nr_accesses = 0;
 			for (i = 0; i < DAMON_MAX_PROBES; i++)
-- 
2.47.3

[RFC PATCH v1.2 08/17] mm/damon/core: remove damon_verify_reset_aggregated()

[SeongJae Park]

nr_accesse_bp is no longer being used in real use cases.  Remove its
validation function.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 1a10eb0694aca..7bb9b3c32d6fc 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1999,23 +1999,6 @@ int damos_walk(struct damon_ctx *ctx, struct damos_walk_control *control)
 	return 0;
 }
 
-#ifdef CONFIG_DAMON_DEBUG_SANITY
-static void damon_verify_reset_aggregated(struct damon_region *r,
-		struct damon_ctx *c)
-{
-	WARN_ONCE(r->nr_accesses_bp != r->last_nr_accesses * 10000,
-			"nr_accesses_bp %u last_nr_accesses %u sis %lu %lu\n",
-			r->nr_accesses_bp, r->last_nr_accesses,
-			c->passed_sample_intervals, c->next_aggregation_sis);
-}
-#else
-static void damon_verify_reset_aggregated(struct damon_region *r,
-		struct damon_ctx *c)
-{
-}
-#endif
-
-
 /*
  * Reset the aggregated monitoring results ('nr_accesses' of each region).
  */
@@ -2044,7 +2027,6 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 			r->nr_accesses = 0;
 			for (i = 0; i < DAMON_MAX_PROBES; i++)
 				r->probe_hits[i] = 0;
-			damon_verify_reset_aggregated(r, c);
 		}
 		ti++;
 	}
-- 
2.47.3

[RFC PATCH v1.2 09/17] mm/damon/core: remove damon_verify_merge_regions_of()

[SeongJae Park]

damon_verify_merge_regions_of() is only for nr_accesses_bp validation.
But nr_accesses_bp is no more being used for a real purpose.  Remove the
validation.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7bb9b3c32d6fc..76ac58bf3dff9 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3114,20 +3114,6 @@ static void damon_merge_two_regions(struct damon_target *t,
 	damon_destroy_region(r, t);
 }
 
-#ifdef CONFIG_DAMON_DEBUG_SANITY
-static void damon_verify_merge_regions_of(struct damon_region *r)
-{
-	WARN_ONCE(r->nr_accesses != r->nr_accesses_bp / 10000,
-			"nr_accesses (%u) != nr_accesses_bp (%u)\n",
-			r->nr_accesses, r->nr_accesses_bp);
-}
-#else
-static void damon_verify_merge_regions_of(struct damon_region *r)
-{
-}
-#endif
-
-
 /*
  * Merge adjacent regions having similar access frequencies
  *
@@ -3141,7 +3127,6 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
 	struct damon_region *r, *prev = NULL, *next;
 
 	damon_for_each_region_safe(r, next, t) {
-		damon_verify_merge_regions_of(r);
 		if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
 			r->age = 0;
 		else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
-- 
2.47.3

[RFC PATCH v1.2 10/17] mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests

[SeongJae Park]

DAMON core unit tests set up nr_accesses_bp for representing realistic
damon_region, and also test the field.  nr_acceses_bp is no longer being
used for a real use case.  Remove the setup and tests.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index cdab14396250f..d1f019ab6fc25 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -118,7 +118,6 @@ static void damon_test_aggregate(struct kunit *test)
 				kunit_skip(test, "region alloc fail");
 			}
 			r->nr_accesses = accesses[it][ir];
-			r->nr_accesses_bp = accesses[it][ir] * 10000;
 			damon_add_region(r, t);
 		}
 		it++;
@@ -155,7 +154,6 @@ static void damon_test_split_at(struct kunit *test)
 		damon_free_target(t);
 		kunit_skip(test, "region alloc fail");
 	}
-	r->nr_accesses_bp = 420000;
 	r->nr_accesses = 42;
 	r->last_nr_accesses = 15;
 	r->age = 10;
@@ -168,7 +166,6 @@ static void damon_test_split_at(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, r_new->ar.start, 25ul);
 	KUNIT_EXPECT_EQ(test, r_new->ar.end, 100ul);
 
-	KUNIT_EXPECT_EQ(test, r->nr_accesses_bp, r_new->nr_accesses_bp);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, r_new->nr_accesses);
 	KUNIT_EXPECT_EQ(test, r->last_nr_accesses, r_new->last_nr_accesses);
 	KUNIT_EXPECT_EQ(test, r->age, r_new->age);
@@ -191,7 +188,6 @@ static void damon_test_merge_two(struct kunit *test)
 		kunit_skip(test, "region alloc fail");
 	}
 	r->nr_accesses = 10;
-	r->nr_accesses_bp = 100000;
 	r->age = 9;
 	damon_add_region(r, t);
 	r2 = damon_new_region(100, 300);
@@ -200,7 +196,6 @@ static void damon_test_merge_two(struct kunit *test)
 		kunit_skip(test, "second region alloc fail");
 	}
 	r2->nr_accesses = 20;
-	r2->nr_accesses_bp = 200000;
 	r2->age = 21;
 	damon_add_region(r2, t);
 
@@ -208,7 +203,6 @@ static void damon_test_merge_two(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
 	KUNIT_EXPECT_EQ(test, r->ar.end, 300ul);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 16u);
-	KUNIT_EXPECT_EQ(test, r->nr_accesses_bp, 160000u);
 	KUNIT_EXPECT_EQ(test, r->age, 17u);
 
 	i = 0;
@@ -256,7 +250,6 @@ static void damon_test_merge_regions_of(struct kunit *test)
 			kunit_skip(test, "region alloc fail");
 		}
 		r->nr_accesses = nrs[i];
-		r->nr_accesses_bp = nrs[i] * 10000;
 		damon_add_region(r, t);
 	}
 
@@ -556,7 +549,6 @@ static void damon_test_update_monitoring_result(struct kunit *test)
 		kunit_skip(test, "region alloc fail");
 
 	r->nr_accesses = 15;
-	r->nr_accesses_bp = 150000;
 	r->age = 20;
 
 	new_attrs = (struct damon_attrs){
-- 
2.47.3

[RFC PATCH v1.2 11/17] selftests/damon/drgn_dump_damon_status: do not dump nr_accesses_bp

[SeongJae Park]

drgn_dump_damon_status is dumping nr_accesses_bp field for future use
case.  nr_accesses_bp is not being used for a real purpose, though.
Hence there will be no future test for it.  Do not dump it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 tools/testing/selftests/damon/drgn_dump_damon_status.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/damon/drgn_dump_damon_status.py b/tools/testing/selftests/damon/drgn_dump_damon_status.py
index 26b207e44268d..09552e91bc782 100755
--- a/tools/testing/selftests/damon/drgn_dump_damon_status.py
+++ b/tools/testing/selftests/damon/drgn_dump_damon_status.py
@@ -59,7 +59,6 @@ def region_to_dict(region):
         ['ar', addr_range_to_dict],
         ['sampling_addr', int],
         ['nr_accesses', int],
-        ['nr_accesses_bp', int],
         ['age', int],
         ])
 
-- 
2.47.3

[RFC PATCH v1.2 12/17] mm/damon/core: remove nr_accesses_bp setups and updates

[SeongJae Park]

DAMON core sets and updates nr_accesses_bp in multiple places.  It
explains how delicate it is.  The field is no more being used for any
real purpose, and replaced by a simpler function.  Remove the setups and
updates.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 76ac58bf3dff9..c3c8e83234011 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -301,7 +301,6 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
 	region->ar.start = start;
 	region->ar.end = end;
 	region->nr_accesses = 0;
-	region->nr_accesses_bp = 0;
 	for (i = 0; i < DAMON_MAX_PROBES; i++)
 		region->probe_hits[i] = 0;
 	INIT_LIST_HEAD(&region->list);
@@ -875,20 +874,17 @@ static void damon_update_monitoring_result(struct damon_region *r,
 {
 	r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
 			r->last_nr_accesses, old_attrs, new_attrs);
-	if (!aggregating) {
+	if (!aggregating)
 		r->nr_accesses = damon_nr_accesses_for_new_attrs(
 				r->nr_accesses, old_attrs, new_attrs);
-		r->nr_accesses_bp = r->nr_accesses * 10000;
-	} else {
+	else
 		/*
 		 * if this is called in the middle of the aggregation, reset
 		 * the aggregations we made so far for this aggregation
 		 * interval.  In other words, make the status like
 		 * kdamond_reset_aggregated() is called.
 		 */
-		r->nr_accesses_bp = r->last_nr_accesses * 10000;
 		r->nr_accesses = 0;
-	}
 	r->age = damon_age_for_new_attrs(r->age, old_attrs, new_attrs);
 }
 
@@ -3103,7 +3099,6 @@ static void damon_merge_two_regions(struct damon_target *t,
 
 	l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
 			(sz_l + sz_r);
-	l->nr_accesses_bp = l->nr_accesses * 10000;
 	l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
 	l->ar.end = r->ar.end;
 	/* todo: do this for only installed probes */
@@ -3215,7 +3210,6 @@ static void damon_split_region_at(struct damon_target *t,
 
 	new->age = r->age;
 	new->last_nr_accesses = r->last_nr_accesses;
-	new->nr_accesses_bp = r->nr_accesses_bp;
 	new->nr_accesses = r->nr_accesses;
 	/* todo: do this for only installed probes */
 	memcpy(new->probe_hits, r->probe_hits, sizeof(r->probe_hits));
@@ -3784,18 +3778,6 @@ static unsigned int damon_moving_sum(unsigned int mvsum, unsigned int nomvsum,
 void damon_update_region_access_rate(struct damon_region *r, bool accessed,
 		struct damon_attrs *attrs)
 {
-	unsigned int len_window = 1;
-
-	/*
-	 * sample_interval can be zero, but cannot be larger than
-	 * aggr_interval, owing to validation of damon_set_attrs().
-	 */
-	if (attrs->sample_interval)
-		len_window = damon_max_nr_accesses(attrs);
-	r->nr_accesses_bp = damon_moving_sum(r->nr_accesses_bp,
-			r->last_nr_accesses * 10000, len_window,
-			accessed ? 10000 : 0);
-
 	if (accessed)
 		r->nr_accesses++;
 }
-- 
2.47.3

[RFC PATCH v1.2 13/17] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[SeongJae Park]

damon_region_access_rate() is not using attrs parameter.  Remove it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 3 +--
 mm/damon/core.c       | 4 +---
 mm/damon/paddr.c      | 4 ++--
 mm/damon/vaddr.c      | 6 +++---
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 55a743d99b567..a09ee8a09b5af 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1018,8 +1018,7 @@ unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
 
 int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 		unsigned int nr_ranges, unsigned long min_region_sz);
-void damon_update_region_access_rate(struct damon_region *r, bool accessed,
-		struct damon_attrs *attrs);
+void damon_update_region_access_rate(struct damon_region *r, bool accessed);
 
 struct damos_filter *damos_new_filter(enum damos_filter_type type,
 		bool matching, bool allow);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index c3c8e83234011..6b36e17660c05 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3768,15 +3768,13 @@ static unsigned int damon_moving_sum(unsigned int mvsum, unsigned int nomvsum,
  * damon_update_region_access_rate() - Update the access rate of a region.
  * @r:		The DAMON region to update for its access check result.
  * @accessed:	Whether the region has accessed during last sampling interval.
- * @attrs:	The damon_attrs of the DAMON context.
  *
  * Update the access rate of a region with the region's last sampling interval
  * access check result.
  *
  * Usually this will be called by &damon_operations->check_accesses callback.
  */
-void damon_update_region_access_rate(struct damon_region *r, bool accessed,
-		struct damon_attrs *attrs)
+void damon_update_region_access_rate(struct damon_region *r, bool accessed)
 {
 	if (accessed)
 		r->nr_accesses++;
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index d0598f5f26882..db48ad274b078 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -93,12 +93,12 @@ static void __damon_pa_check_access(struct damon_region *r,
 	/* If the region is in the last checked page, reuse the result */
 	if (ALIGN_DOWN(last_addr, last_folio_sz) ==
 				ALIGN_DOWN(sampling_addr, last_folio_sz)) {
-		damon_update_region_access_rate(r, last_accessed, attrs);
+		damon_update_region_access_rate(r, last_accessed);
 		return;
 	}
 
 	last_accessed = damon_pa_young(sampling_addr, &last_folio_sz);
-	damon_update_region_access_rate(r, last_accessed, attrs);
+	damon_update_region_access_rate(r, last_accessed);
 
 	last_addr = sampling_addr;
 }
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index d271476035641..2e12cc628fea0 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -503,19 +503,19 @@ static void __damon_va_check_access(struct mm_struct *mm,
 	static bool last_accessed;
 
 	if (!mm) {
-		damon_update_region_access_rate(r, false, attrs);
+		damon_update_region_access_rate(r, false);
 		return;
 	}
 
 	/* If the region is in the last checked page, reuse the result */
 	if (same_target && (ALIGN_DOWN(last_addr, last_folio_sz) ==
 				ALIGN_DOWN(r->sampling_addr, last_folio_sz))) {
-		damon_update_region_access_rate(r, last_accessed, attrs);
+		damon_update_region_access_rate(r, last_accessed);
 		return;
 	}
 
 	last_accessed = damon_va_young(mm, r->sampling_addr, &last_folio_sz);
-	damon_update_region_access_rate(r, last_accessed, attrs);
+	damon_update_region_access_rate(r, last_accessed);
 
 	last_addr = r->sampling_addr;
 }
-- 
2.47.3

[RFC PATCH v1.2 14/17] mm/damonn/paddr: remove attrs param from __damon_pa_check_access()

[SeongJae Park]

The function is not using the parameter.  Remove it.

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

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index db48ad274b078..a301e831a3945 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -82,7 +82,7 @@ static bool damon_pa_young(phys_addr_t paddr, unsigned long *folio_sz)
 }
 
 static void __damon_pa_check_access(struct damon_region *r,
-		struct damon_attrs *attrs, unsigned long addr_unit)
+		unsigned long addr_unit)
 {
 	static phys_addr_t last_addr;
 	static unsigned long last_folio_sz = PAGE_SIZE;
@@ -111,8 +111,7 @@ static unsigned int damon_pa_check_accesses(struct damon_ctx *ctx)
 
 	damon_for_each_target(t, ctx) {
 		damon_for_each_region(r, t) {
-			__damon_pa_check_access(
-					r, &ctx->attrs, ctx->addr_unit);
+			__damon_pa_check_access(r, ctx->addr_unit);
 			max_nr_accesses = max(r->nr_accesses, max_nr_accesses);
 		}
 	}
-- 
2.47.3

[RFC PATCH v1.2 15/17] mm/damon/vaddr: remove attrs param from __damon_va_check_access()

[SeongJae Park]

The function is not using attrs parameter.  Remove it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/vaddr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 2e12cc628fea0..ec08fc6624479 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -495,8 +495,7 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
  * r	the region to be checked
  */
 static void __damon_va_check_access(struct mm_struct *mm,
-				struct damon_region *r, bool same_target,
-				struct damon_attrs *attrs)
+				struct damon_region *r, bool same_target)
 {
 	static unsigned long last_addr;
 	static unsigned long last_folio_sz = PAGE_SIZE;
@@ -532,8 +531,7 @@ static unsigned int damon_va_check_accesses(struct damon_ctx *ctx)
 		mm = damon_get_mm(t);
 		same_target = false;
 		damon_for_each_region(r, t) {
-			__damon_va_check_access(mm, r, same_target,
-					&ctx->attrs);
+			__damon_va_check_access(mm, r, same_target);
 			max_nr_accesses = max(r->nr_accesses, max_nr_accesses);
 			same_target = true;
 		}
-- 
2.47.3

[RFC PATCH v1.2 16/17] mm/damon/core: remove damon_moving_sum() and its unit test

[SeongJae Park]

damon_moving_sum() is no longer being called for real purpose but its
unit test.  Testing a function that is not being used for real users
makes no sense.  Remove the test and the function.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c             | 40 -------------------------------------
 mm/damon/tests/core-kunit.h | 16 ---------------
 2 files changed, 56 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 6b36e17660c05..6542ebf13d4d8 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3724,46 +3724,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 	return damon_set_regions(t, &addr_range, 1, min_region_sz);
 }
 
-/*
- * damon_moving_sum() - Calculate an inferred moving sum value.
- * @mvsum:	Inferred sum of the last @len_window values.
- * @nomvsum:	Non-moving sum of the last discrete @len_window window values.
- * @len_window:	The number of last values to take care of.
- * @new_value:	New value that will be added to the pseudo moving sum.
- *
- * Moving sum (moving average * window size) is good for handling noise, but
- * the cost of keeping past values can be high for arbitrary window size.  This
- * function implements a lightweight pseudo moving sum function that doesn't
- * keep the past window values.
- *
- * It simply assumes there was no noise in the past, and get the no-noise
- * assumed past value to drop from @nomvsum and @len_window.  @nomvsum is a
- * non-moving sum of the last window.  For example, if @len_window is 10 and we
- * have 25 values, @nomvsum is the sum of the 11th to 20th values of the 25
- * values.  Hence, this function simply drops @nomvsum / @len_window from
- * given @mvsum and add @new_value.
- *
- * For example, if @len_window is 10 and @nomvsum is 50, the last 10 values for
- * the last window could be vary, e.g., 0, 10, 0, 10, 0, 10, 0, 0, 0, 20.  For
- * calculating next moving sum with a new value, we should drop 0 from 50 and
- * add the new value.  However, this function assumes it got value 5 for each
- * of the last ten times.  Based on the assumption, when the next value is
- * measured, it drops the assumed past value, 5 from the current sum, and add
- * the new value to get the updated pseduo-moving average.
- *
- * This means the value could have errors, but the errors will be disappeared
- * for every @len_window aligned calls.  For example, if @len_window is 10, the
- * pseudo moving sum with 11th value to 19th value would have an error.  But
- * the sum with 20th value will not have the error.
- *
- * Return: Pseudo-moving average after getting the @new_value.
- */
-static unsigned int damon_moving_sum(unsigned int mvsum, unsigned int nomvsum,
-		unsigned int len_window, unsigned int new_value)
-{
-	return mvsum - nomvsum / len_window + new_value;
-}
-
 /**
  * damon_update_region_access_rate() - Update the access rate of a region.
  * @r:		The DAMON region to update for its access check result.
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index d1f019ab6fc25..df16cc070eeb0 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -600,21 +600,6 @@ static void damon_test_set_attrs(struct kunit *test)
 	damon_destroy_ctx(c);
 }
 
-static void damon_test_moving_sum(struct kunit *test)
-{
-	unsigned int mvsum = 50000, nomvsum = 50000, len_window = 10;
-	unsigned int new_values[] = {10000, 0, 10000, 0, 0, 0, 10000, 0, 0, 0};
-	unsigned int expects[] = {55000, 50000, 55000, 50000, 45000, 40000,
-		45000, 40000, 35000, 30000};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(new_values); i++) {
-		mvsum = damon_moving_sum(mvsum, nomvsum, len_window,
-				new_values[i]);
-		KUNIT_EXPECT_EQ(test, mvsum, expects[i]);
-	}
-}
-
 static void damon_test_mvsum(struct kunit *test)
 {
 	unsigned long input_expects[] = {
@@ -1516,7 +1501,6 @@ static struct kunit_case damon_test_cases[] = {
 	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_moving_sum),
 	KUNIT_CASE(damon_test_mvsum),
 	KUNIT_CASE(damos_test_new_filter),
 	KUNIT_CASE(damos_test_commit_quota_goal),
-- 
2.47.3

[RFC PATCH v1.2 17/17] mm/damon: remove damon_region->nr_accesses_bp

[SeongJae Park]

No code touches damon_region->nr_accesses_bp field.  Remove it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 10 ----------
 mm/damon/core.c       |  3 +--
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index a09ee8a09b5af..fc3d467dc2e34 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -47,8 +47,6 @@ struct damon_size_range {
  * @ar:			The address range of the region.
  * @sampling_addr:	Address of the sample for the next access check.
  * @nr_accesses:	Access frequency of this region.
- * @nr_accesses_bp:	@nr_accesses in basis point (0.01%) that updated for
- *			each sampling interval.
  * @probe_hits:		Number of probe-positive region samples.
  * @list:		List head for siblings.
  * @age:		Age of this region.
@@ -61,13 +59,6 @@ struct damon_size_range {
  * not be done with direct access but with the helper function,
  * damon_update_region_access_rate().
  *
- * @nr_accesses_bp is another representation of @nr_accesses in basis point
- * (1 in 10,000) that updated for every &damon_attrs->sample_interval in a
- * manner similar to moving sum.  By the algorithm, this value becomes
- * @nr_accesses * 10000 for every &struct damon_attrs->aggr_interval.  This can
- * be used when the aggregation interval is too huge and therefore cannot wait
- * for it before getting the access monitoring results.
- *
  * @age is initially zero, increased for each aggregation interval, and reset
  * to zero again if the access frequency is significantly changed.  If two
  * regions are merged into a new region, both @nr_accesses and @age of the new
@@ -77,7 +68,6 @@ struct damon_region {
 	struct damon_addr_range ar;
 	unsigned long sampling_addr;
 	unsigned int nr_accesses;
-	unsigned int nr_accesses_bp;
 	unsigned char probe_hits[DAMON_MAX_PROBES];
 	struct list_head list;
 
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 6542ebf13d4d8..8d3b720f774f6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3584,8 +3584,7 @@ static int kdamond_fn(void *data)
 				 * aggregation, and make aggregation
 				 * information reset for all regions.  Then,
 				 * following kdamond_reset_aggregated() call
-				 * will make the region information invalid,
-				 * particularly for ->nr_accesses_bp.
+				 * will make the region information invalid.
 				 *
 				 * Reset ->next_aggregation_sis to avoid that.
 				 * It will anyway correctly updated after this
-- 
2.47.3

Re: [RFC PATCH v1.2 08/17] mm/damon/core: remove damon_verify_reset_aggregated()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Misspelling of the field name in the commit message.
--

commit 859f6640a11f9d1e3f6d935049f590ffd710d9f9
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: remove damon_verify_reset_aggregated()

This commit removes the damon_verify_reset_aggregated() validation function
from the core DAMON logic, as the field it checks is no longer utilized in 
actual scenarios.

> nr_accesse_bp is no longer being used in real use cases.  Remove its
> validation function.

[Severity: Low]
This isn't a bug, but there is a typo in the commit message here. The field 
name is spelled "nr_accesse_bp" instead of the correct "nr_accesses_bp".

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

Re: [RFC PATCH v1.2 14/17] mm/damonn/paddr: remove attrs param from __damon_pa_check_access()

[sashiko-bot]

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

Pre-existing issues:
- [High] Data race on static variables across multiple kdamond threads.
--

commit 7fa73a58c60db55b9517dcf532023a51c375ba61
Author: SeongJae Park <sj@kernel.org>

mm/damonn/paddr: remove attrs param from __damon_pa_check_access()

The function is not using the parameter.  Remove it.

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index db48ad274b078..a301e831a3945 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -82,7 +82,7 @@ static bool damon_pa_young(phys_addr_t paddr, unsigned long *folio_sz)
>  }
>  
>  static void __damon_pa_check_access(struct damon_region *r,
> -		struct damon_attrs *attrs, unsigned long addr_unit)
> +		unsigned long addr_unit)
>  {
>  	static phys_addr_t last_addr;
>  	static unsigned long last_folio_sz = PAGE_SIZE;
>  	static bool last_accessed;

[Severity: High]
This is a pre-existing issue, but since we are modifying this function,
could these static variables cause data races when multiple independent
kdamond threads run concurrently?

DAMON allows users to create and run multiple contexts concurrently, each
running in its own independent kdamond thread. When multiple threads
perform physical address access checks, they will execute
__damon_pa_check_access() concurrently.

Since last_addr, last_folio_sz, and last_accessed are function-local
static variables without locks or synchronization, thread A could execute
damon_pa_young() and store its result in last_accessed. Concurrently,
thread B could overwrite last_accessed with its own result.

When thread A resumes to call damon_update_region_access_rate(), wouldn't
it read the overwritten value, incorrectly applying thread B's access
result to thread A's region?

Could this mix the core access metrics of different DAMON contexts,
leading to flawed memory management decisions like paging out hot pages?

[ ... ]

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

Re: [RFC PATCH v1.2 01/17] mm/damon: introduce damon_nr_accesses_mvsum()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in left_window_bp calculation on 32-bit architectures
--

commit c5893684a7c356bd0f4693c2ef6895c46e83d332
Author: SeongJae Park <sj@kernel.org>

mm/damon: introduce damon_nr_accesses_mvsum()

Introduce a new DAMON core function, damon_nr_accesses_mvsum(), to return a
pseudo moving sum value of a given region's nr_accesses for the last
aggregation interval. This value acts as a relative access frequency score
that is updated only when requested.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> +unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
> +		struct damon_ctx *ctx)
> +{
> +	unsigned long sample_interval, aggr_interval;
> +	unsigned long window_len, left_window, left_window_bp;
> +
> +	sample_interval = ctx->attrs.sample_interval ? : 1;
> +	aggr_interval = ctx->attrs.aggr_interval ? : 1;
> +	window_len = aggr_interval / sample_interval;
> +	if (time_after_eq(ctx->passed_sample_intervals,
> +				ctx->next_aggregation_sis))
> +		left_window = 0;
> +	else
> +		left_window = ctx->next_aggregation_sis -
> +			ctx->passed_sample_intervals;
> +	left_window_bp = mult_frac(left_window, 10000, window_len);

[Severity: Medium]
Can this cause an integer overflow on 32-bit architectures?

Looking at the mult_frac macro, the remainder is multiplied by the numerator:
r * n_ / d_

If user-configurable parameters result in a large ratio, such as a 10
microsecond sample interval and a 5 second aggregation interval, window_len
becomes 500,000.

If left_window is near that maximum, for example 499,999, the intermediate
multiplication inside the macro would be 499,999 * 10000. This evaluates to
4,999,990,000, which exceeds the 32-bit unsigned long maximum of
4,294,967,295.

Could this wrap around and result in an incorrectly small remainder,
producing an inaccurate left_window_bp percentage? Passing 10000ULL might
enforce 64-bit math to avoid this.

> +
> +	return damon_mvsum(r->nr_accesses, r->last_nr_accesses,
> +			left_window_bp);
> +}

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

Re: [RFC PATCH v1.2 03/17] mm/damon/core: always update ->last_nr_accesses for intervals change

[sashiko-bot]

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

Pre-existing issues:
- [High] Arithmetic overflow in DAMON access scaling functions when interval ratio is large.
- [High] The patch attempts to fix a 'wrong value' returned by `damon_nr_accesses_mvsum()`, but misdiagnoses the root cause and leaves a major double-counting bug unresolved.
--

commit 6636564dd42488eb260d7102569887a00bdd3d71
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: always update ->last_nr_accesses for intervals change

This commit modifies damon_update_monitoring_result() to unconditionally
update last_nr_accesses when aggregation intervals change. It aims to
prevent incorrect calculations in damon_nr_accesses_mvsum() that could occur
if called with a stale last_nr_accesses value alongside a fresh aggregation
timestamp.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index addd14e4e70bf..bc40a6d10f277 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -873,6 +873,8 @@ 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, old_attrs, new_attrs);

[Severity: High]
This is a pre-existing issue, but does this scaling trigger an arithmetic
overflow when the interval ratio is large?

When damon_nr_accesses_for_new_attrs() is called, it performs 32-bit
arithmetic in damon_nr_accesses_to_accesses_bp() and
damon_accesses_bp_to_nr_accesses().

Specifically:

mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
    ...
    return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
}

mm/damon/core.c:damon_accesses_bp_to_nr_accesses() {
    ...
    return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
}

If the ratio of aggr_interval to sample_interval exceeds 429,496 (for
example, a 1s aggregation and 1us sample), the intermediate multiplications
like r * 10000 inside mult_frac() are computed as 32-bit unsigned int,
which overflows the 32-bit maximum.

Can this silently corrupt the region access counts upon attribute updates,
leading to completely incorrect monitoring results?


[Severity: High]
This isn't a bug introduced by this patch, but does scaling
last_nr_accesses fully resolve the incorrect values returned by
damon_nr_accesses_mvsum(), or does it leave a double-counting flaw intact?

The commit message notes that damon_nr_accesses_mvsum() can return the
wrong value. However, the root cause appears to be a state desynchronization
rather than just a stale last_nr_accesses.

When damon_set_attrs() is called, it advances ctx->next_aggregation_sis
into the future:

mm/damon/core.c:damon_set_attrs() {
    ...
    ctx->next_aggregation_sis = ctx->passed_sample_intervals +
                                attrs->aggr_interval / sample_interval;
}

If a subsequent call to damon_nr_accesses_mvsum() evaluates this future
timestamp, it assumes it is at the start of a new interval, meaning
left_window_bp evaluates to 10000.

mm/damon/core.c:damon_nr_accesses_mvsum() {
    ...
    return damon_mvsum(r->nr_accesses, r->last_nr_accesses, left_window_bp);
}

Because kdamond_reset_aggregated() has not yet executed, r->nr_accesses
still holds the fully accumulated count of the just-completed interval.
This means damon_mvsum() adds two full intervals' worth of access counts
together, effectively calculating:
r->nr_accesses + (r->last_nr_accesses * 10000 / 10000).

Does unconditionally scaling last_nr_accesses merely adjust the second term
of this sum while preserving the underlying double-counting logic flaw?

>  	if (!aggregating) {
>  		r->nr_accesses = damon_nr_accesses_for_new_attrs(
>  				r->nr_accesses, old_attrs, new_attrs);
> @@ -884,8 +886,6 @@ static void damon_update_monitoring_result(struct damon_region *r,
>  		 * interval.  In other words, make the status like
>  		 * kdamond_reset_aggregated() is called.
>  		 */
> -		r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
> -				r->last_nr_accesses, old_attrs, new_attrs);
>  		r->nr_accesses_bp = r->last_nr_accesses * 10000;
>  		r->nr_accesses = 0;
>  	}

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

Re: [RFC PATCH v1.2 13/17] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[sashiko-bot]

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

Pre-existing issues:
- [High] Unprotected static local variables in access check functions cause cross-thread data races and cross-context access measurement pollution.
--

commit cc8a115fada6e1595d5b8a6c2e994e38ac982a9c
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: remove attrs param from damon_update_region_access_rate()

This commit removes the unused attrs parameter from
damon_update_region_access_rate() and updates callers in the core,
physical address, and virtual address monitoring code to match the new
signature.

[ ... ]
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index d0598f5f26882..db48ad274b078 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -93,12 +93,12 @@ static void __damon_pa_check_access(struct damon_region *r,
>  	/* If the region is in the last checked page, reuse the result */
>  	if (ALIGN_DOWN(last_addr, last_folio_sz) ==
>  				ALIGN_DOWN(sampling_addr, last_folio_sz)) {
> -		damon_update_region_access_rate(r, last_accessed, attrs);
> +		damon_update_region_access_rate(r, last_accessed);

[Severity: High]
This is a pre-existing issue, but does relying on last_accessed here cause
data races when multiple kdamond threads run concurrently?

In __damon_pa_check_access(), last_addr, last_folio_sz, and last_accessed
are defined as static variables. Because they are shared globally across all
concurrent damon contexts without synchronization, multiple kdamond threads
will read and write them simultaneously.

Could this allow one thread to incorrectly reuse a last_accessed result
generated by a completely different thread monitoring a different physical
region, corrupting the measurement accuracy?

>  		return;
>  	}

[ ... ]
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index d271476035641..2e12cc628fea0 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -503,19 +503,19 @@ static void __damon_va_check_access(struct mm_struct *mm,
>  	static bool last_accessed;

[Severity: High]
This isn't a bug introduced by this patch, but this static variable shares the
same cross-thread data race issue as the physical address check.

If multiple contexts are monitoring different processes, they will write to
these variables simultaneously.

Does this data race allow one kdamond thread to accidentally reuse the
last_accessed result from another thread, breaking context isolation?

>  
>  	if (!mm) {
> -		damon_update_region_access_rate(r, false, attrs);
> +		damon_update_region_access_rate(r, false);
>  		return;
>  	}

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

Re: [RFC PATCH v1.2 09/17] mm/damon/core: remove damon_verify_merge_regions_of()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing size-weighted averaging for `last_nr_accesses` upon region merge causes erroneous age resets and corrupted metrics.
--

commit 6d81112ae07520e7dd1deaeaf23912d17d4d8a57
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: remove damon_verify_merge_regions_of()

This commit removes the damon_verify_merge_regions_of() validation check.
The nr_accesses_bp field is no longer used for a real purpose, making this
validation unnecessary.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ee19238d62f5f..94d8ec2965451 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3114,20 +3114,6 @@ static void damon_merge_two_regions(struct damon_target *t,
>  	damon_destroy_region(r, t);
>  }

[Severity: High]
This is a pre-existing issue, but does damon_merge_two_regions() need to
calculate a size-weighted average for last_nr_accesses alongside nr_accesses
and age?

Currently, when two adjacent regions are combined, last_nr_accesses appears
to be omitted from the averaging logic, causing the newly merged region to
strictly inherit the left region's history.

Because kdamond_merge_regions() iterates its merge loop until the region
limit is satisfied, a region merged in the first iteration will be evaluated
again in the second iteration:

mm/damon/core.c:damon_merge_regions_of() {
        ...
        if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
                r->age = 0;
        ...
}

Since r->nr_accesses was size-averaged but r->last_nr_accesses was not,
this comparison evaluates against mismatched data. Could this frequently
cause it to erroneously reset r->age to 0?

If so, this seems like it might break DAMON schemes that rely on region age
(such as pageout schemes meant to reclaim cold memory) by continuously
resetting the region's age. It might also skew policy frequency scores in
damon_nr_accesses_mvsum() if damon_set_attrs() is called from a user context
before the aggregation interval resets.

> -#ifdef CONFIG_DAMON_DEBUG_SANITY
> -static void damon_verify_merge_regions_of(struct damon_region *r)
> -{
> -	WARN_ONCE(r->nr_accesses != r->nr_accesses_bp / 10000,
> -			"nr_accesses (%u) != nr_accesses_bp (%u)\n",
> -			r->nr_accesses, r->nr_accesses_bp);
> -}
> -#else
> -static void damon_verify_merge_regions_of(struct damon_region *r)
> -{
> -}
> -#endif
> -
> -
>  /*
>   * Merge adjacent regions having similar access frequencies

[ ... ]

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

Re: [RFC PATCH v1.2 12/17] mm/damon/core: remove nr_accesses_bp setups and updates

[sashiko-bot]

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

New issues:
- [Medium] 32-bit integer overflow in mult_frac() calculation for left_window_bp.

Pre-existing issues:
- [High] Region merge operations do not size-weight average the last_nr_accesses field, leading to inaccurate pseudo moving sum calculations.
--

commit f4a10131988cee7930aeb4f2c1a3598bf65f3936
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: remove nr_accesses_bp setups and updates
    
This patch removes the nr_accesses_bp field from struct damon_region and its
associated setups and updates. It states that the field is no longer used for
any real purpose and has been replaced by a simpler function.

The commit message mentions the field is replaced by a simpler function,
which is damon_nr_accesses_mvsum().

Looking at damon_nr_accesses_mvsum(), the fraction of the remaining
aggregation window is calculated like this:

mm/damon/core.c:damon_nr_accesses_mvsum() {
    ...
	left_window_bp = mult_frac(left_window, 10000, window_len);
    ...
}

[Severity: Medium]
Could this result in a 32-bit integer overflow?

The mult_frac(x, n, d) macro safely computes (x / d) * n + (x % d) * n / d.
Since left_window and window_len are of type unsigned long, on 32-bit
architectures the remainder 'r' and the multiplication 'r * 10000' are
evaluated as 32-bit unsigned integers.

Because userspace can configure an arbitrarily large aggr_interval relative
to sample_interval, window_len can exceed 429,496. If left_window %
window_len is large enough, r * 10000 will overflow 2^32 - 1.

Can this overflow result in an incorrect left_window_bp value, corrupting
the pseudo moving sum access frequency score and misleading DAMON's memory
management decisions on 32-bit systems?

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 94d8ec2965451..be41601163e29 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -3103,7 +3099,6 @@ static void damon_merge_two_regions(struct damon_target *t,
>  
>  	l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
>  			(sz_l + sz_r);
> -	l->nr_accesses_bp = l->nr_accesses * 10000;

[Severity: High]
This is a pre-existing issue, but its impact is amplified to a regression by
this patchset. When damon_merge_two_regions() merges two adjacent regions,
it correctly calculates the size-weighted average for nr_accesses and age,
but fails to merge last_nr_accesses.

Prior to this series, the access score was represented by nr_accesses_bp,
which was correctly size-weighted during a merge. By replacing
nr_accesses_bp with damon_nr_accesses_mvsum(), DAMON now actively relies on
last_nr_accesses to compute the moving sum score.

Because last_nr_accesses is not size-weighted during a merge, merging a
large hot region with a small cold region will entirely wipe out the hot
region's access history.

Could this omission cause inaccurate moving sum scores, leading to incorrect
and suboptimal DAMOS targeting decisions?

>  	l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
>  	l->ar.end = r->ar.end;
>  	/* todo: do this for only installed probes */

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

Re: [RFC PATCH v1.2 01/17] mm/damon: introduce damon_nr_accesses_mvsum()

[SeongJae Park]

On Sun, 21 Jun 2026 16:10:38 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Integer overflow in left_window_bp calculation on 32-bit architectures
> --
> 
> commit c5893684a7c356bd0f4693c2ef6895c46e83d332
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon: introduce damon_nr_accesses_mvsum()
> 
> Introduce a new DAMON core function, damon_nr_accesses_mvsum(), to return a
> pseudo moving sum value of a given region's nr_accesses for the last
> aggregation interval. This value acts as a relative access frequency score
> that is updated only when requested.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > +unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
> > +		struct damon_ctx *ctx)
> > +{
> > +	unsigned long sample_interval, aggr_interval;
> > +	unsigned long window_len, left_window, left_window_bp;
> > +
> > +	sample_interval = ctx->attrs.sample_interval ? : 1;
> > +	aggr_interval = ctx->attrs.aggr_interval ? : 1;
> > +	window_len = aggr_interval / sample_interval;
> > +	if (time_after_eq(ctx->passed_sample_intervals,
> > +				ctx->next_aggregation_sis))
> > +		left_window = 0;
> > +	else
> > +		left_window = ctx->next_aggregation_sis -
> > +			ctx->passed_sample_intervals;
> > +	left_window_bp = mult_frac(left_window, 10000, window_len);
> 
> [Severity: Medium]
> Can this cause an integer overflow on 32-bit architectures?
> 
> Looking at the mult_frac macro, the remainder is multiplied by the numerator:
> r * n_ / d_
> 
> If user-configurable parameters result in a large ratio, such as a 10
> microsecond sample interval and a 5 second aggregation interval, window_len
> becomes 500,000.
> 
> If left_window is near that maximum, for example 499,999, the intermediate
> multiplication inside the macro would be 499,999 * 10000. This evaluates to
> 4,999,990,000, which exceeds the 32-bit unsigned long maximum of
> 4,294,967,295.
> 
> Could this wrap around and result in an incorrectly small remainder,
> producing an inaccurate left_window_bp percentage? Passing 10000ULL might
> enforce 64-bit math to avoid this.

It happens only in weird setup, so we are ignoring this issue for now.  We can
revisit if a user claims this as an issue.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 03/17] mm/damon/core: always update ->last_nr_accesses for intervals change

[SeongJae Park]

On Sun, 21 Jun 2026 16:11:04 +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] Arithmetic overflow in DAMON access scaling functions when interval ratio is large.
> - [High] The patch attempts to fix a 'wrong value' returned by `damon_nr_accesses_mvsum()`, but misdiagnoses the root cause and leaves a major double-counting bug unresolved.
> --
> 
> commit 6636564dd42488eb260d7102569887a00bdd3d71
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: always update ->last_nr_accesses for intervals change
> 
> This commit modifies damon_update_monitoring_result() to unconditionally
> update last_nr_accesses when aggregation intervals change. It aims to
> prevent incorrect calculations in damon_nr_accesses_mvsum() that could occur
> if called with a stale last_nr_accesses value alongside a fresh aggregation
> timestamp.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index addd14e4e70bf..bc40a6d10f277 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -873,6 +873,8 @@ 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, old_attrs, new_attrs);
> 
> [Severity: High]
> This is a pre-existing issue, but does this scaling trigger an arithmetic
> overflow when the interval ratio is large?
> 
> When damon_nr_accesses_for_new_attrs() is called, it performs 32-bit
> arithmetic in damon_nr_accesses_to_accesses_bp() and
> damon_accesses_bp_to_nr_accesses().
> 
> Specifically:
> 
> mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
>     ...
>     return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
> }
> 
> mm/damon/core.c:damon_accesses_bp_to_nr_accesses() {
>     ...
>     return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
> }
> 
> If the ratio of aggr_interval to sample_interval exceeds 429,496 (for
> example, a 1s aggregation and 1us sample), the intermediate multiplications
> like r * 10000 inside mult_frac() are computed as 32-bit unsigned int,
> which overflows the 32-bit maximum.
> 
> Can this silently corrupt the region access counts upon attribute updates,
> leading to completely incorrect monitoring results?

Known low priority issue.  Not a blocker of this patch.

> 
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does scaling
> last_nr_accesses fully resolve the incorrect values returned by
> damon_nr_accesses_mvsum(), or does it leave a double-counting flaw intact?
> 
> The commit message notes that damon_nr_accesses_mvsum() can return the
> wrong value. However, the root cause appears to be a state desynchronization
> rather than just a stale last_nr_accesses.
> 
> When damon_set_attrs() is called, it advances ctx->next_aggregation_sis
> into the future:
> 
> mm/damon/core.c:damon_set_attrs() {
>     ...
>     ctx->next_aggregation_sis = ctx->passed_sample_intervals +
>                                 attrs->aggr_interval / sample_interval;
> }
> 
> If a subsequent call to damon_nr_accesses_mvsum() evaluates this future
> timestamp, it assumes it is at the start of a new interval, meaning
> left_window_bp evaluates to 10000.
> 
> mm/damon/core.c:damon_nr_accesses_mvsum() {
>     ...
>     return damon_mvsum(r->nr_accesses, r->last_nr_accesses, left_window_bp);
> }
> 
> Because kdamond_reset_aggregated() has not yet executed, r->nr_accesses
> still holds the fully accumulated count of the just-completed interval.
> This means damon_mvsum() adds two full intervals' worth of access counts
> together, effectively calculating:
> r->nr_accesses + (r->last_nr_accesses * 10000 / 10000).
> 
> Does unconditionally scaling last_nr_accesses merely adjust the second term
> of this sum while preserving the underlying double-counting logic flaw?

Good point.  In the long term, damon_update_monitoring_result() should modify
nr_accesses.  We cannot do that at this moment, since we have to support both
nr_accesses_bp and damon_nr_accesses_mvsum().   I will add a temporal
preparatory hack to damonn_nr_accesses_mvsum() for this issue, like below.

'''
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -268,6 +268,8 @@ unsigned int damon_nr_accesses_mvsum(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_nr_accesses;
        return damon_mvsum(r->nr_accesses, r->last_nr_accesses,
                        left_window_bp);
 }
'''

At the end of the series, we will updte damon_update_monitoring_result() to
reset nr_accesses always,and remove this temporal hack.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 08/17] mm/damon/core: remove damon_verify_reset_aggregated()

[SeongJae Park]

On Sun, 21 Jun 2026 16:06:29 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Misspelling of the field name in the commit message.
> --
> 
> commit 859f6640a11f9d1e3f6d935049f590ffd710d9f9
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: remove damon_verify_reset_aggregated()
> 
> This commit removes the damon_verify_reset_aggregated() validation function
> from the core DAMON logic, as the field it checks is no longer utilized in 
> actual scenarios.
> 
> > nr_accesse_bp is no longer being used in real use cases.  Remove its
> > validation function.
> 
> [Severity: Low]
> This isn't a bug, but there is a typo in the commit message here. The field 
> name is spelled "nr_accesse_bp" instead of the correct "nr_accesses_bp".

Good finding, I will fix this in the next revision.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 09/17] mm/damon/core: remove damon_verify_merge_regions_of()

[SeongJae Park]

On Sun, 21 Jun 2026 18:09:54 +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] Missing size-weighted averaging for `last_nr_accesses` upon region merge causes erroneous age resets and corrupted metrics.
> --
> 
> commit 6d81112ae07520e7dd1deaeaf23912d17d4d8a57
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: remove damon_verify_merge_regions_of()
> 
> This commit removes the damon_verify_merge_regions_of() validation check.
> The nr_accesses_bp field is no longer used for a real purpose, making this
> validation unnecessary.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ee19238d62f5f..94d8ec2965451 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3114,20 +3114,6 @@ static void damon_merge_two_regions(struct damon_target *t,
> >  	damon_destroy_region(r, t);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does damon_merge_two_regions() need to
> calculate a size-weighted average for last_nr_accesses alongside nr_accesses
> and age?
> 
> Currently, when two adjacent regions are combined, last_nr_accesses appears
> to be omitted from the averaging logic, causing the newly merged region to
> strictly inherit the left region's history.
> 
> Because kdamond_merge_regions() iterates its merge loop until the region
> limit is satisfied, a region merged in the first iteration will be evaluated
> again in the second iteration:
> 
> mm/damon/core.c:damon_merge_regions_of() {
>         ...
>         if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
>                 r->age = 0;
>         ...
> }
> 
> Since r->nr_accesses was size-averaged but r->last_nr_accesses was not,
> this comparison evaluates against mismatched data. Could this frequently
> cause it to erroneously reset r->age to 0?
> 
> If so, this seems like it might break DAMON schemes that rely on region age
> (such as pageout schemes meant to reclaim cold memory) by continuously
> resetting the region's age. It might also skew policy frequency scores in
> damon_nr_accesses_mvsum() if damon_set_attrs() is called from a user context
> before the aggregation interval resets.

Good finding, but not necessarily a blocker of this series.  I will separately
work on this.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 12/17] mm/damon/core: remove nr_accesses_bp setups and updates

[SeongJae Park]

On Sun, 21 Jun 2026 18:10:05 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] 32-bit integer overflow in mult_frac() calculation for left_window_bp.

Known low priority issue.  Not a blocker of this patch.

> 
> Pre-existing issues:
> - [High] Region merge operations do not size-weight average the last_nr_accesses field, leading to inaccurate pseudo moving sum calculations.

I will separately work on this.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 13/17] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[SeongJae Park]

On Sun, 21 Jun 2026 16:14:56 +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] Unprotected static local variables in access check functions cause cross-thread data races and cross-context access measurement pollution.
> --
> 
> commit cc8a115fada6e1595d5b8a6c2e994e38ac982a9c
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: remove attrs param from damon_update_region_access_rate()
> 
> This commit removes the unused attrs parameter from
> damon_update_region_access_rate() and updates callers in the core,
> physical address, and virtual address monitoring code to match the new
> signature.
> 
> [ ... ]
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index d0598f5f26882..db48ad274b078 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -93,12 +93,12 @@ static void __damon_pa_check_access(struct damon_region *r,
> >  	/* If the region is in the last checked page, reuse the result */
> >  	if (ALIGN_DOWN(last_addr, last_folio_sz) ==
> >  				ALIGN_DOWN(sampling_addr, last_folio_sz)) {
> > -		damon_update_region_access_rate(r, last_accessed, attrs);
> > +		damon_update_region_access_rate(r, last_accessed);
> 
> [Severity: High]
> This is a pre-existing issue, but does relying on last_accessed here cause
> data races when multiple kdamond threads run concurrently?
> 
> In __damon_pa_check_access(), last_addr, last_folio_sz, and last_accessed
> are defined as static variables. Because they are shared globally across all
> concurrent damon contexts without synchronization, multiple kdamond threads
> will read and write them simultaneously.
> 
> Could this allow one thread to incorrectly reuse a last_accessed result
> generated by a completely different thread monitoring a different physical
> region, corrupting the measurement accuracy?

Nice finding.  Not a blocker of this patch, though.  I will separatelly work on
this.
> 
> >  		return;
> >  	}
> 
> [ ... ]
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index d271476035641..2e12cc628fea0 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -503,19 +503,19 @@ static void __damon_va_check_access(struct mm_struct *mm,
> >  	static bool last_accessed;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but this static variable shares the
> same cross-thread data race issue as the physical address check.
> 
> If multiple contexts are monitoring different processes, they will write to
> these variables simultaneously.
> 
> Does this data race allow one kdamond thread to accidentally reuse the
> last_accessed result from another thread, breaking context isolation?

Ditto.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 14/17] mm/damonn/paddr: remove attrs param from __damon_pa_check_access()

[SeongJae Park]

On Sun, 21 Jun 2026 16:07:41 +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] Data race on static variables across multiple kdamond threads.

I'll separately work on this.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.2 03/17] mm/damon/core: always update ->last_nr_accesses for intervals change

[SeongJae Park]

On Sun, 21 Jun 2026 13:23:16 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Sun, 21 Jun 2026 16:11:04 +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] Arithmetic overflow in DAMON access scaling functions when interval ratio is large.
> > - [High] The patch attempts to fix a 'wrong value' returned by `damon_nr_accesses_mvsum()`, but misdiagnoses the root cause and leaves a major double-counting bug unresolved.
> > --
> > 
> > commit 6636564dd42488eb260d7102569887a00bdd3d71
> > Author: SeongJae Park <sj@kernel.org>
> > 
> > mm/damon/core: always update ->last_nr_accesses for intervals change
> > 
> > This commit modifies damon_update_monitoring_result() to unconditionally
> > update last_nr_accesses when aggregation intervals change. It aims to
> > prevent incorrect calculations in damon_nr_accesses_mvsum() that could occur
> > if called with a stale last_nr_accesses value alongside a fresh aggregation
> > timestamp.
> > 
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index addd14e4e70bf..bc40a6d10f277 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -873,6 +873,8 @@ 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, old_attrs, new_attrs);
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does this scaling trigger an arithmetic
> > overflow when the interval ratio is large?
> > 
> > When damon_nr_accesses_for_new_attrs() is called, it performs 32-bit
> > arithmetic in damon_nr_accesses_to_accesses_bp() and
> > damon_accesses_bp_to_nr_accesses().
> > 
> > Specifically:
> > 
> > mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
> >     ...
> >     return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
> > }
> > 
> > mm/damon/core.c:damon_accesses_bp_to_nr_accesses() {
> >     ...
> >     return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
> > }
> > 
> > If the ratio of aggr_interval to sample_interval exceeds 429,496 (for
> > example, a 1s aggregation and 1us sample), the intermediate multiplications
> > like r * 10000 inside mult_frac() are computed as 32-bit unsigned int,
> > which overflows the 32-bit maximum.
> > 
> > Can this silently corrupt the region access counts upon attribute updates,
> > leading to completely incorrect monitoring results?
> 
> Known low priority issue.  Not a blocker of this patch.
> 
> > 
> > 
> > [Severity: High]
> > This isn't a bug introduced by this patch, but does scaling
> > last_nr_accesses fully resolve the incorrect values returned by
> > damon_nr_accesses_mvsum(), or does it leave a double-counting flaw intact?
> > 
> > The commit message notes that damon_nr_accesses_mvsum() can return the
> > wrong value. However, the root cause appears to be a state desynchronization
> > rather than just a stale last_nr_accesses.
> > 
> > When damon_set_attrs() is called, it advances ctx->next_aggregation_sis
> > into the future:
> > 
> > mm/damon/core.c:damon_set_attrs() {
> >     ...
> >     ctx->next_aggregation_sis = ctx->passed_sample_intervals +
> >                                 attrs->aggr_interval / sample_interval;
> > }
> > 
> > If a subsequent call to damon_nr_accesses_mvsum() evaluates this future
> > timestamp, it assumes it is at the start of a new interval, meaning
> > left_window_bp evaluates to 10000.
> > 
> > mm/damon/core.c:damon_nr_accesses_mvsum() {
> >     ...
> >     return damon_mvsum(r->nr_accesses, r->last_nr_accesses, left_window_bp);
> > }
> > 
> > Because kdamond_reset_aggregated() has not yet executed, r->nr_accesses
> > still holds the fully accumulated count of the just-completed interval.
> > This means damon_mvsum() adds two full intervals' worth of access counts
> > together, effectively calculating:
> > r->nr_accesses + (r->last_nr_accesses * 10000 / 10000).
> > 
> > Does unconditionally scaling last_nr_accesses merely adjust the second term
> > of this sum while preserving the underlying double-counting logic flaw?
> 
> Good point.  In the long term, damon_update_monitoring_result() should modify
> nr_accesses.  We cannot do that at this moment, since we have to support both
> nr_accesses_bp and damon_nr_accesses_mvsum().   I will add a temporal
> preparatory hack to damonn_nr_accesses_mvsum() for this issue, like below.
> 
> '''
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -268,6 +268,8 @@ unsigned int damon_nr_accesses_mvsum(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_nr_accesses;
>         return damon_mvsum(r->nr_accesses, r->last_nr_accesses,
>                         left_window_bp);
>  }
> '''
> 
> At the end of the series, we will updte damon_update_monitoring_result() to
> reset nr_accesses always,and remove this temporal hack.

No, I will not.

After kdamond_call() that could call damon_set_attrs(), if it was the last
iteration for the current aggregation, kdamond_fn() will further invoke
aggregation interval operations based on the before-damon_set_attrs() cached
timestamps.  Those operations include DAMOS (if apply interval is aligned with
the aggregation interval), regions split, regions reset and intervals
auto-tuning.  Some of those operations still use nr_accesses, so nr_accesses
cannot unconditionally reset in damon_update_monitoring_result().

I will make this series be merged with the nr_accesses_mvsum() change, and
cleanup/simplify the logic after this series.


Thanks,
SJ

[...]