* [RFC PATCH v1.1 01/16] mm/damon/core: introduce damon_probe->weight
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
@ 2026-07-06 14:18 ` SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 02/16] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
` (15 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:18 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
Add a new field, weight to damon_probe struct. The field is used to
specify the degree of the API caller's interest to the data attribute
of the probe.
Signed-off-by: SJ Park <sj@kernel.org>
---
include/linux/damon.h | 2 ++
mm/damon/core.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 19b7e839bde0b..7d0a920fe4b83 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -760,10 +760,12 @@ struct damon_filter {
/**
* struct damon_probe - Data region attribute probe.
*
+ * @weight: Relative priority of the attribute for this probe.
* @filters: Filters for assessing if a given region is for this probe.
* @list: Siblings list.
*/
struct damon_probe {
+ unsigned int weight;
struct list_head filters;
struct list_head list;
};
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 390e00b3685ef..ca5c3f3e45fc6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -166,6 +166,7 @@ struct damon_probe *damon_new_probe(void)
p = kmalloc_obj(*p);
if (!p)
return NULL;
+ p->weight = 0;
INIT_LIST_HEAD(&p->filters);
INIT_LIST_HEAD(&p->list);
return p;
@@ -1641,6 +1642,7 @@ static int damon_commit_probes(struct damon_ctx *dst, struct damon_ctx *src)
damon_for_each_probe_safe(dst_probe, next, dst) {
src_probe = damon_nth_probe(i++, src);
if (src_probe) {
+ dst_probe->weight = src_probe->weight;
err = damon_commit_filters(dst_probe, src_probe);
if (err)
return err;
@@ -1657,6 +1659,7 @@ static int damon_commit_probes(struct damon_ctx *dst, struct damon_ctx *src)
if (!new_probe)
return -ENOMEM;
damon_add_probe(dst, new_probe);
+ new_probe->weight = src_probe->weight;
err = damon_commit_filters(new_probe, src_probe);
if (err)
return err;
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [RFC PATCH v1.1 02/16] mm/damon/core: ask apply_probes() ops callback to set sampling address
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 01/16] mm/damon/core: introduce damon_probe->weight SJ Park
@ 2026-07-06 14:18 ` SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 03/16] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
` (14 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:18 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
prepare_access_checks() DAMON ops callback sets the monitoring sampling
address per region. In future, DAMON will be able to call only
apply_probes(). In this case, applyy_probes() may need to do the
sampling address setup, to minimize unnecessary regions iteration.
Update the protocol for the request.
Signed-off-by: SJ Park <sj@kernel.org>
---
include/linux/damon.h | 5 +++--
mm/damon/core.c | 2 +-
mm/damon/paddr.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 7d0a920fe4b83..4aa606f691f28 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -647,7 +647,8 @@ enum damon_ops_id {
* It should also return max number of observed accesses that made as a result
* of its update. The value will be used for regions adjustment threshold.
* @apply_probes should apply the data attribute probes to each region and
- * accordingly update the probe hits counter of the region.
+ * accordingly update the probe hits counter of the region. It should also
+ * &set sampling_addr of each region if ``set_samples`` is true.
* @get_scheme_score should return the priority score of a region for a scheme
* as an integer in [0, &DAMOS_MAX_SCORE].
* @apply_scheme is called from @kdamond when a region for user provided
@@ -665,7 +666,7 @@ struct damon_operations {
void (*update)(struct damon_ctx *context);
void (*prepare_access_checks)(struct damon_ctx *context);
unsigned int (*check_accesses)(struct damon_ctx *context);
- void (*apply_probes)(struct damon_ctx *context);
+ void (*apply_probes)(struct damon_ctx *context, bool set_samples);
int (*get_scheme_score)(struct damon_ctx *context,
struct damon_region *r, struct damos *scheme);
unsigned long (*apply_scheme)(struct damon_ctx *context,
diff --git a/mm/damon/core.c b/mm/damon/core.c
index ca5c3f3e45fc6..7c0184cb78a50 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3675,7 +3675,7 @@ static int kdamond_fn(void *data)
if (ctx->ops.check_accesses)
max_nr_accesses = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes)
- ctx->ops.apply_probes(ctx);
+ ctx->ops.apply_probes(ctx, false);
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 6172464981730..b13bf7c6eade3 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -166,7 +166,7 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
return pass;
}
-static void damon_pa_apply_probes(struct damon_ctx *ctx)
+static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
{
struct damon_target *t;
struct damon_region *r;
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [RFC PATCH v1.1 03/16] mm/damon/paddr: set samples in apply_probes() if requested
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 01/16] mm/damon/core: introduce damon_probe->weight SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 02/16] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
@ 2026-07-06 14:18 ` SJ Park
2026-07-06 14:18 ` [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
` (13 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:18 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
apply_probe() callback implementation in DAMON_PADDR is ignoring
set_samples parameter. Respect it.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/paddr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index b13bf7c6eade3..ff4fcca949660 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -178,6 +178,10 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
phys_addr_t pa;
struct folio *folio;
+ if (set_samples)
+ r->sampling_addr = damon_rand(ctx, r->ar.start,
+ r->ar.end);
+
pa = damon_pa_phys_addr(r->sampling_addr,
ctx->addr_unit);
folio = damon_get_folio(PHYS_PFN(pa));
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (2 preceding siblings ...)
2026-07-06 14:18 ` [RFC PATCH v1.1 03/16] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
@ 2026-07-06 14:18 ` SJ Park
2026-07-06 14:39 ` sashiko-bot
2026-07-06 14:18 ` [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
` (12 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:18 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
check_accesses() DAMON ops callback returns the maximum nr_accesses of
regions. DAMON core uses it to calculate a reasonable region merge
threshold. The core will need to adjust regions for not nr_accesses but
probe hits weighted sum in future. For that, the core needs to know the
maximum weighted sum of the regions. Update the protocol for the task.
Signed-off-by: SJ Park <sj@kernel.org>
---
include/linux/damon.h | 7 +++++--
mm/damon/core.c | 2 +-
mm/damon/paddr.c | 4 +++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 4aa606f691f28..e1244a06ac7e4 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -648,7 +648,9 @@ enum damon_ops_id {
* of its update. The value will be used for regions adjustment threshold.
* @apply_probes should apply the data attribute probes to each region and
* accordingly update the probe hits counter of the region. It should also
- * &set sampling_addr of each region if ``set_samples`` is true.
+ * &set sampling_addr of each region if ``set_samples`` is true. It should
+ * also return maximum probe hits weighted sum of regions if
+ * ``return_max_wsum`` is true.
* @get_scheme_score should return the priority score of a region for a scheme
* as an integer in [0, &DAMOS_MAX_SCORE].
* @apply_scheme is called from @kdamond when a region for user provided
@@ -666,7 +668,8 @@ struct damon_operations {
void (*update)(struct damon_ctx *context);
void (*prepare_access_checks)(struct damon_ctx *context);
unsigned int (*check_accesses)(struct damon_ctx *context);
- void (*apply_probes)(struct damon_ctx *context, bool set_samples);
+ unsigned int (*apply_probes)(struct damon_ctx *context,
+ bool set_samples, bool return_max_wsum);
int (*get_scheme_score)(struct damon_ctx *context,
struct damon_region *r, struct damos *scheme);
unsigned long (*apply_scheme)(struct damon_ctx *context,
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7c0184cb78a50..e42f512d87a11 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3675,7 +3675,7 @@ static int kdamond_fn(void *data)
if (ctx->ops.check_accesses)
max_nr_accesses = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes)
- ctx->ops.apply_probes(ctx, false);
+ ctx->ops.apply_probes(ctx, false, false);
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index ff4fcca949660..3eba3564a31fc 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -166,7 +166,8 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
return pass;
}
-static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
+static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
+ bool set_samples, bool return_max_wsum)
{
struct damon_target *t;
struct damon_region *r;
@@ -194,6 +195,7 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
folio_put(folio);
}
}
+ return 0;
}
/*
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-06 14:18 ` [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
@ 2026-07-06 14:39 ` sashiko-bot
2026-07-06 23:42 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 14:39 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Advancing physical addresses using `addr += folio_size(folio)` in large folio iteration paths skips memory when `addr` is not aligned to the start of the folio.
- [High] Static local variables used for caching are shared across concurrent kernel threads without synchronization.
--
commit 1ecdc723ec9bfdc97493a942107d11622114a694
Author: SJ Park <sj@kernel.org>
mm/damon/core: ask apply_probe() to return max probe hits weighted sum
check_accesses() DAMON ops callback returns the maximum nr_accesses of
regions. DAMON core uses it to calculate a reasonable region merge
threshold. The core will need to adjust regions for not nr_accesses but
probe hits weighted sum in future. For that, the core needs to know the
maximum weighted sum of the regions. Update the protocol for the task.
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index ff4fcca949660..3eba3564a31fc 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -166,7 +166,8 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
> return pass;
> }
>
> -static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> +static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> + bool set_samples, bool return_max_wsum)
> {
> struct damon_target *t;
> struct damon_region *r;
> @@ -194,6 +195,7 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> folio_put(folio);
> }
> }
> + return 0;
> }
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes in
mm/damon/paddr.c, I noticed that __damon_pa_check_access() declares static
local variables for caching:
mm/damon/paddr.c:__damon_pa_check_access() {
static phys_addr_t last_addr;
static unsigned long last_folio_sz = PAGE_SIZE;
static bool last_accessed;
...
}
Because DAMON allows multiple contexts to be created and run concurrently,
each in its own kdamond thread, could these static variables be modified
concurrently without locking? Does this data race risk corrupting the cache
state and misapplying access results between different monitoring contexts?
[Severity: High]
This is a pre-existing issue, but I also noticed a potential overshooting bug
in multiple physical address iteration paths in mm/damon/paddr.c, such as in
damon_pa_pageout():
mm/damon/paddr.c:damon_pa_pageout() {
...
addr += folio_size(folio);
...
}
If a region start address is not aligned to the size of the folio (for
instance, starting in the middle of a 2MB THP), advancing the unaligned addr
by the full folio size will overshoot the folio boundary. Could this silently
skip valid physical memory that falls in the gap?
The same pattern appears to be used in damon_pa_de_activate(),
damon_pa_migrate(), and damon_pa_stat().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=4
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-06 14:39 ` sashiko-bot
@ 2026-07-06 23:42 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:42 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 14:39:37 +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] Advancing physical addresses using `addr += folio_size(folio)` in large folio iteration paths skips memory when `addr` is not aligned to the start of the folio.
> - [High] Static local variables used for caching are shared across concurrent kernel threads without synchronization.
> --
>
> commit 1ecdc723ec9bfdc97493a942107d11622114a694
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: ask apply_probe() to return max probe hits weighted sum
>
> check_accesses() DAMON ops callback returns the maximum nr_accesses of
> regions. DAMON core uses it to calculate a reasonable region merge
> threshold. The core will need to adjust regions for not nr_accesses but
> probe hits weighted sum in future. For that, the core needs to know the
> maximum weighted sum of the regions. Update the protocol for the task.
>
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index ff4fcca949660..3eba3564a31fc 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -166,7 +166,8 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
> > return pass;
> > }
> >
> > -static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> > +static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> > + bool set_samples, bool return_max_wsum)
> > {
> > struct damon_target *t;
> > struct damon_region *r;
> > @@ -194,6 +195,7 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> > folio_put(folio);
> > }
> > }
> > + return 0;
> > }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing the changes in
> mm/damon/paddr.c, I noticed that __damon_pa_check_access() declares static
> local variables for caching:
>
> mm/damon/paddr.c:__damon_pa_check_access() {
> static phys_addr_t last_addr;
> static unsigned long last_folio_sz = PAGE_SIZE;
> static bool last_accessed;
> ...
> }
>
> Because DAMON allows multiple contexts to be created and run concurrently,
> each in its own kdamond thread, could these static variables be modified
> concurrently without locking? Does this data race risk corrupting the cache
> state and misapplying access results between different monitoring contexts?
Correct. I'm separately working [1] on this.
>
> [Severity: High]
> This is a pre-existing issue, but I also noticed a potential overshooting bug
> in multiple physical address iteration paths in mm/damon/paddr.c, such as in
> damon_pa_pageout():
>
> mm/damon/paddr.c:damon_pa_pageout() {
> ...
> addr += folio_size(folio);
> ...
> }
>
> If a region start address is not aligned to the size of the folio (for
> instance, starting in the middle of a 2MB THP), advancing the unaligned addr
> by the full folio size will overshoot the folio boundary. Could this silently
> skip valid physical memory that falls in the gap?
>
> The same pattern appears to be used in damon_pa_de_activate(),
> damon_pa_migrate(), and damon_pa_stat().
correct. I'm separately working [2] on this.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=4
[1] https://lore.kernel.org/20260621204050.10993-1-sj@kernel.org
[2] https://lore.kernel.org/20260517234112.89245-1-sj@kernel.org
Thanks,
SJ
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum()
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (3 preceding siblings ...)
2026-07-06 14:18 ` [RFC PATCH v1.1 04/16] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
@ 2026-07-06 14:18 ` SJ Park
2026-07-06 14:41 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum SJ Park
` (11 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:18 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When damon_probe->weight is set, the weighted sum of probe hits will be
useful. It will be useful for not only the users but also DAMON
internal logics like regions merging. Implement a function for
calculating it.
Signed-off-by: SJ Park <sj@kernel.org>
---
include/linux/damon.h | 2 ++
mm/damon/core.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index e1244a06ac7e4..7ecfd72cb21a2 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1012,6 +1012,8 @@ 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);
+unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
+ 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 e42f512d87a11..d7d1e27d767eb 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -390,6 +390,30 @@ static bool damon_is_last_region(struct damon_region *r,
return list_is_last(&r->list, &t->regions_list);
}
+/**
+ * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
+ * @r: region to get the weigted sum of.
+ * @last: if the request is for last-window agggregated probe hits.
+ * @ctx: context of &r.
+ *
+ * Return: the weighted sum of probe hits of the region.
+ */
+unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
+ struct damon_ctx *ctx)
+{
+ struct damon_probe *probe;
+ unsigned int sum = 0;
+ int i = 0;
+
+ damon_for_each_probe(probe, ctx) {
+ if (last)
+ sum += r->last_probe_hits[i++] * probe->weight;
+ else
+ sum += r->probe_hits[i++] * probe->weight;
+ }
+ return sum;
+}
+
/*
* Check whether a region is intersecting an address range
*
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum()
2026-07-06 14:18 ` [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
@ 2026-07-06 14:41 ` sashiko-bot
2026-07-06 23:46 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 14:41 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The calculation of the weighted sum in `damon_probe_hits_wsum()` is vulnerable to integer overflow, resulting in a silent wrap-around.
- [Low] Misspelled words in the kernel-doc comment for `damon_probe_hits_wsum`.
Pre-existing issues:
- [High] `r->probe_hits[i]` is an 8-bit `unsigned char` but serves as a sample counter across an aggregation interval, making it highly susceptible to wrap-around.
--
commit 627b12cd883dbd832e93cc6b4f4e3373c791b6d0
Author: SJ Park <sj@kernel.org>
mm/damon/core: implement damon_probe_hits_wsum()
This commit implements damon_probe_hits_wsum() to calculate the weighted
sum of probe hits for a DAMON region. This weighted sum is intended to be
used by users as well as DAMON internal logic like region merging.
[ ... ]
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index dbd1f86a0832f..42f27849af722 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -390,6 +390,30 @@ static bool damon_is_last_region(struct damon_region *r,
> return list_is_last(&r->list, &t->regions_list);
> }
>
> +/**
> + * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
> + * @r: region to get the weigted sum of.
> + * @last: if the request is for last-window agggregated probe hits.
[Severity: Low]
This isn't a bug, but there are spelling errors in this kernel-doc block.
"weigted" should be "weighted" and "agggregated" should be "aggregated".
> + * @ctx: context of &r.
> + *
> + * Return: the weighted sum of probe hits of the region.
> + */
> +unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
> + struct damon_ctx *ctx)
> +{
> + struct damon_probe *probe;
> + unsigned int sum = 0;
> + int i = 0;
> +
> + damon_for_each_probe(probe, ctx) {
> + if (last)
> + sum += r->last_probe_hits[i++] * probe->weight;
> + else
> + sum += r->probe_hits[i++] * probe->weight;
[Severity: High]
Can the accumulation into sum silently overflow?
If a large probe weight is configured via sysfs (e.g., > 16,843,009), the
multiplication of this 32-bit unsigned int weight by r->probe_hits[i] could
wrap around when accumulated into sum.
This might corrupt the DAMON region merge and split logic that relies on this
score.
[Severity: High]
This is a pre-existing issue, but can r->probe_hits[i] wrap around during long
aggregation intervals?
The probe_hits array is defined as an 8-bit unsigned char. If the user
configures aggr_interval and sample_interval such that their ratio exceeds 255
(for example, 1s aggregation and 1ms sample), the counter will silently wrap
around to 0 without any bounds checking.
This could severely degrade the accuracy of DAMON's monitoring, and is
exacerbated here by relying on the potentially corrupted counter for the new
weighted sum logic.
> + }
> + return sum;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=5
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum()
2026-07-06 14:41 ` sashiko-bot
@ 2026-07-06 23:46 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:46 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 14:41:34 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] The calculation of the weighted sum in `damon_probe_hits_wsum()` is vulnerable to integer overflow, resulting in a silent wrap-around.
> - [Low] Misspelled words in the kernel-doc comment for `damon_probe_hits_wsum`.
>
> Pre-existing issues:
> - [High] `r->probe_hits[i]` is an 8-bit `unsigned char` but serves as a sample counter across an aggregation interval, making it highly susceptible to wrap-around.
> --
>
> commit 627b12cd883dbd832e93cc6b4f4e3373c791b6d0
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: implement damon_probe_hits_wsum()
>
> This commit implements damon_probe_hits_wsum() to calculate the weighted
> sum of probe hits for a DAMON region. This weighted sum is intended to be
> used by users as well as DAMON internal logic like region merging.
>
> [ ... ]
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index dbd1f86a0832f..42f27849af722 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -390,6 +390,30 @@ static bool damon_is_last_region(struct damon_region *r,
> > return list_is_last(&r->list, &t->regions_list);
> > }
> >
> > +/**
> > + * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
> > + * @r: region to get the weigted sum of.
> > + * @last: if the request is for last-window agggregated probe hits.
>
> [Severity: Low]
> This isn't a bug, but there are spelling errors in this kernel-doc block.
>
> "weigted" should be "weighted" and "agggregated" should be "aggregated".
Good finding, I will fix these in the next spin.
>
> > + * @ctx: context of &r.
> > + *
> > + * Return: the weighted sum of probe hits of the region.
> > + */
> > +unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
> > + struct damon_ctx *ctx)
> > +{
> > + struct damon_probe *probe;
> > + unsigned int sum = 0;
> > + int i = 0;
> > +
> > + damon_for_each_probe(probe, ctx) {
> > + if (last)
> > + sum += r->last_probe_hits[i++] * probe->weight;
> > + else
> > + sum += r->probe_hits[i++] * probe->weight;
>
> [Severity: High]
> Can the accumulation into sum silently overflow?
>
> If a large probe weight is configured via sysfs (e.g., > 16,843,009), the
> multiplication of this 32-bit unsigned int weight by r->probe_hits[i] could
> wrap around when accumulated into sum.
>
> This might corrupt the DAMON region merge and split logic that relies on this
> score.
Yes, it can. But it causes only poor monitoring results for wrong user setup.
Maybe we can improve it, but not necessarily right now. No blocker.
>
> [Severity: High]
> This is a pre-existing issue, but can r->probe_hits[i] wrap around during long
> aggregation intervals?
>
> The probe_hits array is defined as an 8-bit unsigned char. If the user
> configures aggr_interval and sample_interval such that their ratio exceeds 255
> (for example, 1s aggregation and 1ms sample), the counter will silently wrap
> around to 0 without any bounds checking.
>
> This could severely degrade the accuracy of DAMON's monitoring, and is
> exacerbated here by relying on the potentially corrupted counter for the new
> weighted sum logic.
Known low priority issue. No blocker.
>
> > + }
> > + return sum;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=5
Thanks,
SJ
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (4 preceding siblings ...)
2026-07-06 14:18 ` [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 14:44 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits SJ Park
` (10 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
apply_probes() ops implementation in DAMON_PADDR is ignoring
return_max_wsum. Respect it.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/paddr.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 3eba3564a31fc..b85f88a7a38f4 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -172,6 +172,7 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
struct damon_target *t;
struct damon_region *r;
struct damon_probe *p;
+ unsigned int max_wsum = 0;
damon_for_each_target(t, ctx) {
damon_for_each_region(r, t) {
@@ -182,7 +183,6 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
if (set_samples)
r->sampling_addr = damon_rand(ctx, r->ar.start,
r->ar.end);
-
pa = damon_pa_phys_addr(r->sampling_addr,
ctx->addr_unit);
folio = damon_get_folio(PHYS_PFN(pa));
@@ -193,9 +193,12 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
}
if (folio)
folio_put(folio);
+ if (return_max_wsum)
+ max_wsum = max(damon_probe_hits_wsum(r, false,
+ ctx), max_wsum);
}
}
- return 0;
+ return max_wsum;
}
/*
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum
2026-07-06 14:19 ` [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum SJ Park
@ 2026-07-06 14:44 ` sashiko-bot
2026-07-06 23:47 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 14:44 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `probe_hits` array in `struct damon_region` is of type `unsigned char`, causing silent wrap-around if the number of sampling intervals in an aggregation interval (`aggr_samples`) exceeds 255.
--
commit d141f3812ddd7e9395e9f362f8ddc5918e7ab555
Author: SJ Park <sj@kernel.org>
mm/damon/paddr: respect return_max_wsum
apply_probes() ops implementation in DAMON_PADDR is ignoring
return_max_wsum. Respect it.
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 3eba3564a31fc..b85f88a7a38f4 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
[ ... ]
> @@ -193,9 +193,12 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> }
> if (folio)
> folio_put(folio);
> + if (return_max_wsum)
> + max_wsum = max(damon_probe_hits_wsum(r, false,
> + ctx), max_wsum);
> }
> }
> - return 0;
> + return max_wsum;
> }
[Severity: High]
This is a pre-existing issue, but I noticed something about the probe_hits
array while reviewing this area. Does this code silently wrap around if the
number of samples in an aggregation interval exceeds 255?
Looking at the damon_region structure in include/linux/damon.h, probe_hits
is defined as an unsigned char array:
include/linux/damon.h:struct damon_region {
...
unsigned char probe_hits[DAMON_MAX_PROBES];
...
};
In mm/damon/paddr.c:damon_pa_apply_probes(), this value is incremented on
every sampling interval without bounds checking:
mm/damon/paddr.c:damon_pa_apply_probes() {
...
damon_for_each_probe(p, ctx) {
if (damon_pa_filter_pass(pa, folio, p))
r->probe_hits[i]++;
i++;
}
...
}
If a user configures the sysfs settings such that the aggregation interval
is significantly larger than the sample interval (for example, a 1ms
sample interval and a 1s aggregation interval yields 1000 samples), will
r->probe_hits[i] wrap around and start over from zero?
If so, it appears this might break DAMON region merging and monitoring
statistics by operating on incorrect, wrapped-around numbers.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=6
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum
2026-07-06 14:44 ` sashiko-bot
@ 2026-07-06 23:47 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:47 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 14:44:25 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The `probe_hits` array in `struct damon_region` is of type `unsigned char`, causing silent wrap-around if the number of sampling intervals in an aggregation interval (`aggr_samples`) exceeds 255.
> --
>
> commit d141f3812ddd7e9395e9f362f8ddc5918e7ab555
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/paddr: respect return_max_wsum
>
> apply_probes() ops implementation in DAMON_PADDR is ignoring
> return_max_wsum. Respect it.
>
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 3eba3564a31fc..b85f88a7a38f4 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
>
> [ ... ]
>
> > @@ -193,9 +193,12 @@ static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> > }
> > if (folio)
> > folio_put(folio);
> > + if (return_max_wsum)
> > + max_wsum = max(damon_probe_hits_wsum(r, false,
> > + ctx), max_wsum);
> > }
> > }
> > - return 0;
> > + return max_wsum;
> > }
>
> [Severity: High]
> This is a pre-existing issue, but I noticed something about the probe_hits
> array while reviewing this area. Does this code silently wrap around if the
> number of samples in an aggregation interval exceeds 255?
>
> Looking at the damon_region structure in include/linux/damon.h, probe_hits
> is defined as an unsigned char array:
>
> include/linux/damon.h:struct damon_region {
> ...
> unsigned char probe_hits[DAMON_MAX_PROBES];
> ...
> };
>
> In mm/damon/paddr.c:damon_pa_apply_probes(), this value is incremented on
> every sampling interval without bounds checking:
>
> mm/damon/paddr.c:damon_pa_apply_probes() {
> ...
> damon_for_each_probe(p, ctx) {
> if (damon_pa_filter_pass(pa, folio, p))
> r->probe_hits[i]++;
> i++;
> }
> ...
> }
>
> If a user configures the sysfs settings such that the aggregation interval
> is significantly larger than the sample interval (for example, a 1ms
> sample interval and a 1s aggregation interval yields 1000 samples), will
> r->probe_hits[i] wrap around and start over from zero?
>
> If so, it appears this might break DAMON region merging and monitoring
> statistics by operating on incorrect, wrapped-around numbers.
Known low priority issue. No blocker.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=6
Thanks,
SJ
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (5 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 06/16] mm/damon/paddr: respect return_max_wsum SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 14:55 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set SJ Park
` (9 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When probe weights are set, users may want DAMON monitoring results to
be optimized for the weights. For that, regions adjustment should work
for the weighted sum of probe hits. Extend damon_merge_regions_of() to
detect if the weights are set, and work with probe hits in the case.
The weights setup detection function is incomplete. It always returns
false. It is intentional, so that more changes to completely support
weights can be made in an incremental but safe way. Until the function
is completed, all changes depend on it is no-op, so DAMON works in the
current mode.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 48 +++++++++++++++++++++++++++++--------
mm/damon/tests/core-kunit.h | 13 ++++++++--
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index d7d1e27d767eb..9bb884ab135ec 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -209,6 +209,11 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
return NULL;
}
+static bool damon_has_probe_weights(struct damon_ctx *c)
+{
+ return false;
+}
+
/*
* damon_mvsum() - Returns pseudo moving sum value for a time window.
* @current_nr: The value of the current aggregation window.
@@ -3231,6 +3236,16 @@ static void damon_merge_two_regions(struct damon_target *t,
damon_destroy_region(r, t);
}
+static unsigned int damon_merge_score(struct damon_region *r, bool last,
+ struct damon_ctx *ctx, bool use_probe_hits)
+{
+ if (use_probe_hits)
+ return damon_probe_hits_wsum(r, last, ctx);
+ if (last)
+ return r->last_nr_accesses;
+ return r->nr_accesses;
+}
+
/*
* Merge adjacent regions having similar access frequencies
*
@@ -3239,24 +3254,37 @@ static void damon_merge_two_regions(struct damon_target *t,
* sz_limit size upper limit of each region
*/
static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
- unsigned long sz_limit)
+ unsigned long sz_limit, struct damon_ctx *ctx)
{
struct damon_region *r, *prev = NULL, *next;
+ bool use_probe_hits = damon_has_probe_weights(ctx);
damon_for_each_region_safe(r, next, t) {
- if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
+ unsigned int score, last_score;
+
+ score = damon_merge_score(r, false, ctx, use_probe_hits);
+ last_score = damon_merge_score(r, true, ctx, use_probe_hits);
+
+ if (abs(score - last_score) > thres)
r->age = 0;
- else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
+ else if ((score == 0) != (last_score == 0))
r->age = 0;
else
r->age++;
- if (prev && prev->ar.end == r->ar.start &&
- abs(prev->nr_accesses - r->nr_accesses) <= thres &&
- damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
- damon_merge_two_regions(t, prev, r);
- else
- prev = r;
+ if (!prev)
+ goto set_prev_continue;
+ if (prev->ar.end != r->ar.start)
+ goto set_prev_continue;
+ if (abs(damon_merge_score(prev, false, ctx, use_probe_hits) -
+ score) > thres)
+ goto set_prev_continue;
+ if (damon_sz_region(prev) + damon_sz_region(r) > sz_limit)
+ goto set_prev_continue;
+ damon_merge_two_regions(t, prev, r);
+ continue;
+set_prev_continue:
+ prev = r;
}
}
@@ -3289,7 +3317,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
do {
nr_regions = 0;
damon_for_each_target(t, c) {
- damon_merge_regions_of(t, threshold, sz_limit);
+ damon_merge_regions_of(t, threshold, sz_limit, c);
nr_regions += damon_nr_regions(t);
}
threshold = max(1, threshold * 2);
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 0124f83b39b83..fe948000b72cf 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -226,6 +226,7 @@ static struct damon_region *__nth_region_of(struct damon_target *t, int idx)
static void damon_test_merge_regions_of(struct kunit *test)
{
+ struct damon_ctx *ctx;
struct damon_target *t;
struct damon_region *r;
unsigned long sa[] = {0, 100, 114, 122, 130, 156, 170, 184, 230};
@@ -236,20 +237,27 @@ static void damon_test_merge_regions_of(struct kunit *test)
unsigned long eaddrs[] = {112, 130, 156, 170, 230, 10170};
int i;
+ ctx = damon_new_ctx();
+ if (!ctx)
+ kunit_skip(test, "ctx alloc fail");
+
t = damon_new_target();
- if (!t)
+ if (!t) {
+ damon_destroy_ctx(ctx);
kunit_skip(test, "target alloc fail");
+ }
for (i = 0; i < ARRAY_SIZE(sa); i++) {
r = damon_new_region(sa[i], ea[i]);
if (!r) {
damon_free_target(t);
+ damon_destroy_ctx(ctx);
kunit_skip(test, "region alloc fail");
}
r->nr_accesses = nrs[i];
damon_add_region(r, t);
}
- damon_merge_regions_of(t, 9, 9999);
+ damon_merge_regions_of(t, 9, 9999, ctx);
/* 0-112, 114-130, 130-156, 156-170, 170-230, 230-10170 */
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 6u);
for (i = 0; i < 6; i++) {
@@ -258,6 +266,7 @@ static void damon_test_merge_regions_of(struct kunit *test)
KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
}
damon_free_target(t);
+ damon_destroy_ctx(ctx);
}
static void damon_test_split_regions_of(struct kunit *test)
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits
2026-07-06 14:19 ` [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits SJ Park
@ 2026-07-06 14:55 ` sashiko-bot
2026-07-06 23:52 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 14:55 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] API Misuse / Silent Overflow: Using `abs()` on the difference of two large unsigned integers causes incorrect results.
- [High] Integer Overflow: `damon_probe_hits_wsum()` wraps around the 32-bit boundary when large weights are used.
--
commit b2b82783307a6d10c78010502dd30f5168036627
Author: SJ Park <sj@kernel.org>
mm/damon/core: extend merge function to work with probe hits
When probe weights are set, this commit extends damon_merge_regions_of() to
work with the weighted sum of probe hits. It also introduces a placeholder
detection function to allow safe incremental changes.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 42f27849af722..7aff741d43bd4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3230,6 +3235,16 @@ static void damon_merge_two_regions(struct damon_target *t,
> damon_destroy_region(r, t);
> }
>
> +static unsigned int damon_merge_score(struct damon_region *r, bool last,
> + struct damon_ctx *ctx, bool use_probe_hits)
> +{
> + if (use_probe_hits)
> + return damon_probe_hits_wsum(r, last, ctx);
^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this code wrap around the 32-bit boundary when large weights are used?
Looking at damon_probe_hits_wsum(), it accumulates probe_hits[i] * probe->weight
into a 32-bit unsigned int sum:
mm/damon/core.c:damon_probe_hits_wsum() {
...
unsigned int sum = 0;
...
if (last)
sum += r->last_probe_hits[i++] * probe->weight;
else
sum += r->probe_hits[i++] * probe->weight;
...
return sum;
}
If a user configures a large weight (e.g., > 16,843,000) for a probe via
sysfs, this could wrap around (e.g., 255 hits * 40,000,000 weight = 10.2
billion, which wraps to ~1.6 billion).
Could this break the scoring mechanism, causing damon_merge_regions_of() to
merge active and inactive regions incorrectly?
> + if (last)
> + return r->last_nr_accesses;
> + return r->nr_accesses;
> +}
> +
[ ... ]
> @@ -3238,24 +3253,37 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> - unsigned long sz_limit)
> + unsigned long sz_limit, struct damon_ctx *ctx)
> {
> struct damon_region *r, *prev = NULL, *next;
> + bool use_probe_hits = damon_has_probe_weights(ctx);
>
> damon_for_each_region_safe(r, next, t) {
> - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> + unsigned int score, last_score;
> +
> + score = damon_merge_score(r, false, ctx, use_probe_hits);
> + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> +
> + if (abs(score - last_score) > thres)
^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Can this call to abs() cause incorrect results due to silent overflow?
Since score and last_score are unsigned integers, abs(score - last_score)
casts the unsigned difference to a signed int.
If the actual difference exceeds the maximum signed integer value, it wraps
to a negative value, which abs() then negates into a smaller positive value.
Could this falsely small difference cause the threshold check > thres to fail
incorrectly, allowing vastly different regions to be merged and affecting the
accuracy of the monitor?
> r->age = 0;
> - else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> + else if ((score == 0) != (last_score == 0))
> r->age = 0;
> else
> r->age++;
>
> - if (prev && prev->ar.end == r->ar.start &&
> - abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> - damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
> - damon_merge_two_regions(t, prev, r);
> - else
> - prev = r;
> + if (!prev)
> + goto set_prev_continue;
> + if (prev->ar.end != r->ar.start)
> + goto set_prev_continue;
> + if (abs(damon_merge_score(prev, false, ctx, use_probe_hits) -
> + score) > thres)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this second use of abs() on the unsigned difference of the merge scores
suffer from the exact same signed-integer overflow vulnerability?
> + goto set_prev_continue;
> + if (damon_sz_region(prev) + damon_sz_region(r) > sz_limit)
> + goto set_prev_continue;
> + damon_merge_two_regions(t, prev, r);
> + continue;
> +set_prev_continue:
> + prev = r;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=7
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits
2026-07-06 14:55 ` sashiko-bot
@ 2026-07-06 23:52 ` SJ Park
2026-07-07 3:30 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:52 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 14:55:38 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] API Misuse / Silent Overflow: Using `abs()` on the difference of two large unsigned integers causes incorrect results.
> - [High] Integer Overflow: `damon_probe_hits_wsum()` wraps around the 32-bit boundary when large weights are used.
> --
>
> commit b2b82783307a6d10c78010502dd30f5168036627
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: extend merge function to work with probe hits
>
> When probe weights are set, this commit extends damon_merge_regions_of() to
> work with the weighted sum of probe hits. It also introduces a placeholder
> detection function to allow safe incremental changes.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 42f27849af722..7aff741d43bd4 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
>
> [ ... ]
>
> > @@ -3230,6 +3235,16 @@ static void damon_merge_two_regions(struct damon_target *t,
> > damon_destroy_region(r, t);
> > }
> >
> > +static unsigned int damon_merge_score(struct damon_region *r, bool last,
> > + struct damon_ctx *ctx, bool use_probe_hits)
> > +{
> > + if (use_probe_hits)
> > + return damon_probe_hits_wsum(r, last, ctx);
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: High]
> Does this code wrap around the 32-bit boundary when large weights are used?
>
> Looking at damon_probe_hits_wsum(), it accumulates probe_hits[i] * probe->weight
> into a 32-bit unsigned int sum:
>
> mm/damon/core.c:damon_probe_hits_wsum() {
> ...
> unsigned int sum = 0;
> ...
> if (last)
> sum += r->last_probe_hits[i++] * probe->weight;
> else
> sum += r->probe_hits[i++] * probe->weight;
> ...
> return sum;
> }
>
> If a user configures a large weight (e.g., > 16,843,000) for a probe via
> sysfs, this could wrap around (e.g., 255 hits * 40,000,000 weight = 10.2
> billion, which wraps to ~1.6 billion).
>
> Could this break the scoring mechanism, causing damon_merge_regions_of() to
> merge active and inactive regions incorrectly?
Known low priority issue. No blocker.
>
> > + if (last)
> > + return r->last_nr_accesses;
> > + return r->nr_accesses;
> > +}
> > +
>
> [ ... ]
>
> > @@ -3238,24 +3253,37 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> > - unsigned long sz_limit)
> > + unsigned long sz_limit, struct damon_ctx *ctx)
> > {
> > struct damon_region *r, *prev = NULL, *next;
> > + bool use_probe_hits = damon_has_probe_weights(ctx);
> >
> > damon_for_each_region_safe(r, next, t) {
> > - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> > + unsigned int score, last_score;
> > +
> > + score = damon_merge_score(r, false, ctx, use_probe_hits);
> > + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> > +
> > + if (abs(score - last_score) > thres)
> ^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: High]
> Can this call to abs() cause incorrect results due to silent overflow?
>
> Since score and last_score are unsigned integers, abs(score - last_score)
> casts the unsigned difference to a signed int.
>
> If the actual difference exceeds the maximum signed integer value, it wraps
> to a negative value, which abs() then negates into a smaller positive value.
>
> Could this falsely small difference cause the threshold check > thres to fail
> incorrectly, allowing vastly different regions to be merged and affecting the
> accuracy of the monitor?
Same issue exists for the above abs() call. User impact is minor and I'm
separately working [1] on it.
>
> > r->age = 0;
> > - else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> > + else if ((score == 0) != (last_score == 0))
> > r->age = 0;
> > else
> > r->age++;
> >
> > - if (prev && prev->ar.end == r->ar.start &&
> > - abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> > - damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
> > - damon_merge_two_regions(t, prev, r);
> > - else
> > - prev = r;
> > + if (!prev)
> > + goto set_prev_continue;
> > + if (prev->ar.end != r->ar.start)
> > + goto set_prev_continue;
> > + if (abs(damon_merge_score(prev, false, ctx, use_probe_hits) -
> > + score) > thres)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: High]
> Does this second use of abs() on the unsigned difference of the merge scores
> suffer from the exact same signed-integer overflow vulnerability?
Ditto.
>
> > + goto set_prev_continue;
> > + if (damon_sz_region(prev) + damon_sz_region(r) > sz_limit)
> > + goto set_prev_continue;
> > + damon_merge_two_regions(t, prev, r);
> > + continue;
> > +set_prev_continue:
> > + prev = r;
> > }
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=7
[1] https://lore.kernel.org/20260705211101.F2F061F000E9@smtp.kernel.org
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits
2026-07-06 23:52 ` SJ Park
@ 2026-07-07 3:30 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 3:30 UTC (permalink / raw)
To: SJ Park; +Cc: sashiko-bot, damon
On Mon, 6 Jul 2026 16:52:00 -0700 SJ Park <sj@kernel.org> wrote:
> On Mon, 06 Jul 2026 14:55:38 +0000 sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] API Misuse / Silent Overflow: Using `abs()` on the difference of two large unsigned integers causes incorrect results.
> > - [High] Integer Overflow: `damon_probe_hits_wsum()` wraps around the 32-bit boundary when large weights are used.
> > --
> >
> > commit b2b82783307a6d10c78010502dd30f5168036627
> > Author: SJ Park <sj@kernel.org>
> >
> > mm/damon/core: extend merge function to work with probe hits
> >
> > When probe weights are set, this commit extends damon_merge_regions_of() to
> > work with the weighted sum of probe hits. It also introduces a placeholder
> > detection function to allow safe incremental changes.
> >
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index 42f27849af722..7aff741d43bd4 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> >
> > [ ... ]
> >
> > > @@ -3230,6 +3235,16 @@ static void damon_merge_two_regions(struct damon_target *t,
> > > damon_destroy_region(r, t);
> > > }
> > >
> > > +static unsigned int damon_merge_score(struct damon_region *r, bool last,
> > > + struct damon_ctx *ctx, bool use_probe_hits)
> > > +{
> > > + if (use_probe_hits)
> > > + return damon_probe_hits_wsum(r, last, ctx);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > [Severity: High]
> > Does this code wrap around the 32-bit boundary when large weights are used?
> >
> > Looking at damon_probe_hits_wsum(), it accumulates probe_hits[i] * probe->weight
> > into a 32-bit unsigned int sum:
> >
> > mm/damon/core.c:damon_probe_hits_wsum() {
> > ...
> > unsigned int sum = 0;
> > ...
> > if (last)
> > sum += r->last_probe_hits[i++] * probe->weight;
> > else
> > sum += r->probe_hits[i++] * probe->weight;
> > ...
> > return sum;
> > }
> >
> > If a user configures a large weight (e.g., > 16,843,000) for a probe via
> > sysfs, this could wrap around (e.g., 255 hits * 40,000,000 weight = 10.2
> > billion, which wraps to ~1.6 billion).
> >
> > Could this break the scoring mechanism, causing damon_merge_regions_of() to
> > merge active and inactive regions incorrectly?
>
> Known low priority issue. No blocker.
>
> >
> > > + if (last)
> > > + return r->last_nr_accesses;
> > > + return r->nr_accesses;
> > > +}
> > > +
> >
> > [ ... ]
> >
> > > @@ -3238,24 +3253,37 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> > > - unsigned long sz_limit)
> > > + unsigned long sz_limit, struct damon_ctx *ctx)
> > > {
> > > struct damon_region *r, *prev = NULL, *next;
> > > + bool use_probe_hits = damon_has_probe_weights(ctx);
> > >
> > > damon_for_each_region_safe(r, next, t) {
> > > - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> > > + unsigned int score, last_score;
> > > +
> > > + score = damon_merge_score(r, false, ctx, use_probe_hits);
> > > + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> > > +
> > > + if (abs(score - last_score) > thres)
> > ^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > [Severity: High]
> > Can this call to abs() cause incorrect results due to silent overflow?
> >
> > Since score and last_score are unsigned integers, abs(score - last_score)
> > casts the unsigned difference to a signed int.
> >
> > If the actual difference exceeds the maximum signed integer value, it wraps
> > to a negative value, which abs() then negates into a smaller positive value.
> >
> > Could this falsely small difference cause the threshold check > thres to fail
> > incorrectly, allowing vastly different regions to be merged and affecting the
> > accuracy of the monitor?
>
> Same issue exists for the above abs() call. User impact is minor and I'm
> separately working [1] on it.
Now I think this has no reason to fix right now. I'll fix it in the next
revision.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (6 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 07/16] mm/damon/core: extend merge function to work with probe hits SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:03 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if " SJ Park
` (8 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When users set probe weights, they are interested in the data
attributes. Not necessarily they are interested in data access
patterns. Disable the data access monitoring in the case.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 9bb884ab135ec..ad0f28469e633 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3714,17 +3714,18 @@ static int kdamond_fn(void *data)
unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
unsigned long sample_interval = ctx->attrs.sample_interval;
+ bool access_check_disabled = damon_has_probe_weights(ctx);
if (kdamond_wait_activation(ctx))
break;
- if (ctx->ops.prepare_access_checks)
+ if (!access_check_disabled && ctx->ops.prepare_access_checks)
ctx->ops.prepare_access_checks(ctx);
kdamond_usleep(sample_interval);
ctx->passed_sample_intervals++;
- if (ctx->ops.check_accesses)
+ if (!access_check_disabled && ctx->ops.check_accesses)
max_nr_accesses = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes)
ctx->ops.apply_probes(ctx, false, false);
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set
2026-07-06 14:19 ` [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set SJ Park
@ 2026-07-06 15:03 ` sashiko-bot
2026-07-06 23:54 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:03 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The region merging loop termination condition is incompatible with weighted probe hits, leading to unbounded region growth.
- [High] Unconditionally disabling access checks forces `nr_accesses` to 0, breaking access-based DAMOS schemes and interval auto-tuning.
--
commit d9de5e6aadab2e17a0c4428844dec0cfc05d08ef
Author: SJ Park <sj@kernel.org>
mm/damon/core: disable access monitoring when probe weights are set
When users set probe weights, they are interested in the data attributes
rather than data access patterns. This commit disables data access
monitoring when probe weights are set.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 7aff741d43bd4..a2f768bd07223 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3713,17 +3713,18 @@ static int kdamond_fn(void *data)
> unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
> unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
> unsigned long sample_interval = ctx->attrs.sample_interval;
> + bool access_check_disabled = damon_has_probe_weights(ctx);
[Severity: High]
Can configuring probe weights lead to unbounded region growth during merging?
When damon_has_probe_weights(ctx) is true, the region merging loop in
kdamond_merge_regions() compares a threshold scaled with weighted probe hits
against max_thres:
mm/damon/core.c:kdamond_merge_regions() {
...
max_thres = c->attrs.aggr_interval /
(c->attrs.sample_interval ? c->attrs.sample_interval : 1);
do {
...
} while (nr_regions > c->attrs.max_nr_regions &&
threshold / 2 < max_thres);
}
Since max_thres is hardcoded to the maximum unscaled accesses
(aggr_interval / sample_interval) and doesn't account for probe weights, the
loop could terminate prematurely.
If weighted probe hits result in score differences exceeding this unscaled cap,
wouldn't this prevent regions from merging back down below the user-configured
max_nr_regions limit?
>
> if (kdamond_wait_activation(ctx))
> break;
>
> - if (ctx->ops.prepare_access_checks)
> + if (!access_check_disabled && ctx->ops.prepare_access_checks)
> ctx->ops.prepare_access_checks(ctx);
>
> kdamond_usleep(sample_interval);
> ctx->passed_sample_intervals++;
>
> - if (ctx->ops.check_accesses)
> + if (!access_check_disabled && ctx->ops.check_accesses)
> max_nr_accesses = ctx->ops.check_accesses(ctx);
[Severity: High]
Does disabling the access checks here break DAMOS schemes and interval
auto-tuning?
By preventing ctx->ops.check_accesses from running when probe weights are
configured, r->nr_accesses will remain 0.
Since damon_nr_accesses_mvsum() consumes these 0s, it seems this would cause
interval auto-tuning to exponentially increase the sampling interval until it
hits the maximum allowed value.
Additionally, wouldn't this force all regions to instantly match any DAMOS
scheme configured to target low-access memory?
> if (ctx->ops.apply_probes)
> ctx->ops.apply_probes(ctx, false, false);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=8
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set
2026-07-06 15:03 ` sashiko-bot
@ 2026-07-06 23:54 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:54 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:03:08 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The region merging loop termination condition is incompatible with weighted probe hits, leading to unbounded region growth.
> - [High] Unconditionally disabling access checks forces `nr_accesses` to 0, breaking access-based DAMOS schemes and interval auto-tuning.
> --
>
> commit d9de5e6aadab2e17a0c4428844dec0cfc05d08ef
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: disable access monitoring when probe weights are set
>
> When users set probe weights, they are interested in the data attributes
> rather than data access patterns. This commit disables data access
> monitoring when probe weights are set.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 7aff741d43bd4..a2f768bd07223 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3713,17 +3713,18 @@ static int kdamond_fn(void *data)
> > unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
> > unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
> > unsigned long sample_interval = ctx->attrs.sample_interval;
> > + bool access_check_disabled = damon_has_probe_weights(ctx);
>
> [Severity: High]
> Can configuring probe weights lead to unbounded region growth during merging?
>
> When damon_has_probe_weights(ctx) is true, the region merging loop in
> kdamond_merge_regions() compares a threshold scaled with weighted probe hits
> against max_thres:
>
> mm/damon/core.c:kdamond_merge_regions() {
> ...
> max_thres = c->attrs.aggr_interval /
> (c->attrs.sample_interval ? c->attrs.sample_interval : 1);
> do {
> ...
> } while (nr_regions > c->attrs.max_nr_regions &&
> threshold / 2 < max_thres);
> }
>
> Since max_thres is hardcoded to the maximum unscaled accesses
> (aggr_interval / sample_interval) and doesn't account for probe weights, the
> loop could terminate prematurely.
>
> If weighted probe hits result in score differences exceeding this unscaled cap,
> wouldn't this prevent regions from merging back down below the user-configured
> max_nr_regions limit?
No such issue can happen because damon_has_probe_weights() always returns
false.
>
> >
> > if (kdamond_wait_activation(ctx))
> > break;
> >
> > - if (ctx->ops.prepare_access_checks)
> > + if (!access_check_disabled && ctx->ops.prepare_access_checks)
> > ctx->ops.prepare_access_checks(ctx);
> >
> > kdamond_usleep(sample_interval);
> > ctx->passed_sample_intervals++;
> >
> > - if (ctx->ops.check_accesses)
> > + if (!access_check_disabled && ctx->ops.check_accesses)
> > max_nr_accesses = ctx->ops.check_accesses(ctx);
>
> [Severity: High]
> Does disabling the access checks here break DAMOS schemes and interval
> auto-tuning?
>
> By preventing ctx->ops.check_accesses from running when probe weights are
> configured, r->nr_accesses will remain 0.
>
> Since damon_nr_accesses_mvsum() consumes these 0s, it seems this would cause
> interval auto-tuning to exponentially increase the sampling interval until it
> hits the maximum allowed value.
>
> Additionally, wouldn't this force all regions to instantly match any DAMOS
> scheme configured to target low-access memory?
That's the intended behavior. Users are informed that access monitoring is
disabled in the mode.
>
> > if (ctx->ops.apply_probes)
> > ctx->ops.apply_probes(ctx, false, false);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=8
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (7 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 08/16] mm/damon/core: disable access monitoring when probe weights are set SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:02 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 10/16] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
` (7 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When probe weights are set, the prepare_access_checks() ops callback is
not invoked, so sampling addresses of regions are unset. Ask
apply_probes() to set those together in the case.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index ad0f28469e633..71008ee6ca243 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3728,7 +3728,8 @@ static int kdamond_fn(void *data)
if (!access_check_disabled && ctx->ops.check_accesses)
max_nr_accesses = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes)
- ctx->ops.apply_probes(ctx, false, false);
+ ctx->ops.apply_probes(ctx, access_check_disabled,
+ false);
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-06 14:19 ` [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if " SJ Park
@ 2026-07-06 15:02 ` sashiko-bot
2026-07-06 23:56 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:02 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Region merging loop bound `max_thres` is incorrectly scaled when probe weights are used, causing premature loop termination and permanent violation of `max_nr_regions` limits.
--
commit cc6e72a3b37a62b24190b0e42f141fba6caab8fc
Author: SJ Park <sj@kernel.org>
mm/damon/core: set samples in apply_probes() if probe weights are set
When probe weights are set, the prepare_access_checks() ops callback is
not invoked, so sampling addresses of regions are unset. Ask
apply_probes() to set those together in the case.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index a2f768bd07223..5311aa689c8e7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3727,7 +3727,8 @@ static int kdamond_fn(void *data)
> if (!access_check_disabled && ctx->ops.check_accesses)
> max_nr_accesses = ctx->ops.check_accesses(ctx);
> if (ctx->ops.apply_probes)
> - ctx->ops.apply_probes(ctx, false, false);
> + ctx->ops.apply_probes(ctx, access_check_disabled,
> + false);
[Severity: High]
Does enabling probe weights here introduce a regression with the region
merging loop bound in kdamond_merge_regions()?
Later in the series, a commit ("mm/damon/core: get merge threshold from
probe hits when weights are set") derives the merge threshold from weighted
sums when probe weights are used. However, the loop bound max_thres in
kdamond_merge_regions() remains tied to the raw access frequency:
mm/damon/core.c:kdamond_merge_regions() {
...
max_thres = c->attrs.aggr_interval /
(c->attrs.sample_interval ? c->attrs.sample_interval : 1);
do {
nr_regions = 0;
damon_for_each_target(t, c) {
damon_merge_regions_of(t, threshold, sz_limit, c);
nr_regions += damon_nr_regions(t);
}
threshold = max(1, threshold * 2);
} while (nr_regions > c->attrs.max_nr_regions &&
threshold / 2 < max_thres);
}
If a large weighted sum is used as the initial threshold, it seems the loop
will exit immediately on the first iteration without merging enough regions
because threshold / 2 will already exceed the raw max_thres.
Can this permanently violate the max_nr_regions limits?
>
> if (time_after_eq(ctx->passed_sample_intervals,
> next_aggregation_sis)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=9
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-06 15:02 ` sashiko-bot
@ 2026-07-06 23:56 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 23:56 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:02:05 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Region merging loop bound `max_thres` is incorrectly scaled when probe weights are used, causing premature loop termination and permanent violation of `max_nr_regions` limits.
> --
>
> commit cc6e72a3b37a62b24190b0e42f141fba6caab8fc
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: set samples in apply_probes() if probe weights are set
>
> When probe weights are set, the prepare_access_checks() ops callback is
> not invoked, so sampling addresses of regions are unset. Ask
> apply_probes() to set those together in the case.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index a2f768bd07223..5311aa689c8e7 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3727,7 +3727,8 @@ static int kdamond_fn(void *data)
> > if (!access_check_disabled && ctx->ops.check_accesses)
> > max_nr_accesses = ctx->ops.check_accesses(ctx);
> > if (ctx->ops.apply_probes)
> > - ctx->ops.apply_probes(ctx, false, false);
> > + ctx->ops.apply_probes(ctx, access_check_disabled,
> > + false);
>
> [Severity: High]
> Does enabling probe weights here introduce a regression with the region
> merging loop bound in kdamond_merge_regions()?
>
> Later in the series, a commit ("mm/damon/core: get merge threshold from
> probe hits when weights are set") derives the merge threshold from weighted
> sums when probe weights are used. However, the loop bound max_thres in
> kdamond_merge_regions() remains tied to the raw access frequency:
>
> mm/damon/core.c:kdamond_merge_regions() {
> ...
> max_thres = c->attrs.aggr_interval /
> (c->attrs.sample_interval ? c->attrs.sample_interval : 1);
> do {
> nr_regions = 0;
> damon_for_each_target(t, c) {
> damon_merge_regions_of(t, threshold, sz_limit, c);
> nr_regions += damon_nr_regions(t);
> }
> threshold = max(1, threshold * 2);
> } while (nr_regions > c->attrs.max_nr_regions &&
> threshold / 2 < max_thres);
> }
>
> If a large weighted sum is used as the initial threshold, it seems the loop
> will exit immediately on the first iteration without merging enough regions
> because threshold / 2 will already exceed the raw max_thres.
>
> Can this permanently violate the max_nr_regions limits?
No. damon_has_probe_weights() always returns false at this moment, so no
issue.
>
> >
> > if (time_after_eq(ctx->passed_sample_intervals,
> > next_aggregation_sis)) {
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=9
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 10/16] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn()
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (8 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if " SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 14:19 ` [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
` (6 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When probe weights are set, DAMON merge logic works with the probe hits
weighted sum. The core logic is setting the merge threshold based on
the access frequency, though. Before using probe hits based threshold,
rename the variables to look more general.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 71008ee6ca243..a7ca69ea8e565 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3681,7 +3681,6 @@ static void kdamond_init_ctx(struct damon_ctx *ctx)
static int kdamond_fn(void *data)
{
struct damon_ctx *ctx = data;
- unsigned int max_nr_accesses = 0;
unsigned long sz_limit = 0;
pr_debug("kdamond (%d) starts\n", current->pid);
@@ -3715,6 +3714,7 @@ static int kdamond_fn(void *data)
unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
unsigned long sample_interval = ctx->attrs.sample_interval;
bool access_check_disabled = damon_has_probe_weights(ctx);
+ unsigned int max_merge_score = 0;
if (kdamond_wait_activation(ctx))
break;
@@ -3726,7 +3726,7 @@ static int kdamond_fn(void *data)
ctx->passed_sample_intervals++;
if (!access_check_disabled && ctx->ops.check_accesses)
- max_nr_accesses = ctx->ops.check_accesses(ctx);
+ max_merge_score = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes)
ctx->ops.apply_probes(ctx, access_check_disabled,
false);
@@ -3734,7 +3734,7 @@ static int kdamond_fn(void *data)
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
kdamond_merge_regions(ctx,
- max_nr_accesses / 10,
+ max_merge_score / 10,
sz_limit);
/* online updates might be made */
sz_limit = damon_apply_min_nr_regions(ctx);
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (9 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 10/16] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:18 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight() SJ Park
` (5 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When probe weights are set, DAMON merges regions based on their probe
hits weighted sum. But the merge threshold is calculated based on the
access frequency. Update it to retrieve the maximum probe hits weighted
sum in the snapshot from apply_probes() ops callback, and generate the
threshold based on it.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index a7ca69ea8e565..83db073c74e2b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3714,7 +3714,8 @@ static int kdamond_fn(void *data)
unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
unsigned long sample_interval = ctx->attrs.sample_interval;
bool access_check_disabled = damon_has_probe_weights(ctx);
- unsigned int max_merge_score = 0;
+ unsigned int max_merge_score = 0, max_wsum;
+ bool get_max_wsum;
if (kdamond_wait_activation(ctx))
break;
@@ -3727,9 +3728,18 @@ static int kdamond_fn(void *data)
if (!access_check_disabled && ctx->ops.check_accesses)
max_merge_score = ctx->ops.check_accesses(ctx);
- if (ctx->ops.apply_probes)
- ctx->ops.apply_probes(ctx, access_check_disabled,
- false);
+ if (ctx->ops.apply_probes) {
+ if (time_after_eq(ctx->passed_sample_intervals,
+ next_aggregation_sis) &&
+ access_check_disabled)
+ get_max_wsum = true;
+ else
+ get_max_wsum = false;
+ max_wsum = ctx->ops.apply_probes(ctx,
+ access_check_disabled, get_max_wsum);
+ if (get_max_wsum)
+ max_merge_score = max_wsum;
+ }
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-06 14:19 ` [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
@ 2026-07-06 15:18 ` sashiko-bot
2026-07-07 0:03 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:18 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Region merge loop terminates prematurely because `max_thres` is unscaled for probe hit weighted sums, causing a failure to respect `max_nr_regions`.
- [High] Adding probe weights to contexts using operations without `apply_probes` (e.g., `vaddr`) silently breaks all monitoring.
--
commit f896f3e81bf75b446b16cb4f79f6673a154b512b
Author: SJ Park <sj@kernel.org>
mm/damon/core: get merge threshold from probe hits when weights are set
When probe weights are set, DAMON merges regions based on their probe
hits weighted sum. But the merge threshold is calculated based on the
access frequency. Update it to retrieve the maximum probe hits weighted
sum in the snapshot from apply_probes() ops callback, and generate the
threshold based on it.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index be026c86384e4..fa78d8d592aa3 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3726,9 +3727,18 @@ static int kdamond_fn(void *data)
>
> if (!access_check_disabled && ctx->ops.check_accesses)
> max_merge_score = ctx->ops.check_accesses(ctx);
[Severity: High]
Does setting probe weights on a context that lacks an apply_probes callback
(like vaddr) silently break monitoring here?
If a user sets probe weights via sysfs, access_check_disabled will be true.
This skips check_accesses(), but since apply_probes is NULL, neither callback
runs. The accesses are never updated, and all memory appears completely cold.
> - if (ctx->ops.apply_probes)
> - ctx->ops.apply_probes(ctx, access_check_disabled,
> - false);
> + if (ctx->ops.apply_probes) {
> + if (time_after_eq(ctx->passed_sample_intervals,
> + next_aggregation_sis) &&
> + access_check_disabled)
> + get_max_wsum = true;
> + else
> + get_max_wsum = false;
> + max_wsum = ctx->ops.apply_probes(ctx,
> + access_check_disabled, get_max_wsum);
> + if (get_max_wsum)
> + max_merge_score = max_wsum;
[Severity: High]
Can this cause kdamond_merge_regions() to fail to reduce the number of
regions below max_nr_regions?
When kdamond_fn() later passes this max_wsum / 10 as the initial threshold
to kdamond_merge_regions(), the merge loop termination condition compares
it against an unscaled max_thres:
mm/damon/core.c:kdamond_merge_regions() {
...
max_thres = c->attrs.aggr_interval /
(c->attrs.sample_interval ? c->attrs.sample_interval : 1);
...
} while (nr_regions > c->attrs.max_nr_regions &&
threshold / 2 < max_thres);
}
Since a user-defined weighted sum can be vastly larger than the unweighted
max_thres, will the loop condition fail prematurely, leading to an
unbounded growth of memory regions?
> + }
>
> if (time_after_eq(ctx->passed_sample_intervals,
> next_aggregation_sis)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=11
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-06 15:18 ` sashiko-bot
@ 2026-07-07 0:03 ` SJ Park
2026-07-07 3:34 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-07 0:03 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:18:01 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Region merge loop terminates prematurely because `max_thres` is unscaled for probe hit weighted sums, causing a failure to respect `max_nr_regions`.
> - [High] Adding probe weights to contexts using operations without `apply_probes` (e.g., `vaddr`) silently breaks all monitoring.
> --
>
> commit f896f3e81bf75b446b16cb4f79f6673a154b512b
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: get merge threshold from probe hits when weights are set
>
> When probe weights are set, DAMON merges regions based on their probe
> hits weighted sum. But the merge threshold is calculated based on the
> access frequency. Update it to retrieve the maximum probe hits weighted
> sum in the snapshot from apply_probes() ops callback, and generate the
> threshold based on it.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index be026c86384e4..fa78d8d592aa3 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3726,9 +3727,18 @@ static int kdamond_fn(void *data)
> >
> > if (!access_check_disabled && ctx->ops.check_accesses)
> > max_merge_score = ctx->ops.check_accesses(ctx);
>
> [Severity: High]
> Does setting probe weights on a context that lacks an apply_probes callback
> (like vaddr) silently break monitoring here?
>
> If a user sets probe weights via sysfs, access_check_disabled will be true.
> This skips check_accesses(), but since apply_probes is NULL, neither callback
> runs. The accesses are never updated, and all memory appears completely cold.
I was initially thinking this is no blocker. But, since the core layer params
validation patch series is merged into mm-new, fixing this should be simple,
and we don't really need to rush for this series. I will fix this in the next
spin.
>
> > - if (ctx->ops.apply_probes)
> > - ctx->ops.apply_probes(ctx, access_check_disabled,
> > - false);
> > + if (ctx->ops.apply_probes) {
> > + if (time_after_eq(ctx->passed_sample_intervals,
> > + next_aggregation_sis) &&
> > + access_check_disabled)
> > + get_max_wsum = true;
> > + else
> > + get_max_wsum = false;
> > + max_wsum = ctx->ops.apply_probes(ctx,
> > + access_check_disabled, get_max_wsum);
> > + if (get_max_wsum)
> > + max_merge_score = max_wsum;
>
> [Severity: High]
> Can this cause kdamond_merge_regions() to fail to reduce the number of
> regions below max_nr_regions?
>
> When kdamond_fn() later passes this max_wsum / 10 as the initial threshold
> to kdamond_merge_regions(), the merge loop termination condition compares
> it against an unscaled max_thres:
>
> mm/damon/core.c:kdamond_merge_regions() {
> ...
> max_thres = c->attrs.aggr_interval /
> (c->attrs.sample_interval ? c->attrs.sample_interval : 1);
> ...
> } while (nr_regions > c->attrs.max_nr_regions &&
> threshold / 2 < max_thres);
> }
>
> Since a user-defined weighted sum can be vastly larger than the unweighted
> max_thres, will the loop condition fail prematurely, leading to an
> unbounded growth of memory regions?
Yes, but only for one aggregation interval. I don't think that's an issue.
>
> > + }
> >
> > if (time_after_eq(ctx->passed_sample_intervals,
> > next_aggregation_sis)) {
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=11
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-07 0:03 ` SJ Park
@ 2026-07-07 3:34 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 3:34 UTC (permalink / raw)
To: SJ Park; +Cc: sashiko-bot, damon
On Mon, 6 Jul 2026 17:03:28 -0700 SJ Park <sj@kernel.org> wrote:
> On Mon, 06 Jul 2026 15:18:01 +0000 sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] Region merge loop terminates prematurely because `max_thres` is unscaled for probe hit weighted sums, causing a failure to respect `max_nr_regions`.
> > - [High] Adding probe weights to contexts using operations without `apply_probes` (e.g., `vaddr`) silently breaks all monitoring.
> > --
> >
> > commit f896f3e81bf75b446b16cb4f79f6673a154b512b
> > Author: SJ Park <sj@kernel.org>
> >
> > mm/damon/core: get merge threshold from probe hits when weights are set
> >
> > When probe weights are set, DAMON merges regions based on their probe
> > hits weighted sum. But the merge threshold is calculated based on the
> > access frequency. Update it to retrieve the maximum probe hits weighted
> > sum in the snapshot from apply_probes() ops callback, and generate the
> > threshold based on it.
> >
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index be026c86384e4..fa78d8d592aa3 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > [ ... ]
> > > @@ -3726,9 +3727,18 @@ static int kdamond_fn(void *data)
> > >
> > > if (!access_check_disabled && ctx->ops.check_accesses)
> > > max_merge_score = ctx->ops.check_accesses(ctx);
> >
> > [Severity: High]
> > Does setting probe weights on a context that lacks an apply_probes callback
> > (like vaddr) silently break monitoring here?
> >
> > If a user sets probe weights via sysfs, access_check_disabled will be true.
> > This skips check_accesses(), but since apply_probes is NULL, neither callback
> > runs. The accesses are never updated, and all memory appears completely cold.
>
> I was initially thinking this is no blocker. But, since the core layer params
> validation patch series is merged into mm-new, fixing this should be simple,
> and we don't really need to rush for this series. I will fix this in the next
> spin.
I changed my mind. There are many features that supported by only specific
DAMON ops implementations. We allow setting such configuration, and later add
supports if needed. It is arguable, but I think keeping the consistency is the
right thing for now. I will keep this behavior unless someone argues
differently.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight()
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (10 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 11/16] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:16 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file SJ Park
` (4 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
DAMON monitors only data attributes when probe weights are set.
However, the function for detecting it is intentionally returning false
always. The intention was to make changes for complete support of the
mode in a safe and incremental way. Now all the building blocks are
made. Complete the mode detection function so that the mode can really
start working.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 83db073c74e2b..b44393ccaf6b1 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -211,6 +211,12 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
static bool damon_has_probe_weights(struct damon_ctx *c)
{
+ struct damon_probe *p;
+
+ damon_for_each_probe(p, c) {
+ if (p->weight)
+ return true;
+ }
return false;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight()
2026-07-06 14:19 ` [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight() SJ Park
@ 2026-07-06 15:16 ` sashiko-bot
2026-07-07 0:05 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:16 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Setting probe weights on a DAMON context whose operations do not support `apply_probes` (e.g., `vaddr`) silently disables all monitoring.
Pre-existing issues:
- [High] The `probe_hits` array in `struct damon_region` uses `unsigned char` counters which wrap around at 256, corrupting monitoring data when the number of samples per aggregation interval exceeds 255.
--
commit 3891f362834bef45987ebcee1cdb0abcd9df6ea2
Author: SJ Park <sj@kernel.org>
mm/damon/core: implement damon_has_probe_weight()
DAMON monitors only data attributes when probe weights are set.
However, the function for detecting it is intentionally returning false
always. The intention was to make changes for complete support of the
mode in a safe and incremental way. Now all the building blocks are
made. Complete the mode detection function so that the mode can really
start working.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index fa78d8d592aa3..6e5a029989a0c 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -211,6 +211,12 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
>
> static bool damon_has_probe_weights(struct damon_ctx *c)
> {
> + struct damon_probe *p;
> +
> + damon_for_each_probe(p, c) {
> + if (p->weight)
> + return true;
[Severity: High]
Does returning true here silently disable monitoring if the context operations
do not support probes (e.g., vaddr)?
In kdamond_fn(), if this returns true, access_check_disabled bypasses standard
checks. However, if ctx->ops.apply_probes is NULL, the probes are skipped:
mm/damon/core.c:kdamond_fn() {
...
bool access_check_disabled = damon_has_probe_weights(ctx);
...
if (!access_check_disabled && ctx->ops.prepare_access_checks)
ctx->ops.prepare_access_checks(ctx);
...
if (ctx->ops.apply_probes) {
...
}
Would it be better to ensure ctx->ops.apply_probes is also implemented before
returning true?
[Severity: High]
This is a pre-existing issue, but since this patch enables probe weights,
can the 8-bit probe_hits counters overflow if the number of samples per
aggregation interval exceeds 255?
Looking at struct damon_region in include/linux/damon.h, probe_hits is
declared as an unsigned char array:
include/linux/damon.h:struct damon_region {
...
unsigned char probe_hits[DAMON_MAX_PROBES];
...
}
In damon_pa_apply_probes(), it is incremented during every sample interval
without checking for overflow:
mm/damon/paddr.c:damon_pa_apply_probes() {
...
damon_for_each_probe(p, ctx) {
if (damon_pa_filter_pass(pa, folio, p))
r->probe_hits[i]++;
...
}
If a user configures an aggregation interval that is > 255 times the sample
interval, could highly-hit regions wrap around to 0 and be falsely identified
as cold memory?
> + }
> return false;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=12
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight()
2026-07-06 15:16 ` sashiko-bot
@ 2026-07-07 0:05 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 0:05 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:16:52 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Setting probe weights on a DAMON context whose operations do not support `apply_probes` (e.g., `vaddr`) silently disables all monitoring.
>
> Pre-existing issues:
> - [High] The `probe_hits` array in `struct damon_region` uses `unsigned char` counters which wrap around at 256, corrupting monitoring data when the number of samples per aggregation interval exceeds 255.
> --
>
> commit 3891f362834bef45987ebcee1cdb0abcd9df6ea2
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: implement damon_has_probe_weight()
>
> DAMON monitors only data attributes when probe weights are set.
> However, the function for detecting it is intentionally returning false
> always. The intention was to make changes for complete support of the
> mode in a safe and incremental way. Now all the building blocks are
> made. Complete the mode detection function so that the mode can really
> start working.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index fa78d8d592aa3..6e5a029989a0c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -211,6 +211,12 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
> >
> > static bool damon_has_probe_weights(struct damon_ctx *c)
> > {
> > + struct damon_probe *p;
> > +
> > + damon_for_each_probe(p, c) {
> > + if (p->weight)
> > + return true;
>
> [Severity: High]
> Does returning true here silently disable monitoring if the context operations
> do not support probes (e.g., vaddr)?
>
> In kdamond_fn(), if this returns true, access_check_disabled bypasses standard
> checks. However, if ctx->ops.apply_probes is NULL, the probes are skipped:
>
> mm/damon/core.c:kdamond_fn() {
> ...
> bool access_check_disabled = damon_has_probe_weights(ctx);
> ...
> if (!access_check_disabled && ctx->ops.prepare_access_checks)
> ctx->ops.prepare_access_checks(ctx);
> ...
> if (ctx->ops.apply_probes) {
> ...
> }
>
> Would it be better to ensure ctx->ops.apply_probes is also implemented before
> returning true?
I will fix this in the next revision.
>
> [Severity: High]
> This is a pre-existing issue, but since this patch enables probe weights,
> can the 8-bit probe_hits counters overflow if the number of samples per
> aggregation interval exceeds 255?
>
> Looking at struct damon_region in include/linux/damon.h, probe_hits is
> declared as an unsigned char array:
>
> include/linux/damon.h:struct damon_region {
> ...
> unsigned char probe_hits[DAMON_MAX_PROBES];
> ...
> }
>
> In damon_pa_apply_probes(), it is incremented during every sample interval
> without checking for overflow:
>
> mm/damon/paddr.c:damon_pa_apply_probes() {
> ...
> damon_for_each_probe(p, ctx) {
> if (damon_pa_filter_pass(pa, folio, p))
> r->probe_hits[i]++;
> ...
> }
>
> If a user configures an aggregation interval that is > 255 times the sample
> interval, could highly-hit regions wrap around to 0 and be falsely identified
> as cold memory?
Known low priority issue. No blocker.
>
> > + }
> > return false;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=12
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (11 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 12/16] mm/damon/core: implement damon_has_probe_weight() SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:19 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring SJ Park
` (3 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
Implement a new sysfs file, 'weight', under probe directory. Users will
be able to set the probe weight and enable the attributes only
monitoring using it.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/sysfs.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 6d7e36c9e509d..b5fe036f78015 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1065,6 +1065,7 @@ static const struct kobj_type damon_sysfs_filters_ktype = {
struct damon_sysfs_probe {
struct kobject kobj;
+ unsigned int weight;
struct damon_sysfs_filters *filters;
};
@@ -1100,12 +1101,35 @@ static void damon_sysfs_probe_rm_dirs(struct damon_sysfs_probe *probe)
}
}
+static ssize_t weight_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct damon_sysfs_probe *probe = container_of(kobj,
+ struct damon_sysfs_probe, kobj);
+
+ return sysfs_emit(buf, "%u\n", probe->weight);
+}
+
+static ssize_t weight_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf, size_t count)
+{
+ struct damon_sysfs_probe *probe = container_of(kobj,
+ struct damon_sysfs_probe, kobj);
+ int err = kstrtouint(buf, 0, &probe->weight);
+
+ return err ? err : count;
+}
+
static void damon_sysfs_probe_release(struct kobject *kobj)
{
kfree(container_of(kobj, struct damon_sysfs_probe, kobj));
}
+static struct kobj_attribute damon_sysfs_probe_weight_attr =
+ __ATTR_RW_MODE(weight, 0600);
+
static struct attribute *damon_sysfs_probe_attrs[] = {
+ &damon_sysfs_probe_weight_attr.attr,
NULL,
};
ATTRIBUTE_GROUPS(damon_sysfs_probe);
@@ -1965,6 +1989,7 @@ static int damon_sysfs_set_probes(struct damon_ctx *ctx,
return -ENOMEM;
damon_add_probe(ctx, p);
sys_probe = sys_probes->probes_arr[i];
+ p->weight = sys_probe->weight;
err = damon_sysfs_set_probe(p, sys_probe);
if (err)
return err;
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file
2026-07-06 14:19 ` [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file SJ Park
@ 2026-07-06 15:19 ` sashiko-bot
2026-07-07 0:08 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:19 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbounded `weight` from sysfs causes an integer overflow in `damon_probe_hits_wsum()`, which corrupts the core region scoring algorithm.
- [High] Setting a large `weight` via sysfs trivially bypasses the `max_nr_regions` limit, leading to unbounded kernel memory usage and CPU starvation.
--
commit 13d5322850c993787f439a6daf8065f8717e7f80
Author: SJ Park <sj@kernel.org>
mm/damon/sysfs: implement probe/weight file
This commit implements a new sysfs file, weight, under the probe directory.
Users can set the probe weight and enable monitoring using it.
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index b65651498e0d1..a8ef7903bf94f 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
[ ... ]
> @@ -1100,12 +1101,35 @@ static void damon_sysfs_probe_rm_dirs(struct damon_sysfs_probe *probe)
[ ... ]
> +static ssize_t weight_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf, size_t count)
> +{
> + struct damon_sysfs_probe *probe = container_of(kobj,
> + struct damon_sysfs_probe, kobj);
> + int err = kstrtouint(buf, 0, &probe->weight);
[Severity: High]
Does this allow an unbounded unsigned int weight to be set, potentially
causing an integer overflow during calculation?
In damon_probe_hits_wsum(), the score is calculated by multiplying the
hits and the weight:
mm/damon/core.c:damon_probe_hits_wsum() {
...
sum += r->last_probe_hits[i++] * probe->weight;
...
}
Because weight is allowed to be up to UINT_MAX, and probe_hits can be up to
255, could this calculation wrap around? If it wraps around, regions with a
higher number of probe hits might erroneously compute a lower weighted sum
than colder regions, which seems like it would break the ability to
accurately track memory.
[Severity: High]
Could a large weight bypass the max_nr_regions limit and lead to unbounded
region growth?
In kdamond_merge_regions(), the merge loop condition checks against max_thres:
mm/damon/core.c:kdamond_merge_regions() {
...
} while (nr_regions > c->attrs.max_nr_regions &&
threshold / 2 < max_thres);
...
}
The initial threshold scales with the unbounded weight. If a user configures a
large weight, does threshold / 2 immediately exceed max_thres on the first
loop iteration?
This would cause the loop to terminate prematurely, failing to iteratively
increase the threshold to merge regions. Could this allow the number of
regions to grow without bound and lead to severe CPU starvation?
> +
> + return err ? err : count;
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=13
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file
2026-07-06 15:19 ` sashiko-bot
@ 2026-07-07 0:08 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 0:08 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:19:38 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unbounded `weight` from sysfs causes an integer overflow in `damon_probe_hits_wsum()`, which corrupts the core region scoring algorithm.
> - [High] Setting a large `weight` via sysfs trivially bypasses the `max_nr_regions` limit, leading to unbounded kernel memory usage and CPU starvation.
> --
>
> commit 13d5322850c993787f439a6daf8065f8717e7f80
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/sysfs: implement probe/weight file
>
> This commit implements a new sysfs file, weight, under the probe directory.
> Users can set the probe weight and enable monitoring using it.
>
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index b65651498e0d1..a8ef7903bf94f 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
>
> [ ... ]
>
> > @@ -1100,12 +1101,35 @@ static void damon_sysfs_probe_rm_dirs(struct damon_sysfs_probe *probe)
>
> [ ... ]
>
> > +static ssize_t weight_store(struct kobject *kobj,
> > + struct kobj_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct damon_sysfs_probe *probe = container_of(kobj,
> > + struct damon_sysfs_probe, kobj);
> > + int err = kstrtouint(buf, 0, &probe->weight);
>
> [Severity: High]
> Does this allow an unbounded unsigned int weight to be set, potentially
> causing an integer overflow during calculation?
>
> In damon_probe_hits_wsum(), the score is calculated by multiplying the
> hits and the weight:
>
> mm/damon/core.c:damon_probe_hits_wsum() {
> ...
> sum += r->last_probe_hits[i++] * probe->weight;
> ...
> }
>
> Because weight is allowed to be up to UINT_MAX, and probe_hits can be up to
> 255, could this calculation wrap around? If it wraps around, regions with a
> higher number of probe hits might erroneously compute a lower weighted sum
> than colder regions, which seems like it would break the ability to
> accurately track memory.
A minor issue for unlikely user setup. But I agree this is better to be fixed,
and the core layer parameters validation makes this easy to fix. I will work
on fixing this, in the next version of this series, or separately.
>
>
> [Severity: High]
> Could a large weight bypass the max_nr_regions limit and lead to unbounded
> region growth?
>
> In kdamond_merge_regions(), the merge loop condition checks against max_thres:
>
> mm/damon/core.c:kdamond_merge_regions() {
> ...
> } while (nr_regions > c->attrs.max_nr_regions &&
> threshold / 2 < max_thres);
> ...
> }
>
> The initial threshold scales with the unbounded weight. If a user configures a
> large weight, does threshold / 2 immediately exceed max_thres on the first
> loop iteration?
>
> This would cause the loop to terminate prematurely, failing to iteratively
> increase the threshold to merge regions. Could this allow the number of
> regions to grow without bound and lead to severe CPU starvation?
Yes, but only for one aggregation interval. I don't think this is an issue.
>
> > +
> > + return err ? err : count;
> > +}
> > +
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=13
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (12 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 15:34 ` sashiko-bot
2026-07-06 14:19 ` [RFC PATCH v1.1 15/16] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
` (2 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Liam R. Howlett, Andrew Morton, David Hildenbrand,
Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
linux-kernel, linux-mm
Update DAMON design document for the attributes only monitoring mode.
Signed-off-by: SJ Park <sj@kernel.org>
---
Documentation/mm/damon/design.rst | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 457d6e8bc7878..6d79e7d69be84 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -316,6 +316,26 @@ Another way to do this for higher accuracy is using :ref:`DAMOS filter
information in page level. But, because it is operated in page level, the
overhead is proportional to the size of the memory.
+Data Attributes-only Monitoring
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Data access is the primary monitoring information for DAMON. Hence it
+:ref:`adjusts <damon_design_data_attrs_monitoring>` regions using the access
+:ref:`counter <damon_design_region_based_sampling>` (``nr_accesses``). In some
+use cases, however, users may want some of :ref:`attributes
+<damon_design_data_attrs_monitoring>` to be the primary information.
+
+Data attributes-only monitoring mode supports this use case. For the mode,
+each attribute probe has their priority weight value. Users can describe by
+what combination of the attributes the primary information is decided, by
+setting the priority weight value. If the total sum of the weights is not
+zero, the mode is enabled. The regions adjustment mechanism uses the weighted
+sum of the :ref:`probe hit counts <damon_design_data_attrs_monitoring>` instead
+of ``nr_accesses`` in the case. When the mode is enabled, access monitoring is
+automatically turned off. The access counter (``nr_accesses``) will always be
+zero and not updated. Hence the mode is called Data Attributes "only"
+monitoring.
+
Dynamic Target Space Updates Handling
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring
2026-07-06 14:19 ` [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring SJ Park
@ 2026-07-06 15:34 ` sashiko-bot
2026-07-07 0:11 ` SJ Park
0 siblings, 1 reply; 40+ messages in thread
From: sashiko-bot @ 2026-07-06 15:34 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Multiplication and accumulation in `damon_probe_hits_wsum()` can silently overflow a 32-bit `unsigned int`.
- [High] Silently disables all monitoring if probe weights are configured for operations sets that lack probe support (e.g., `vaddr`).
Pre-existing issues:
- [High] The `abs()` macro behaves incorrectly for large `unsigned int` differences, breaking region adjustment logic.
- [High] The `probe_hits` array relies on 8-bit `unsigned char`, leading to silent wraparound during long aggregation intervals.
--
commit 61ffda736cfeed6cc995344cdd1d753a189751a8
Author: SJ Park <sj@kernel.org>
Docs/mm/damon/design: document attrs-only monitoring
This patch series introduces Attributes-only Monitoring for DAMON, adding
probe weights to dictate memory management scoring. When weights are
configured, data access monitoring is disabled and regions are evaluated
based on weighted probe hits instead.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3c5a46e1c079a..6e5a029989a0c 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -389,6 +401,30 @@ static bool damon_is_last_region(struct damon_region *r,
> return list_is_last(&r->list, &t->regions_list);
> }
>
> +/**
> + * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
> + * @r: region to get the weigted sum of.
> + * @last: if the request is for last-window agggregated probe hits.
> + * @ctx: context of &r.
> + *
> + * Return: the weighted sum of probe hits of the region.
> + */
> +unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
> + struct damon_ctx *ctx)
> +{
> + struct damon_probe *probe;
> + unsigned int sum = 0;
> + int i = 0;
> +
> + damon_for_each_probe(probe, ctx) {
> + if (last)
> + sum += r->last_probe_hits[i++] * probe->weight;
> + else
> + sum += r->probe_hits[i++] * probe->weight;
[Severity: High]
Does sum need protection against integer overflow here?
If a user configures a large priority weight for a probe via sysfs, this
multiplication can silently overflow the 32-bit unsigned int sum (for
example, 255 * 4294967295).
Could this integer overflow wrap around the score metric, making heavily
weighted regions incorrectly appear as "cold" regions and corrupting the core
memory management data?
[Severity: High]
This is a pre-existing issue, but since Attributes-only Monitoring makes
probe hits the primary metric, could probe_hits overflow its 8-bit limit?
In include/linux/damon.h, the array is defined as an 8-bit value:
struct damon_region {
...
unsigned char probe_hits[DAMON_MAX_PROBES];
...
};
If a user configures an aggregation window that encompasses more than 255
samples, the counter increment in damon_pa_apply_probes():
r->probe_hits[i]++;
will repeatedly wrap around, destroying the statistical data and corrupting
the region scoring.
> + }
> + return sum;
> +}
[ ... ]
> @@ -3211,24 +3259,37 @@ static void damon_merge_two_regions(struct damon_target *t,
> * sz_limit size upper limit of each region
> */
> static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> - unsigned long sz_limit)
> + unsigned long sz_limit, struct damon_ctx *ctx)
> {
> struct damon_region *r, *prev = NULL, *next;
> + bool use_probe_hits = damon_has_probe_weights(ctx);
>
> damon_for_each_region_safe(r, next, t) {
> - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> + unsigned int score, last_score;
> +
> + score = damon_merge_score(r, false, ctx, use_probe_hits);
> + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> +
> + if (abs(score - last_score) > thres)
[Severity: High]
This is a pre-existing issue, but it seems exacerbated now that arbitrary
weight multipliers are introduced. Does abs() behave correctly for large
unsigned int differences?
The abs() macro implicitly casts its arguments to signed int. If the
absolute difference between score and last_score exceeds INT_MAX, it
will overflow and be evaluated incorrectly as a smaller positive number.
Because weights can now easily push scores past INT_MAX, will this cause
damon_merge_regions_of() to improperly ignore large score differences?
> r->age = 0;
> - else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> + else if ((score == 0) != (last_score == 0))
> r->age = 0;
[ ... ]
> @@ -3658,25 +3718,38 @@ static int kdamond_fn(void *data)
> unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
> unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
> unsigned long sample_interval = ctx->attrs.sample_interval;
> + bool access_check_disabled = damon_has_probe_weights(ctx);
> + unsigned int max_merge_score = 0, max_wsum;
> + bool get_max_wsum;
>
> if (kdamond_wait_activation(ctx))
> break;
>
> - if (ctx->ops.prepare_access_checks)
> + if (!access_check_disabled && ctx->ops.prepare_access_checks)
> ctx->ops.prepare_access_checks(ctx);
>
> kdamond_usleep(sample_interval);
> ctx->passed_sample_intervals++;
>
> - if (ctx->ops.check_accesses)
> - max_nr_accesses = ctx->ops.check_accesses(ctx);
> - if (ctx->ops.apply_probes)
> - ctx->ops.apply_probes(ctx);
> + if (!access_check_disabled && ctx->ops.check_accesses)
> + max_merge_score = ctx->ops.check_accesses(ctx);
> + if (ctx->ops.apply_probes) {
[Severity: High]
If a user configures probe weights via sysfs for an operations set that
doesn't implement apply_probes (e.g., DAMON_OPS_VADDR), will this silently
disable all monitoring?
Because access_check_disabled becomes true unconditionally when weights are
present, default access checking is skipped. Without ctx->ops.apply_probes,
no probes are applied either, effectively breaking the DAMON monitoring thread
for that operations set.
> + if (time_after_eq(ctx->passed_sample_intervals,
> + next_aggregation_sis) &&
> + access_check_disabled)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=14
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring
2026-07-06 15:34 ` sashiko-bot
@ 2026-07-07 0:11 ` SJ Park
0 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 0:11 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Mon, 06 Jul 2026 15:34:57 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Multiplication and accumulation in `damon_probe_hits_wsum()` can silently overflow a 32-bit `unsigned int`.
> - [High] Silently disables all monitoring if probe weights are configured for operations sets that lack probe support (e.g., `vaddr`).
>
> Pre-existing issues:
> - [High] The `abs()` macro behaves incorrectly for large `unsigned int` differences, breaking region adjustment logic.
> - [High] The `probe_hits` array relies on 8-bit `unsigned char`, leading to silent wraparound during long aggregation intervals.
> --
>
> commit 61ffda736cfeed6cc995344cdd1d753a189751a8
> Author: SJ Park <sj@kernel.org>
>
> Docs/mm/damon/design: document attrs-only monitoring
>
> This patch series introduces Attributes-only Monitoring for DAMON, adding
> probe weights to dictate memory management scoring. When weights are
> configured, data access monitoring is disabled and regions are evaluated
> based on weighted probe hits instead.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3c5a46e1c079a..6e5a029989a0c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
>
> [ ... ]
>
> > @@ -389,6 +401,30 @@ static bool damon_is_last_region(struct damon_region *r,
> > return list_is_last(&r->list, &t->regions_list);
> > }
> >
> > +/**
> > + * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
> > + * @r: region to get the weigted sum of.
> > + * @last: if the request is for last-window agggregated probe hits.
> > + * @ctx: context of &r.
> > + *
> > + * Return: the weighted sum of probe hits of the region.
> > + */
> > +unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
> > + struct damon_ctx *ctx)
> > +{
> > + struct damon_probe *probe;
> > + unsigned int sum = 0;
> > + int i = 0;
> > +
> > + damon_for_each_probe(probe, ctx) {
> > + if (last)
> > + sum += r->last_probe_hits[i++] * probe->weight;
> > + else
> > + sum += r->probe_hits[i++] * probe->weight;
>
> [Severity: High]
> Does sum need protection against integer overflow here?
>
> If a user configures a large priority weight for a probe via sysfs, this
> multiplication can silently overflow the 32-bit unsigned int sum (for
> example, 255 * 4294967295).
>
> Could this integer overflow wrap around the score metric, making heavily
> weighted regions incorrectly appear as "cold" regions and corrupting the core
> memory management data?
Will fix in the next version of this series, or separately.
>
> [Severity: High]
> This is a pre-existing issue, but since Attributes-only Monitoring makes
> probe hits the primary metric, could probe_hits overflow its 8-bit limit?
>
> In include/linux/damon.h, the array is defined as an 8-bit value:
>
> struct damon_region {
> ...
> unsigned char probe_hits[DAMON_MAX_PROBES];
> ...
> };
>
> If a user configures an aggregation window that encompasses more than 255
> samples, the counter increment in damon_pa_apply_probes():
>
> r->probe_hits[i]++;
>
> will repeatedly wrap around, destroying the statistical data and corrupting
> the region scoring.
Known low priority issue. No blocker. But I will consider fixing this in the
next revision or seaprately.
>
> > + }
> > + return sum;
> > +}
>
> [ ... ]
>
> > @@ -3211,24 +3259,37 @@ static void damon_merge_two_regions(struct damon_target *t,
> > * sz_limit size upper limit of each region
> > */
> > static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> > - unsigned long sz_limit)
> > + unsigned long sz_limit, struct damon_ctx *ctx)
> > {
> > struct damon_region *r, *prev = NULL, *next;
> > + bool use_probe_hits = damon_has_probe_weights(ctx);
> >
> > damon_for_each_region_safe(r, next, t) {
> > - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> > + unsigned int score, last_score;
> > +
> > + score = damon_merge_score(r, false, ctx, use_probe_hits);
> > + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> > +
> > + if (abs(score - last_score) > thres)
>
> [Severity: High]
> This is a pre-existing issue, but it seems exacerbated now that arbitrary
> weight multipliers are introduced. Does abs() behave correctly for large
> unsigned int differences?
>
> The abs() macro implicitly casts its arguments to signed int. If the
> absolute difference between score and last_score exceeds INT_MAX, it
> will overflow and be evaluated incorrectly as a smaller positive number.
>
> Because weights can now easily push scores past INT_MAX, will this cause
> damon_merge_regions_of() to improperly ignore large score differences?
Will fix in the next revision.
>
> > r->age = 0;
> > - else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> > + else if ((score == 0) != (last_score == 0))
> > r->age = 0;
>
> [ ... ]
>
> > @@ -3658,25 +3718,38 @@ static int kdamond_fn(void *data)
> > unsigned long next_aggregation_sis = ctx->next_aggregation_sis;
> > unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
> > unsigned long sample_interval = ctx->attrs.sample_interval;
> > + bool access_check_disabled = damon_has_probe_weights(ctx);
> > + unsigned int max_merge_score = 0, max_wsum;
> > + bool get_max_wsum;
> >
> > if (kdamond_wait_activation(ctx))
> > break;
> >
> > - if (ctx->ops.prepare_access_checks)
> > + if (!access_check_disabled && ctx->ops.prepare_access_checks)
> > ctx->ops.prepare_access_checks(ctx);
> >
> > kdamond_usleep(sample_interval);
> > ctx->passed_sample_intervals++;
> >
> > - if (ctx->ops.check_accesses)
> > - max_nr_accesses = ctx->ops.check_accesses(ctx);
> > - if (ctx->ops.apply_probes)
> > - ctx->ops.apply_probes(ctx);
> > + if (!access_check_disabled && ctx->ops.check_accesses)
> > + max_merge_score = ctx->ops.check_accesses(ctx);
> > + if (ctx->ops.apply_probes) {
>
> [Severity: High]
> If a user configures probe weights via sysfs for an operations set that
> doesn't implement apply_probes (e.g., DAMON_OPS_VADDR), will this silently
> disable all monitoring?
>
> Because access_check_disabled becomes true unconditionally when weights are
> present, default access checking is skipped. Without ctx->ops.apply_probes,
> no probes are applied either, effectively breaking the DAMON monitoring thread
> for that operations set.
I will fix this in the next revision, or separately.
>
> > + if (time_after_eq(ctx->passed_sample_intervals,
> > + next_aggregation_sis) &&
> > + access_check_disabled)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=14
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH v1.1 15/16] Docs/admin-guide/mm/damon/usage: document weight sysfs file
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (13 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 14/16] Docs/mm/damon/design: document attrs-only monitoring SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-06 14:19 ` [RFC PATCH v1.1 16/16] Docs/ABI/damon: document probe weight file SJ Park
2026-07-07 0:13 ` [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Liam R. Howlett, Andrew Morton, David Hildenbrand,
Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
linux-kernel, linux-mm
Update DAMON usage document for newly added probe weight file.
Signed-off-by: SJ Park <sj@kernel.org>
---
Documentation/admin-guide/mm/damon/usage.rst | 11 ++++++++---
Documentation/mm/damon/design.rst | 5 +++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
index b2649ea011f93..da5f9afd08aef 100644
--- a/Documentation/admin-guide/mm/damon/usage.rst
+++ b/Documentation/admin-guide/mm/damon/usage.rst
@@ -73,9 +73,10 @@ comma (",").
│ │ │ │ │ │ │ intervals_goal/access_bp,aggrs,min_sample_us,max_sample_us
│ │ │ │ │ │ nr_regions/min,max
│ │ │ │ │ │ :ref:`probes <damon_usage_sysfs_probes>`/nr_probes
- │ │ │ │ │ │ │ 0/filters/nr_filters
- │ │ │ │ │ │ │ │ 0/type,matching,allow,path
- │ │ │ │ │ │ │ │ ...
+ │ │ │ │ │ │ │ 0/weight
+ │ │ │ │ │ │ │ │ filters/nr_filters
+ │ │ │ │ │ │ │ │ │ 0/type,matching,allow,path
+ │ │ │ │ │ │ │ │ │ ...
│ │ │ │ │ │ │ ...
│ │ │ │ │ :ref:`targets <sysfs_targets>`/nr_targets
│ │ │ │ │ │ :ref:`0 <sysfs_target>`/pid_target,obsolete_target
@@ -286,6 +287,10 @@ In each probe directory, one directory, ``filters`` exists. The directory
contains files for installing filters for the probe, that is used to determine
the data attribute for the probe.
+Each probe directory also contains ``weight`` file. Reading from and writing
+to the file gets and sets the :ref:`attributes-only monitoring
+<damon_design_attrs_only_monitoring>` weight for the attribute of the probe.
+
In the beginning, ``filters`` directory has only one file, ``nr_filters``.
Writing a number (``N``) to the file creates the number of child directories
named ``0`` to ``N-1``. Each directory represents each filter and works in a
diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 6d79e7d69be84..aed6cb1cf4831 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -316,6 +316,8 @@ Another way to do this for higher accuracy is using :ref:`DAMOS filter
information in page level. But, because it is operated in page level, the
overhead is proportional to the size of the memory.
+.. _damon_design_attrs_only_monitoring:
+
Data Attributes-only Monitoring
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -336,6 +338,9 @@ automatically turned off. The access counter (``nr_accesses``) will always be
zero and not updated. Hence the mode is called Data Attributes "only"
monitoring.
+Refer to the :ref:`admin guide <damon_usage_sysfs_probes>` to know how users
+can use the mode.
+
Dynamic Target Space Updates Handling
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [RFC PATCH v1.1 16/16] Docs/ABI/damon: document probe weight file
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (14 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 15/16] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
@ 2026-07-06 14:19 ` SJ Park
2026-07-07 0:13 ` [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-06 14:19 UTC (permalink / raw)
Cc: SJ Park, Liam R. Howlett, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, damon, linux-kernel, linux-mm
Update DAMON ABI document for the newly added probe weight sysfs file.
Signed-off-by: SJ Park <sj@kernel.org>
---
Documentation/ABI/testing/sysfs-kernel-mm-damon | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index dd6b5bd76e119..907a504fb64c5 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -164,6 +164,12 @@ Description: Writing a number 'N' to this file creates the number of
directories for each DAMON probe named '0' to 'N-1' under the
probes/ directory.
+What: /sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/monitoring_attrs/probes/<P>/weight
+Date: Jul 2026
+Contact: SJ Park <sj@kernel.org>
+Description: Writing to and reading from this file sets and gets the
+ per-probe attribute weight.
+
What: /sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/monitoring_attrs/probes/<P>/filters/nr_filters
Date: May 2026
Contact: SJ Park <sj@kernel.org>
--
2.47.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring
2026-07-06 14:18 [RFC PATCH v1.1 00/16] mm/damon: introduce data attributes only monitoring SJ Park
` (15 preceding siblings ...)
2026-07-06 14:19 ` [RFC PATCH v1.1 16/16] Docs/ABI/damon: document probe weight file SJ Park
@ 2026-07-07 0:13 ` SJ Park
16 siblings, 0 replies; 40+ messages in thread
From: SJ Park @ 2026-07-07 0:13 UTC (permalink / raw)
To: SJ Park
Cc: Liam R. Howlett, Andrew Morton, David Hildenbrand,
Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
linux-kernel, linux-mm
On Mon, 6 Jul 2026 07:18:54 -0700 SJ Park <sj@kernel.org> wrote:
> TL;DR: Introduce a way to get DAMON's best effort accuracy monitoring of
> user-demanding non-access data attributes.
Sashiko is continuously finding a few things including integer overflows from
wrong or weird user setups. Those are minor issues in my opinion. But because
the core layer parameters validation patch series is merged into mm-new, fixing
thosse should be straightforward.
I will fix simple ones in the way, as a part of the next series or as a
separate series.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread