[PATCH 00/18] mm/damon: optimize out nr_accesses_bp

[PATCH 00/18] mm/damon: optimize out nr_accesses_bp

[SJ 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 and 4 update monitoring logic and
the new function to ready for safe use on the existing logic.  Patches
5-7 replace uses of nr_accesses_bp in DAMOS, tracepoints and DAMON sysfs
interface with the new function, respectively.  Patches 8-10 removes
nr_accesses_bp validation functions in DAMON core, one by one.  Patches
11 and 12 further remove tests and test helper for nr_accesses_bp,
respectively.  Patches 13 removes the setups and updates or
nr_accesses_bp field.  Patches 14-16 cleans up function parameters that
are no more being used due to the previous patch.  Patch 17 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 18 removes damon_region->nr_accesses_bp field.

Changes from RFC v1.3
- RFC v1.3: https://lore.kernel.org/20260622142139.30269-1-sj@kernel.org
- Wordsmith and fixup commit subjects.
- Drop RFC tag.
- Rebase to latest mm-new.
Changes from RFC v1.2
- RFC v1.2: https://lore.kernel.org/20260621155715.87932-1-sj@kernel.org
- Explicitly ignore nr_accesses from mvsum at the beginning of
  aggregation.
- Fix a typo in a commit message.
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 parameter changes.
- 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.

SJ Park (18):
  mm/damon/core: 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: handle unreset nr_accesses in damon_nr_accesses_mvsum()
  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/damon/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/core: remove damon_region->nr_accesses_bp

 include/linux/damon.h                         |  15 +-
 include/trace/events/damon.h                  |   8 +-
 mm/damon/core.c                               | 201 +++++++-----------
 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, 119 insertions(+), 170 deletions(-)


base-commit: d74f4c01f930b32580e172a786efbcb352c312f1
-- 
2.47.3

[PATCH 01/18] mm/damon/core: introduce damon_nr_accesses_mvsum()

[SJ 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: SJ 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 cfbbf8ba28f63..87c1ff479da82 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1011,6 +1011,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 972a19fcee3ec..c588f943080f9 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -208,6 +208,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

Re: [PATCH 01/18] mm/damon/core: introduce damon_nr_accesses_mvsum()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Calculation of `left_window_bp` and moving sums in `damon_nr_accesses_mvsum()` and `damon_mvsum()` suffers from an integer overflow on 32-bit architectures when `window_len > 429,496`.
--

commit 55e8315faa6caec232a35b71d630c4dd8cbb568f
Author: SJ Park <sj@kernel.org>

mm/damon/core: introduce damon_nr_accesses_mvsum()

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 027250e43c66f..fe4c470726d0b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -208,6 +208,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.

[ ... ]

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

[Severity: Medium]
Could this mult_frac() calculation suffer from an integer overflow on 32-bit
architectures?

If a user configures DAMON with a large aggr_interval and a small
sample_interval such that window_len exceeds 429,496, last_nr could be
equally large. Since unsigned long is 32 bits on those systems, can the
internal multiplication inside mult_frac() silently overflow and result in
incorrect moving sums?

> +}
> +
> +/**
> + * damon_nr_accesses_mvsum() - Returns 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);

[Severity: Medium]
Does this calculation also overflow on 32-bit systems?

The mult_frac() macro calculates (left_window % window_len) * 10000. If
left_window is greater than 429,496, multiplying by 10000 will exceed the
maximum 32-bit unsigned long limit (4,294,967,295), causing a truncated
left_window_bp result.

Should these calculations use 64-bit arithmetic, such as div64_ul(), to
safely support large intervals on 32-bit systems?

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

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

Re: [PATCH 01/18] mm/damon/core: introduce damon_nr_accesses_mvsum()

[SJ Park]

On Tue, 30 Jun 2026 04:19:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Calculation of `left_window_bp` and moving sums in `damon_nr_accesses_mvsum()` and `damon_mvsum()` suffers from an integer overflow on 32-bit architectures when `window_len > 429,496`.
> --
> 
> commit 55e8315faa6caec232a35b71d630c4dd8cbb568f
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: introduce damon_nr_accesses_mvsum()
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 027250e43c66f..fe4c470726d0b 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -208,6 +208,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.
> 
> [ ... ]
> 
> > +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);
> 
> [Severity: Medium]
> Could this mult_frac() calculation suffer from an integer overflow on 32-bit
> architectures?
> 
> If a user configures DAMON with a large aggr_interval and a small
> sample_interval such that window_len exceeds 429,496, last_nr could be
> equally large. Since unsigned long is 32 bits on those systems, can the
> internal multiplication inside mult_frac() silently overflow and result in
> incorrect moving sums?

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

> 
> > +}
> > +
> > +/**
> > + * damon_nr_accesses_mvsum() - Returns 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);
> 
> [Severity: Medium]
> Does this calculation also overflow on 32-bit systems?
> 
> The mult_frac() macro calculates (left_window % window_len) * 10000. If
> left_window is greater than 429,496, multiplying by 10000 will exceed the
> maximum 32-bit unsigned long limit (4,294,967,295), causing a truncated
> left_window_bp result.
> 
> Should these calculations use 64-bit arithmetic, such as div64_ul(), to
> safely support large intervals on 32-bit systems?

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


Thanks,
SJ

[...]

[PATCH 02/18] mm/damon/tests/core-kunit: test damon_mvsum()

[SJ 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: SJ 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 a00168730445d..ff13d1b1cbc94 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -682,6 +682,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;
@@ -1561,6 +1585,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

[PATCH 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change

[SJ 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: SJ 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 c588f943080f9..669b701e50a7d 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -877,6 +877,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);
@@ -888,8 +890,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

Re: [PATCH 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change

[sashiko-bot]

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

Pre-existing issues:
- [High] A 32-bit unsigned integer overflow in `mult_frac()` silently corrupts region access basis point conversions when the aggregation interval is significantly larger than the sample interval.
--

commit f786cbe1a79f2e1958843b60ee1875084aebedbc
Author: SJ Park <sj@kernel.org>

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

Each iteration of kdamond_fn() caches the next aggregation time. This patch
ensures last_nr_accesses is always updated to prevent returning wrong values
when damon_nr_accesses_mvsum() is called during an intervals change.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -871,6 +871,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 the underlying access basis point
conversion silently overflow on 32-bit values?

When this calls damon_nr_accesses_for_new_attrs(), it relies on
damon_nr_accesses_to_accesses_bp() to calculate the scaling:

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

Because mult_frac() evaluates the multiplication using the type of
nr_accesses (a 32-bit unsigned int), multiplying by 10000 will silently wrap
around if nr_accesses exceeds ~429,496. This could happen if a large ratio
between the aggregation interval and sample interval is configured via sysfs,
causing hot memory regions to be incorrectly evaluated as cold.

A similar issue appears to exist in damon_nr_accesses_mvsum() where
left_window (a 32-bit unsigned long on 32-bit architectures) is multiplied
by 10000 inside mult_frac():

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

Could these operands be safely cast to a larger type to prevent the overflow
before passing them to mult_frac()?

>  	if (!aggregating) {
>  		r->nr_accesses = damon_nr_accesses_for_new_attrs(
>  				r->nr_accesses, old_attrs, new_attrs);

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

Re: [PATCH 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change

[SJ Park]

On Tue, 30 Jun 2026 04:21:43 +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] A 32-bit unsigned integer overflow in `mult_frac()` silently corrupts region access basis point conversions when the aggregation interval is significantly larger than the sample interval.
> --
> 
> commit f786cbe1a79f2e1958843b60ee1875084aebedbc
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: always update ->last_nr_accesses for intervals change
> 
> Each iteration of kdamond_fn() caches the next aggregation time. This patch
> ensures last_nr_accesses is always updated to prevent returning wrong values
> when damon_nr_accesses_mvsum() is called during an intervals change.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -871,6 +871,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 the underlying access basis point
> conversion silently overflow on 32-bit values?
> 
> When this calls damon_nr_accesses_for_new_attrs(), it relies on
> damon_nr_accesses_to_accesses_bp() to calculate the scaling:
> 
> mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
>     return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
> }
> 
> Because mult_frac() evaluates the multiplication using the type of
> nr_accesses (a 32-bit unsigned int), multiplying by 10000 will silently wrap
> around if nr_accesses exceeds ~429,496. This could happen if a large ratio
> between the aggregation interval and sample interval is configured via sysfs,
> causing hot memory regions to be incorrectly evaluated as cold.
> 
> A similar issue appears to exist in damon_nr_accesses_mvsum() where
> left_window (a 32-bit unsigned long on 32-bit architectures) is multiplied
> by 10000 inside mult_frac():
> 
> mm/damon/core.c:damon_nr_accesses_mvsum() {
>     ...
>     left_window_bp = mult_frac(left_window, 10000, window_len);
>     ...
> }
> 
> Could these operands be safely cast to a larger type to prevent the overflow
> before passing them to mult_frac()?

Known low priority issue.  No blocker for this patch.


Thanks,
SJ

[...]

[PATCH 04/18] mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum()

[SJ Park]

damon_set_attrs() works like reverting aggregations that were made so
far for this aggregation window.  If this is the end of the aggregation,
however, kdamond_fn() will do the operations at the end of the
aggregation interval, using cached timestamps.  For such operations that
rely on damon_region->nr_accesses, damon_update_monitoring_results()
doesn't reset the nr_accesses if it is called at the end of the
aggregation window.

damon_nr_accesses_mvsum() works with fresh timestamps, though.  The
nr_accesses that are not reset in this case can make the logic to
unnecessarily count nr_accesses, resulting in returning
higher-than-expected pseudo moving sum nr_accesses.  No code is using
damon_nr_accesses_mvsum() yet, so this is not causing a real problem.
Following commits will add usage of the function, though.  For safe
usages, calculate the pseudo moving sum without nr_accesses if the
remaining window is full.

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 669b701e50a7d..70cbc382bec04 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -266,6 +266,9 @@ 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);
 }
-- 
2.47.3

Re: [PATCH 04/18] mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] damon_nr_accesses_mvsum() incorrectly returns stale data (r->last_nr_accesses) when damon_set_attrs() is called exactly at the end of an aggregation window.
--

commit ebe6cc65ba2de465cab83d7ffe913bb9f8304f66
Author: SJ Park <sj@kernel.org>

mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum()

This commit adds logic to damon_nr_accesses_mvsum() to return
last_nr_accesses when the remaining window is full. It aims to avoid
unnecessarily counting unreset nr_accesses when damon_set_attrs() is
called at the end of an aggregation window.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 94dd425ee4353..df79f4d10bdc3 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -266,6 +266,9 @@ 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;
> +

[Severity: Medium]
Will this early return cause the function to return stale data from the 
previous window instead of freshly accumulated data?

If damon_set_attrs() is called at the end of an aggregation window,
damon_update_monitoring_result() processes this with aggregating = false. 
This intentionally preserves the fully accumulated r->nr_accesses for the
newly completed window. 

After damon_set_attrs() pushes ctx->next_aggregation_sis forward, a
subsequent call to damon_nr_accesses_mvsum() will calculate left_window_bp
as 10000. 

Because of this early return, won't it discard the freshly accumulated 
r->nr_accesses and incorrectly return the older r->last_nr_accesses instead?

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

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

Re: [PATCH 04/18] mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum()

[SJ Park]

On Tue, 30 Jun 2026 04:23:36 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] damon_nr_accesses_mvsum() incorrectly returns stale data (r->last_nr_accesses) when damon_set_attrs() is called exactly at the end of an aggregation window.
> --
> 
> commit ebe6cc65ba2de465cab83d7ffe913bb9f8304f66
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum()
> 
> This commit adds logic to damon_nr_accesses_mvsum() to return
> last_nr_accesses when the remaining window is full. It aims to avoid
> unnecessarily counting unreset nr_accesses when damon_set_attrs() is
> called at the end of an aggregation window.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 94dd425ee4353..df79f4d10bdc3 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -266,6 +266,9 @@ 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;
> > +
> 
> [Severity: Medium]
> Will this early return cause the function to return stale data from the 
> previous window instead of freshly accumulated data?
> 
> If damon_set_attrs() is called at the end of an aggregation window,
> damon_update_monitoring_result() processes this with aggregating = false. 
> This intentionally preserves the fully accumulated r->nr_accesses for the
> newly completed window. 
> 
> After damon_set_attrs() pushes ctx->next_aggregation_sis forward, a
> subsequent call to damon_nr_accesses_mvsum() will calculate left_window_bp
> as 10000. 
> 
> Because of this early return, won't it discard the freshly accumulated 
> r->nr_accesses and incorrectly return the older r->last_nr_accesses instead?

Too minor issue to block this series, as I also previously replied [1].  I will
further work on improving this when I get a bandwidth.

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


Thanks,
SJ

[...]

[PATCH 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

[SJ 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: SJ 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 70cbc382bec04..612762490e866 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2132,10 +2132,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 &&
@@ -2161,7 +2162,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;
@@ -2747,7 +2748,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 */
@@ -3036,7 +3037,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

Re: [PATCH 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] 32-bit integer overflow in `damon_nr_accesses_mvsum()` when calculating `left_window_bp` via `mult_frac()`.
--

commit 73c4d41e08fa85ce70e1cc04b1dee325a553e44e
Author: SJ Park <sj@kernel.org>

mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

This commit replaces the direct read of nr_accesses_bp with a call to
damon_nr_accesses_mvsum() in __damos_valid_target(). This calculates the
access score dynamically to be more tolerant to errors.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index df79f4d10bdc3..50939c3359bf7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -2126,10 +2126,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);

