mm/damon: provide pseudo moving sum probe_hits

PATCH Applied SJ Park 2 participants 8 messages Jul 3

Subsystems (2)

Memory management DAMON

lore

Share & tools

lore

SJ Park

author maintainer

2026-07-03 17:06 UTC #0 lore raw

cover mm/damon: provide pseudo moving sum probe_hits

Data attribute counters (probe_hits) of DAMON are managed in the classical way. The counter value is accumulated every sampling interval, gets the complete view at the end of the aggregation interval, and is reset when the next aggregation interval starts. Hence, the complete view can be retrieved only once per aggregation interval, which can be quite long. With the suggested intervals autotuning setup, it becomes 2-4 seconds in common real production systems. It can span up to 200 seconds in theory. This will restrict online monitoring use case of DAMON.

Actually DAMON is already providing online monitoring of probe_hits. DAMON sysfs interface exposes the values via schemes tried regions directory files. However, due to the above mentioned limitation, it usually shows only partially accumulated hit counters and therefore not useful.

DAMOS is not using probe_hits at the moment. In the future, using it can further strengthen DAMOS. However, a recommended setup of DAMOS is utilizing sampling/aggregation intervals auto-tuning, and having its own DAMOS apply_interval (1 second is mostly recommended). In the setup, DAMOS will nearly always show incompletely accumulated probe_hits, which will not really be useful.

Data frequency counter (nr_accesses) of DAMON solves this problem using the pseudo moving sum value. The infrastructure is not limited to nr_accesses but general sampling based counters. Maintain and provide the pseudo moving sum of probe_hits similar to nr_accesses, using the infrastructure.

Tests
=====

On an idle system, I ran DAMON with an attribute probe filter for non-anonymous page, using DAMON user-space tool, damo [1], like below.

    $ sudo ./damo start --probe_filter allow non anon

Because the system is idle, nearly all memory is not an anonymous page but a free page, so the probe_hits are expected to be nearly always full. In this setup, since the sampling interval is 5ms and the aggregation interval is 100ms, the counter value is expected to always be near 20.

On kernels not having this series, if we retrieve the probe hits in an arbitrary time that is likely not aligned to the aggregation interval, the values are usually much lower than the expectation like below. This is because the tool is showing the incompletely aggregated values.

    $ sudo ./damo report access --format append region "probe_hits: <probe hits>"
    heatmap: 00000000000000000000000000000000000000008999999711111111000000000000000000000000
    # min/max temperatures: -1,630,000,000, 0, column size: 99.800 MiB
    intervals: sample 5 ms aggr 100 ms (max access hz 200)
    0   addr 4.000 KiB    size 3.898 GiB   access 0 hz   age 16.300 s     probe_hits: 11
    1   addr 3.898 GiB    size 77.859 MiB  access 0 hz   age 1.500 s      probe_hits: 11
    2   addr 3.974 GiB    size 700.770 MiB access 0 hz   age 0 ns         probe_hits: 11
    3   addr 4.659 GiB    size 791.078 MiB access 0 hz   age 13.700 s     probe_hits: 11
    4   addr 5.431 GiB    size 1.472 GiB   access 0 hz   age 15.800 s     probe_hits: 11
    5   addr 6.903 GiB    size 915.059 MiB access 0 hz   age 15.300 s     probe_hits: 11
    memory bw estimate: 0 B per second
    total size: 7.797 GiB
    record DAMON intervals: sample 5 ms, aggr 100 ms

After applying this series, I was able to reliably show the expected results like below.

    $ sudo ./damo report access --format append region "probe_hits: <probe hits>"
    heatmap: 00000000333333330000000166666665111111139999999855555555333333333333333444444444
    intervals: sample 5 ms aggr 100 ms (max access hz 200)
    0   addr 4.000 KiB    size 790.496 MiB access 0 hz   age 1 m 33.300 s probe_hits: 20
    1   addr 790.500 MiB  size 791.160 MiB access 0 hz   age 1 m 15.400 s probe_hits: 19
    2   addr 1.545 GiB    size 792.316 MiB access 0 hz   age 1 m 32.400 s probe_hits: 19
    3   addr 2.318 GiB    size 795.465 MiB access 0 hz   age 1 m 2.600 s  probe_hits: 19
    4   addr 3.095 GiB    size 797.102 MiB access 0 hz   age 1 m 23.500 s probe_hits: 20
    5   addr 3.874 GiB    size 797.293 MiB access 0 hz   age 47.900 s     probe_hits: 20
    6   addr 4.652 GiB    size 787.516 MiB access 0 hz   age 1 m 3.800 s  probe_hits: 20
    7   addr 5.421 GiB    size 784.461 MiB access 0 hz   age 1 m 14.400 s probe_hits: 19
    8   addr 6.187 GiB    size 795.621 MiB access 0 hz   age 1 m 15.700 s probe_hits: 20
    9   addr 6.964 GiB    size 798.000 MiB access 0 hz   age 1 m 10.200 s probe_hits: 20
    10  addr 7.744 GiB    size 54.566 MiB  access 0 hz   age 1 m 9.300 s  probe_hits: 20
    memory bw estimate: 0 B per second
    total size: 7.797 GiB
    record DAMON intervals: sample 5 ms, aggr 100 ms

FYI, 'damo report access' output format has changed on v3.3.0. Above outputs can be reproduced on <3.3.0 versions of damo.

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

Patch 1 adds probe_hits counters for values that fully accumulated in the last aggregation interval. This is required for using the moving sum infrastructure. Patch 2 introduces a function for getting the
moving sum values on demand, using the infrastructure. Finally, patch
3 updates the DAMON sysfs interface to expose the moving sum values to the schemes tried regions directory.

Changelog
=========

Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/[email protected]
- Drop RFC tag.
- Rebase to latest mm-new.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/[email protected]
- Wordsmith cover letterr, specify damo version of the test.
- Rebase to the latest mm-new.

[1] https://github.com/damonitor/damo

SJ Park (3):
mm/damon: add damon_region->last_probe_hits mm/damon/core: introduce damon_probe_hits_mvsum() mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

 include/linux/damon.h    |  4 +++-
 mm/damon/core.c          | 31 +++++++++++++++++++++++++++++--
 mm/damon/sysfs-schemes.c |  3 ++-
 3 files changed, 34 insertions(+), 4 deletions(-)

base-commit: f0eadb83262e3eac82b67cb7fb5456c2e9db75f7

Signature · 1 line

2.47.3

SJ Park

author maintainer

2026-07-03 17:06 UTC #1 lore raw

PATCH 1/3 mm/damon: add damon_region->last_probe_hits

Add new damon_region filed, last_probe_hits. Maintain fully accumulated probe_hits values from the last aggregation interval in the field.

Signed-off-by SJ Park <[email protected]>

---
 include/linux/damon.h |  2 +-
 mm/damon/core.c       | 10 ++++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

include/linux/damon.h +1 -1

index 805e089ff4f2f..4f7e305b261c3 100644
@@ -68,10 +68,10 @@ struct damon_region {
 	unsigned int nr_accesses;
 	unsigned char probe_hits[DAMON_MAX_PROBES];
 	struct list_head list;
-
 	unsigned int age;
 /* private: Internal value for age calculation. */
 	unsigned int last_nr_accesses;
+	unsigned char last_probe_hits[DAMON_MAX_PROBES];
 };
 
 /**

mm/damon/core.c +8 -2

index b2fc15a3804ff..d45dc87fbbd9c 100644
@@ -302,8 +302,10 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
 	region->ar.start = start;
 	region->ar.end = end;
 	region->nr_accesses = 0;
-	for (i = 0; i < DAMON_MAX_PROBES; i++)
+	for (i = 0; i < DAMON_MAX_PROBES; i++) {
 		region->probe_hits[i] = 0;
+		region->last_probe_hits[i] = 0;
+	}
 	INIT_LIST_HEAD(&region->list);
 
 	region->age = 0;

@@ -2047,8 +2049,10 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 					damon_nr_regions(t), nr_probes);
 			r->last_nr_accesses = r->nr_accesses;
 			r->nr_accesses = 0;
-			for (i = 0; i < DAMON_MAX_PROBES; i++)
+			for (i = 0; i < DAMON_MAX_PROBES; i++) {
+				r->last_probe_hits[i] = r->probe_hits[i];
 				r->probe_hits[i] = 0;
+			}
 		}
 		ti++;
 	}

@@ -3239,6 +3243,8 @@ static void damon_split_region_at(struct damon_target *t,
 	new->nr_accesses = r->nr_accesses;
 	/* todo: do this for only installed probes */
 	memcpy(new->probe_hits, r->probe_hits, sizeof(r->probe_hits));