[Severity: Medium]
Does wiring up damon_nr_accesses_mvsum() to the active DAMOS region filtering
path expose a 32-bit integer overflow?

Inside damon_nr_accesses_mvsum(), the calculation of left_window_bp uses
the mult_frac() macro:

left_window_bp = mult_frac(left_window, 10000, window_len);

If a user or auto-tuning configures an aggr_interval / sample_interval ratio
greater than ~429,496 (for example, an aggr_interval of 5,000,000 us and a
sample_interval of 10 us), the intermediate multiplication inside mult_frac()
can exceed the maximum 32-bit unsigned long value on 32-bit architectures:

(left_window % window_len) * 10000

This truncation would result in a severely deflated left_window_bp, leading
to incorrect moving sum access scores. 

Could this cause DAMOS schemes to evaluate targets improperly, leading to 
sub-optimal memory reclaim or migration decisions?

>  
>  	sz = damon_sz_region(r);
>  	return s->pattern.min_sz_region <= sz &&

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

Re: [PATCH 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

[SJ Park]

On Tue, 30 Jun 2026 04:25:40 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] 32-bit integer overflow in `damon_nr_accesses_mvsum()` when calculating `left_window_bp` via `mult_frac()`.
> --
> 
> commit 73c4d41e08fa85ce70e1cc04b1dee325a553e44e
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()
> 
> This commit replaces the direct read of nr_accesses_bp with a call to
> damon_nr_accesses_mvsum() in __damos_valid_target(). This calculates the
> access score dynamically to be more tolerant to errors.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index df79f4d10bdc3..50939c3359bf7 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2126,10 +2126,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);
> 
> [Severity: Medium]
> Does wiring up damon_nr_accesses_mvsum() to the active DAMOS region filtering
> path expose a 32-bit integer overflow?
> 
> Inside damon_nr_accesses_mvsum(), the calculation of left_window_bp uses
> the mult_frac() macro:
> 
> left_window_bp = mult_frac(left_window, 10000, window_len);
> 
> If a user or auto-tuning configures an aggr_interval / sample_interval ratio
> greater than ~429,496 (for example, an aggr_interval of 5,000,000 us and a
> sample_interval of 10 us), the intermediate multiplication inside mult_frac()
> can exceed the maximum 32-bit unsigned long value on 32-bit architectures:
> 
> (left_window % window_len) * 10000
> 
> This truncation would result in a severely deflated left_window_bp, leading
> to incorrect moving sum access scores. 
> 
> Could this cause DAMOS schemes to evaluate targets improperly, leading to 
> sub-optimal memory reclaim or migration decisions?

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


Thanks,
SJ

[...]

[PATCH 06/18] mm/damon/core: use damon_nr_accesses_mvsum() for damos region tracing

[SJ 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: SJ 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 612762490e866..6423e7417c24b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2446,7 +2446,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() */
@@ -2461,6 +2461,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;
 	}
 
@@ -2476,7 +2477,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

[PATCH 07/18] mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions

[SJ 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: SJ 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 41f93a1823bf1..dbf2b0515d582 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -157,7 +157,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);
 
@@ -165,7 +165,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);
@@ -3122,7 +3122,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

[PATCH 08/18] mm/damon/core: remove damon_warn_fix_nr_accesses_corruption()

[SJ 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: SJ 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 6423e7417c24b..634708475c01b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2006,19 +2006,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)
@@ -2060,7 +2047,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

[PATCH 09/18] mm/damon/core: remove damon_verify_reset_aggregated()

[SJ Park]

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

Signed-off-by: SJ 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 634708475c01b..bfef6a8556c8d 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2006,23 +2006,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).
  */
@@ -2051,7 +2034,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

[PATCH 10/18] mm/damon/core: remove damon_verify_merge_regions_of()

[SJ 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: SJ 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 bfef6a8556c8d..78c32ee45d28b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3121,20 +3121,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
  *
@@ -3148,7 +3134,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

[PATCH 11/18] mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests

[SJ 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: SJ 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 ff13d1b1cbc94..e1d22abfbbc22 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -114,7 +114,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++;
@@ -151,7 +150,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;
@@ -164,7 +162,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);
@@ -187,7 +184,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);
@@ -196,7 +192,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);
 