+	memcpy(new->last_probe_hits, r->last_probe_hits,
+			sizeof(r->last_probe_hits));
 
 	damon_insert_region(new, r, damon_next_region(r), t);
 }

Signature · 1 line

2.47.3

SJ Park

author maintainer

2026-07-03 17:06 UTC #4 lore raw

PATCH 2/3 mm/damon/core: introduce damon_probe_hits_mvsum()

In reply to SJ Park

Implement a function for getting a reasonable best effort quality pseudo moving sums of probe_hits on demands. It reuses the internal function for the pseudo moving sum for data access frequency (nr_accesses).

Signed-off-by SJ Park <[email protected]>

---
 include/linux/damon.h |  2 ++
 mm/damon/core.c       | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

include/linux/damon.h +2 -0

index 4f7e305b261c3..616bdf0954b52 100644
@@ -1003,6 +1003,8 @@ 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);
+unsigned char damon_probe_hits_mvsum(int probe_idx, 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);

mm/damon/core.c +21 -0

index d45dc87fbbd9c..871c6f5257c9e 100644
@@ -273,6 +273,27 @@ unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
 			left_window_bp);
 }
 
+unsigned char damon_probe_hits_mvsum(int probe_idx, 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->probe_hits[probe_idx],
+			r->last_probe_hits[probe_idx], left_window_bp);
+}
+
 #ifdef CONFIG_DAMON_DEBUG_SANITY
 static void damon_verify_new_region(unsigned long start, unsigned long end)
 {

Signature · 1 line

2.47.3

SJ Park

author maintainer

2026-07-03 17:06 UTC #5 lore raw

PATCH 3/3 mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

In reply to SJ Park

DAMON sysfs interface exposes damon_region->probe_hits via probe hit files. Because the counters are completed only at the end of the aggregation interval, users can show incomplete values if they requested the file content update (update_schemes_tried_regions command) in the middle of an aggregation interval. Set the value as the pseudo moving sum value of the counter, similar to that for nr_accesses.

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/sysfs-schemes.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

mm/damon/sysfs-schemes.c +2 -1

index dbf2b0515d582..32f495a96b17a 100644
@@ -110,7 +110,8 @@ static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes,
 		struct damos_sysfs_probe *sys_probe;
 		int err;
 
-		sys_probe = damos_sysfs_probe_alloc(region->probe_hits[i]);
+		sys_probe = damos_sysfs_probe_alloc(
+				damon_probe_hits_mvsum(i, region, ctx));
 		if (!sys_probe) {
 			damos_sysfs_probes_rm_dirs(probes);
 			return -ENOMEM;

Signature · 1 line

2.47.3

[email protected]

AI review

2026-07-03 17:24 UTC #2 lore raw

Re: [PATCH 1/3] mm/damon: add damon_region->last_probe_hits

In reply to SJ Park

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

New issues:
- [Medium] damos_apply_target() merges regions without validating or averaging the newly added last_probe_hits array.
- [Medium] Missing reset of probe_hits when monitoring attributes change dynamically mid-aggregation corrupts the new last_probe_hits.

Pre-existing issues:
- [High] last_nr_accesses and newly added last_probe_hits are not updated or averaged when regions are merged in damon_merge_two_regions(). --

commit 43369d5eda0b4a1301ca211bc77bce0681b92eac
Author: SJ Park <[email protected]>

mm/damon: add damon_region->last_probe_hits

This commit adds a new last_probe_hits array to the damon_region struct. It maintains fully accumulated probe_hits values from the last aggregation interval in this new field.

Quoted 5 lines

include/linux/damon.h +0 -0
index 805e089ff4f2f..4f7e305b261c3 100644

include/linux/damon.h struct damon_region { · line 75

Quoted 9 lines

include/linux/damon.h +1 -1

@@ -68,10 +68,10 @@ struct damon_region {
 	unsigned int nr_accesses;
 	unsigned char probe_hits[DAMON_MAX_PROBES];
 	struct list_head list;
-
 	unsigned int age;
 /* private: Internal value for age calculation. */
 	unsigned int last_nr_accesses;
+	unsigned char last_probe_hits[DAMON_MAX_PROBES];
 };

[Severity: High]
This is a pre-existing issue, but does damon_merge_two_regions() also need to average last_nr_accesses and this newly added last_probe_hits array?

In mm/damon/core.c:damon_merge_two_regions():

    l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
                     (sz_l + sz_r);
    l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
    l->ar.end = r->ar.end;

Because last_nr_accesses and last_probe_hits are left unmodified as the left region's values, it seems this can cause spurious age resets in later loop passes. If the left region's last_nr_accesses was 0, but the newly averaged nr_accesses is non-zero, the condition ((r->nr_accesses == 0) != (r->last_nr_accesses == 0)) evaluates to true and spuriously resets r->age to 0.

[Severity: Medium]
Should this newly added field also be validated in damos_apply_target()?

When damos_apply_target() iterates over regions, it merges adjacent ones if their age, nr_accesses, and last_nr_accesses match:

    if (prev_r->last_nr_accesses != r->last_nr_accesses)
        continue;
    if (prev_r->nr_accesses != r->nr_accesses)
        continue;

Because last_probe_hits isn't compared, regions with different probe statistics might be erroneously merged, and the right region's probe hits would be silently discarded.

[ ... ]

Quoted 4 lines

mm/damon/core.c +0 -0

index c6fc35be633b3..60be3e9d87887 100644

[ ... ]

mm/damon/core.c static void kdamond_reset_aggregated(struct damon_ctx *c) · line 2051

Quoted 12 lines

mm/damon/core.c +3 -1

@@ -2040,8 +2042,10 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 					damon_nr_regions(t), nr_probes);
 			r->last_nr_accesses = r->nr_accesses;
 			r->nr_accesses = 0;
-			for (i = 0; i < DAMON_MAX_PROBES; i++)
+			for (i = 0; i < DAMON_MAX_PROBES; i++) {
+				r->last_probe_hits[i] = r->probe_hits[i];
 				r->probe_hits[i] = 0;
+			}
 		}
 		ti++;
 	}