@@ -204,7 +199,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;
@@ -252,7 +246,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);
 	}
 
@@ -615,7 +608,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

[PATCH 12/18] selftests/damon/drgn_dump_damon_status: do not dump nr_accesses_bp

[SJ 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: SJ 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

[PATCH 13/18] mm/damon/core: remove nr_accesses_bp setups and updates

[SJ 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: SJ 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 78c32ee45d28b..dcd5e484fa442 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -302,7 +302,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);
@@ -882,20 +881,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);
 }
 
@@ -3110,7 +3106,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 */
@@ -3222,7 +3217,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));
@@ -3830,18 +3824,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

[PATCH 14/18] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[SJ Park]

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

Signed-off-by: SJ 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 87c1ff479da82..02ed47c558cc2 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1016,8 +1016,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 dcd5e484fa442..33c6fade59d37 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3814,15 +3814,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 5c2da45f988cf..8530213086691 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -91,12 +91,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 e73ec1ce016e5..2eaced0765e26 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -501,19 +501,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

Re: [PATCH 14/18] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[sashiko-bot]

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

New issues:
- [Low] Typo in the commit message referencing the wrong function name.

Pre-existing issues:
- [High] Static local variables used for caching access checks cause data races and cross-context contamination between concurrent DAMON threads.
--

commit 75de4af226707e222be94ee075dde34562b3817d
Author: SJ Park <sj@kernel.org>

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

The damon_update_region_access_rate() function does not use the attrs
parameter. This commit removes the unused parameter to clean up the code.

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

[Severity: Low]
This isn't a bug, but there's a typo in the commit message referencing
damon_region_access_rate() instead of damon_update_region_access_rate().

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 5c2da45f988cf..8530213086691 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -91,12 +91,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 using function-scoped static variables
for last_addr, last_folio_sz, and last_accessed here in 
__damon_pa_check_access() cause data races when multiple DAMON threads run 
concurrently?

Since these variables are shared globally across all DAMON physical address 
monitoring threads without synchronization, it seems an interleaved execution
could overwrite the cached access state. Could this result in cross-context
contamination where one thread erroneously reuses last_accessed for a 
completely different memory region?

>  		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 e73ec1ce016e5..2eaced0765e26 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
[ ... ]
> @@ -501,19 +501,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);

[Severity: High]
This is also a pre-existing issue, but do the static local variables in
__damon_va_check_access() present the same data race risk?

When multiple concurrent kdamond threads check virtual memory accesses, they
share these unprotected variables. If identical virtual addresses exist in
different monitored processes, could one context reuse the last_accessed result
from another process due to interleaved thread execution?

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

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

Re: [PATCH 14/18] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[SJ Park]