[Severity: Medium]
Could this copy un-reset, partial-interval probe_hits into last_probe_hits when monitoring attributes change dynamically?

When damon_set_attrs() dynamically changes intervals mid-aggregation, damon_update_monitoring_result() explicitly resets nr_accesses to drop the partial state:

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

However, probe_hits is not reset. When kdamond_reset_aggregated() fires next, will it populate last_probe_hits with mixed-interval garbage, violating the fully accumulated semantic guarantee?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

[email protected]

AI review

2026-07-03 17:26 UTC #6 lore raw

Re: [PATCH 3/3] mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

In reply to SJ Park

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing left_window_bp == 10000 guard in damon_probe_hits_mvsum causes wrapped/doubled values at…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing left_window_bp == 10000 guard in damon_probe_hits_mvsum causes wrapped/doubled values at the aggregation interval boundary. --

commit a039a69816e23f1f7d4a1680c2ab2713e7d81fe1
Author: SJ Park <[email protected]>

mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

mm/damon/sysfs-schemes.c static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes, · line 114

Quoted 11 lines

mm/damon/sysfs-schemes.c +2 -1

index dbf2b0515d582..32f495a96b17a 100644
@@ -110,7 +110,8 @@ static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes,
 		struct damos_sysfs_probe *sys_probe;
 		int err;
 
-		sys_probe = damos_sysfs_probe_alloc(region->probe_hits[i]);
+		sys_probe = damos_sysfs_probe_alloc(
+				damon_probe_hits_mvsum(i, region, ctx));

[Severity: Medium]
Could this result in wrapped or doubled values at the aggregation interval boundary?

Looking at damon_probe_hits_mvsum() in mm/damon/core.c, it seems to calculate the moving sum without guarding against the boundary race condition, unlike damon_nr_accesses_mvsum():

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

    return damon_mvsum(r->probe_hits[probe_idx],
            r->last_probe_hits[probe_idx], left_window_bp);
}

If damon_set_attrs() updates ctx->next_aggregation_sis (e.g., via a sysfs commit command) just before kdamond_apply_schemes() executes in the same loop iteration, left_window_bp could evaluate to 10000.

If a user concurrently queues an update_schemes_tried_regions walk exactly at this moment, the walk callback invokes damon_probe_hits_mvsum(). Because the guard if (left_window_bp == 10000) is missing, the function evaluates to probe_hits + last_probe_hits.