On Tue, 30 Jun 2026 04:19:23 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Low] Typo in the commit message referencing the wrong function name.
> 
> Pre-existing issues:
> - [High] Static local variables used for caching access checks cause data races and cross-context contamination between concurrent DAMON threads.
> --
> 
> commit 75de4af226707e222be94ee075dde34562b3817d
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove attrs param from damon_update_region_access_rate()
> 
> The damon_update_region_access_rate() function does not use the attrs
> parameter. This commit removes the unused parameter to clean up the code.
> 
> > damon_region_access_rate() is not using attrs parameter.  Remove it.
> 
> [Severity: Low]
> This isn't a bug, but there's a typo in the commit message referencing
> damon_region_access_rate() instead of damon_update_region_access_rate().

Good finding.  I will ask Andrew to fix up when he picks this, unless I end up
posting a new revision.

> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 5c2da45f988cf..8530213086691 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -91,12 +91,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 using function-scoped static variables
> for last_addr, last_folio_sz, and last_accessed here in 
> __damon_pa_check_access() cause data races when multiple DAMON threads run 
> concurrently?
> 
> Since these variables are shared globally across all DAMON physical address 
> monitoring threads without synchronization, it seems an interleaved execution
> could overwrite the cached access state. Could this result in cross-context
> contamination where one thread erroneously reuses last_accessed for a 
> completely different memory region?

The same issue was found by Sashiko before, and I'm separately working on it.
Not a blocker of this issue, anyway.

> 
> >  		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 e73ec1ce016e5..2eaced0765e26 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> [ ... ]
> > @@ -501,19 +501,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);
> 
> [Severity: High]
> This is also a pre-existing issue, but do the static local variables in
> __damon_va_check_access() present the same data race risk?
> 
> When multiple concurrent kdamond threads check virtual memory accesses, they
> share these unprotected variables. If identical virtual addresses exist in
> different monitored processes, could one context reuse the last_accessed result
> from another process due to interleaved thread execution?

Ditto.


Thanks,
SJ

[...]

Re: [PATCH 14/18] mm/damon/core: remove attrs param from damon_update_region_access_rate()

[SJ Park]

On Mon, 29 Jun 2026 21:08:07 -0700 SJ Park <sj@kernel.org> wrote:

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

The function name on the commit message here is wrong.  It is
damon_update_region_access_rate(), as you can show from the diff below.
Andrew, could you please fix this up when you pick this patch?  Let me know if
you prefer sending a new revision.

> 
> Signed-off-by: SJ 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 87c1ff479da82..02ed47c558cc2 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -1016,8 +1016,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);

Above diff shows the correct function name.


Thanks,
SJ

[...]

[PATCH 15/18] mm/damon/paddr: remove attrs param from __damon_pa_check_access()

[SJ Park]

The function is not using the parameter.  Remove it.

Signed-off-by: SJ 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 8530213086691..6172464981730 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -80,7 +80,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;
@@ -109,8 +109,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

[PATCH 16/18] mm/damon/vaddr: remove attrs param from __damon_va_check_access()

[SJ Park]

The function is not using attrs parameter.  Remove it.

Signed-off-by: SJ 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 2eaced0765e26..2058db9c01d52 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -493,8 +493,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;
@@ -530,8 +529,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

Re: [PATCH 16/18] mm/damon/vaddr: remove attrs param from __damon_va_check_access()

[sashiko-bot]

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

Pre-existing issues:
- [High] Thread-unsafe static state in access check functions leads to memory access rate corruption across processes.
- [High] Incorrect PTE advancement for large folios in DAMON operations causes skipped memory.
--

commit 126a430836c56a402d2fefdf82a9e58aed70a6d7
Author: SJ Park <sj@kernel.org>

mm/damon/vaddr: remove attrs param from __damon_va_check_access()

The function is not using attrs parameter. Remove it.

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 2eaced0765e26..2058db9c01d52 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -493,8 +493,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;
>  	static bool last_accessed;

[Severity: High]
This isn't a bug introduced by this patch, but does using static variables
here cause a cross-thread data race?

If multiple DAMON contexts (running in separate kdamond threads) monitor
virtual addresses simultaneously, a thread processing one process's virtual
address might read values just written by another thread.

Could this lead to incorrect reuse of the last_accessed result and corrupt
access rate statistics across different processes? A similar pattern appears
to exist in mm/damon/paddr.c:__damon_pa_check_access() as well.

[ ... ]

Additionally, while reviewing this file, I noticed another pre-existing
issue in damos_va_stat_pmd_entry() and damos_va_migrate_pmd_entry():