Since probe_hits retains its un-cleared values from before the attribute update (as damon_update_monitoring_result() does not clear it), does this effectively double the count and cause the 8-bit unsigned char to silently wrap around?

While this requires a tight race between sysfs writes (commit and update_schemes_tried_regions), it appears structurally possible.

Quoted 3 lines

mm/damon/sysfs-schemes.c +0 -0

 		if (!sys_probe) {
 			damos_sysfs_probes_rm_dirs(probes);
 			return -ENOMEM;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

SJ Park

author maintainer

2026-07-03 17:38 UTC #3 lore raw

Re: [PATCH 1/3] mm/damon: add damon_region->last_probe_hits

In reply to [email protected]

Quoted 1 line · On Fri, 03 Jul 2026 17:24:47 +0000 [email protected] wrote:

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

No blocker for this patch. Read below for more details.

Quoted 43 lines

New issues:
- [Medium] damos_apply_target() merges regions without validating or averaging the newly added last_probe_hits array.
- [Medium] Missing reset of probe_hits when monitoring attributes change dynamically mid-aggregation corrupts the new last_probe_hits.
Pre-existing issues:
- [High] last_nr_accesses and newly added last_probe_hits are not updated or averaged when regions are merged in damon_merge_two_regions(). --
commit 43369d5eda0b4a1301ca211bc77bce0681b92eac
Author: SJ Park <[email protected]>
mm/damon: add damon_region->last_probe_hits
This commit adds a new last_probe_hits array to the damon_region struct. It maintains fully accumulated probe_hits values from the last aggregation interval in this new field.
Quoted 5 lines
include/linux/damon.h +0 -0
index 805e089ff4f2f..4f7e305b261c3 100644
include/linux/damon.h struct damon_region { · line 75
Quoted 9 lines
include/linux/damon.h +1 -1
@@ -68,10 +68,10 @@ struct damon_region {
 	unsigned int nr_accesses;
 	unsigned char probe_hits[DAMON_MAX_PROBES];
 	struct list_head list;
-
 	unsigned int age;
 /* private: Internal value for age calculation. */
 	unsigned int last_nr_accesses;
+	unsigned char last_probe_hits[DAMON_MAX_PROBES];
 };
[Severity: High]
This is a pre-existing issue, but does damon_merge_two_regions() also need to average last_nr_accesses and this newly added last_probe_hits array?
In mm/damon/core.c:damon_merge_two_regions():
    l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
                     (sz_l + sz_r);
    l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
    l->ar.end = r->ar.end;

Quoted 6 lines

Because last_nr_accesses and last_probe_hits are left unmodified as the left region's values, it seems this can cause spurious age resets in later loop passes. If the left region's last_nr_accesses was 0, but the newly averaged nr_accesses is non-zero, the condition ((r->nr_accesses == 0) != (r->last_nr_accesses == 0)) evaluates to true and spuriously resets r->age to 0.

Good finding. Too mild issue to Cc stable@, though. And not a blocker of this patch. I will separately work on this.

Quoted 7 lines

[Severity: Medium]
Should this newly added field also be validated in damos_apply_target()?
When damos_apply_target() iterates over regions, it merges adjacent ones if their age, nr_accesses, and last_nr_accesses match:

Quoted 8 lines

    if (prev_r->last_nr_accesses != r->last_nr_accesses)
        continue;
    if (prev_r->nr_accesses != r->nr_accesses)
        continue;
Because last_probe_hits isn't compared, regions with different probe statistics might be erroneously merged, and the right region's probe hits would be silently discarded.

DAMOS works only for nr_accesses. At the moment, probe_hits are only additional information. So this is not an issue.

Quoted 35 lines

[ ... ]
Quoted 4 lines
mm/damon/core.c +0 -0
index c6fc35be633b3..60be3e9d87887 100644
[ ... ]
mm/damon/core.c static void kdamond_reset_aggregated(struct damon_ctx *c) · line 2051
Quoted 12 lines
mm/damon/core.c +3 -1
@@ -2040,8 +2042,10 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 					damon_nr_regions(t), nr_probes);
 			r->last_nr_accesses = r->nr_accesses;
 			r->nr_accesses = 0;
-			for (i = 0; i < DAMON_MAX_PROBES; i++)
+			for (i = 0; i < DAMON_MAX_PROBES; i++) {
+				r->last_probe_hits[i] = r->probe_hits[i];
 				r->probe_hits[i] = 0;
+			}
 		}
 		ti++;
 	}