mm/damon/vaddr.c:damos_va_stat_pmd_entry() {
        ...
        nr = folio_nr_pages(folio);
        s->last_applied = folio;
        ...
}

[Severity: High]
This is a pre-existing issue, but does advancing the PTE by
folio_nr_pages() overshoot the loop bounds when the start address falls
in the middle of a large folio?

Because the address is not necessarily aligned to the start of the large
folio, advancing by the entire folio size might overshoot the end of the
folio and inadvertently skip subsequent PTEs that belong to unrelated
memory. Should the advancement step account for the subpage offset?

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

Re: [PATCH 16/18] mm/damon/vaddr: remove attrs param from __damon_va_check_access()

[SJ Park]

On Tue, 30 Jun 2026 04:22:14 +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] Thread-unsafe static state in access check functions leads to memory access rate corruption across processes.
> - [High] Incorrect PTE advancement for large folios in DAMON operations causes skipped memory.
> --
> 
> commit 126a430836c56a402d2fefdf82a9e58aed70a6d7
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/vaddr: remove attrs param from __damon_va_check_access()
> 
> The function is not using attrs parameter. Remove it.
> 
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 2eaced0765e26..2058db9c01d52 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -493,8 +493,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;
> >  	static bool last_accessed;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does using static variables
> here cause a cross-thread data race?
> 
> If multiple DAMON contexts (running in separate kdamond threads) monitor
> virtual addresses simultaneously, a thread processing one process's virtual
> address might read values just written by another thread.
> 
> Could this lead to incorrect reuse of the last_accessed result and corrupt
> access rate statistics across different processes? A similar pattern appears
> to exist in mm/damon/paddr.c:__damon_pa_check_access() as well.

I'm separately working on it.  Not a blocker for this patch.

> 
> [ ... ]
> 
> Additionally, while reviewing this file, I noticed another pre-existing
> issue in damos_va_stat_pmd_entry() and damos_va_migrate_pmd_entry():
> 
> mm/damon/vaddr.c:damos_va_stat_pmd_entry() {
>         ...
>         nr = folio_nr_pages(folio);
>         s->last_applied = folio;
>         ...
> }
> 
> [Severity: High]
> This is a pre-existing issue, but does advancing the PTE by
> folio_nr_pages() overshoot the loop bounds when the start address falls
> in the middle of a large folio?
> 
> Because the address is not necessarily aligned to the start of the large
> folio, advancing by the entire folio size might overshoot the end of the
> folio and inadvertently skip subsequent PTEs that belong to unrelated
> memory. Should the advancement step account for the subpage offset?

The user impact should be minor.  I'm separately working [1] on this.  Not a
blocker for this series.

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


Thanks,
SJ

[...]

[PATCH 17/18] mm/damon/core: remove damon_moving_sum() and its unit test

[SJ 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: SJ 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 33c6fade59d37..83753a53e7b98 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3770,46 +3770,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 e1d22abfbbc22..aef7e0553cba4 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -659,21 +659,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[] = {
@@ -1576,7 +1561,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

[PATCH 18/18] mm/damon/core: remove damon_region->nr_accesses_bp

[SJ Park]

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

Signed-off-by: SJ 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 02ed47c558cc2..805e089ff4f2f 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -45,8 +45,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.
@@ -59,13 +57,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
@@ -75,7 +66,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 83753a53e7b98..59a91a0ab6d45 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3630,8 +3630,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: [PATCH 00/18] mm/damon: optimize out nr_accesses_bp

[SJ Park]

On Mon, 29 Jun 2026 21:07:53 -0700 SJ Park <sj@kernel.org> wrote:

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

Sashiko found no blocker for this series.  All Sashiko findings and my
responses can be found from the complete thread [1] at lore.kernel.org.

It found a typo in the commit message of patch 14, though.  I added the detail
about the typo as a reply [2] to the patch.  Andrew, could you please fix the
commit message when you pick it up?  If you prefer sending a new revision with
the fixup, pleae feel free to let me know.

[1] https://lore.kernel.org/20260630040812.149729-1-sj@kernel.org
[2] https://lore.kernel.org/20260630044859.152347-1-sj@kernel.org/

Thanks,
SJ

[...]