[Severity: Medium]
Could this copy un-reset, partial-interval probe_hits into last_probe_hits when monitoring attributes change dynamically?
When damon_set_attrs() dynamically changes intervals mid-aggregation, damon_update_monitoring_result() explicitly resets nr_accesses to drop the partial state:
    if (!aggregating)
        r->nr_accesses = damon_nr_accesses_for_new_attrs(r->nr_accesses,
                                                         old_attrs, new_attrs);
    else
        r->nr_accesses = 0;

Quoted 3 lines

However, probe_hits is not reset. When kdamond_reset_aggregated() fires next, will it populate last_probe_hits with mixed-interval garbage, violating the fully accumulated semantic guarantee?

At the moment, probe_hits are only additional information. So this is not an issue.

Quoted 3 lines

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks,
SJ

[...]

SJ Park

author maintainer

2026-07-03 17:42 UTC #7 lore raw

Re: [PATCH 3/3] mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

In reply to [email protected]

Quoted 2 lines · On Fri, 03 Jul 2026 17:26:50 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing left_window_bp == 10000 guard in damon_probe_hits_mvsum causes wrapped/doubled values at the aggregation interval boundary.

No blocker for this patch in my opinion.

Quoted 51 lines

--
commit a039a69816e23f1f7d4a1680c2ab2713e7d81fe1
Author: SJ Park <[email protected]>
mm/damon/sysfs-schemes: set probe hits as pseudo moving sums
DAMON sysfs interface exposes damon_region->probe_hits via probe hit files. Because the counters are completed only at the end of the aggregation interval, users can show incomplete values if they requested the file content update (update_schemes_tried_regions command) in the middle of an aggregation interval. Set the value as the pseudo moving sum value of the counter, similar to that for nr_accesses.
mm/damon/sysfs-schemes.c static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes, · line 114
Quoted 11 lines
mm/damon/sysfs-schemes.c +2 -1
index dbf2b0515d582..32f495a96b17a 100644
@@ -110,7 +110,8 @@ static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes,
 		struct damos_sysfs_probe *sys_probe;
 		int err;
 
-		sys_probe = damos_sysfs_probe_alloc(region->probe_hits[i]);
+		sys_probe = damos_sysfs_probe_alloc(
+				damon_probe_hits_mvsum(i, region, ctx));
[Severity: Medium]
Could this result in wrapped or doubled values at the aggregation interval boundary?
Looking at damon_probe_hits_mvsum() in mm/damon/core.c, it seems to calculate the moving sum without guarding against the boundary race condition, unlike damon_nr_accesses_mvsum():
mm/damon/core.c:damon_probe_hits_mvsum() {
    ...
    left_window_bp = mult_frac(left_window, 10000, window_len);

    return damon_mvsum(r->probe_hits[probe_idx],
            r->last_probe_hits[probe_idx], left_window_bp);
}
If damon_set_attrs() updates ctx->next_aggregation_sis (e.g., via a sysfs commit command) just before kdamond_apply_schemes() executes in the same loop iteration, left_window_bp could evaluate to 10000.
If a user concurrently queues an update_schemes_tried_regions walk exactly at this moment, the walk callback invokes damon_probe_hits_mvsum(). Because the guard if (left_window_bp == 10000) is missing, the function evaluates to probe_hits + last_probe_hits.

Quoted 6 lines

Since probe_hits retains its un-cleared values from before the attribute update (as damon_update_monitoring_result() does not clear it), does this effectively double the count and cause the 8-bit unsigned char to silently wrap around?
While this requires a tight race between sysfs writes (commit and update_schemes_tried_regions), it appears structurally possible.

Good finding, but this issue will only occasionally degrade the accuracy of probe_hits. It is too mild issue to block this patch in my opinion. So, no blocker unless someone raise a different opinion.

Quoted 7 lines

Quoted 4 lines

mm/damon/sysfs-schemes.c +0 -0

 		if (!sys_probe) {
 			damos_sysfs_probes_rm_dirs(probes);
 			return -ENOMEM;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

Thanks,
SJ