* [RFC PATCH v1.2 01/19] mm/damon/core: introduce damon_probe->weight
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
` (17 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 63f596957c1f1..d8251c83e6e55 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 11473aa1f70f4..327398fa4ee56 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;
@@ -1658,6 +1659,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;
@@ -1674,6 +1676,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] 38+ messages in thread* [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 01/19] mm/damon/core: introduce damon_probe->weight SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:16 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
` (16 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 d8251c83e6e55..3c2a4d673bf62 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 327398fa4ee56..9fbea6204520b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3692,7 +3692,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] 38+ messages in thread* Re: [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address
2026-07-09 14:05 ` [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
@ 2026-07-09 14:16 ` sashiko-bot
2026-07-09 14:47 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:16 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `damon_pa_apply_probes()` callback ignores the newly added `set_samples` parameter, violating the updated API contract documented in the header.
- [Low] The use of `&set` in the kernel-doc comment will generate a warning during the documentation build.
--
--- Patch [2]: [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address ---
commit d206ad52f1017ca79add3bb060530dc8e4e317fd
Author: SJ Park <sj@kernel.org>
mm/damon/core: ask apply_probes() ops callback to set sampling address
This patch updates the apply_probes() DAMON ops callback by adding a
set_samples boolean argument. This allows the callback to optionally handle
sampling address setup to minimize unnecessary region iteration.
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index d8251c83e6e55..3c2a4d673bf62 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.
[Severity: Low]
This isn't a bug, but will this use of '&set' in include/linux/damon.h
generate a kernel-doc build warning?
The '&' prefix in kernel-doc is strictly reserved for referencing structures,
enumerations, typedefs, or unions. Using it to emphasize the verb 'set' is
invalid syntax.
[ ... ]
> 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;
[Severity: Medium]
Does damon_pa_apply_probes() in mm/damon/paddr.c need to use the new
set_samples parameter?
According to the updated API documentation, this callback should conditionally
set r->sampling_addr if set_samples is true. However, the parameter appears to
be ignored in the function body in this commit, which could lead to incorrect
or stale sampling if a future caller passes true.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address
2026-07-09 14:16 ` sashiko-bot
@ 2026-07-09 14:47 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:47 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:16:46 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `damon_pa_apply_probes()` callback ignores the newly added `set_samples` parameter, violating the updated API contract documented in the header.
> - [Low] The use of `&set` in the kernel-doc comment will generate a warning during the documentation build.
> --
>
> --- Patch [2]: [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address ---
> commit d206ad52f1017ca79add3bb060530dc8e4e317fd
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: ask apply_probes() ops callback to set sampling address
>
> This patch updates the apply_probes() DAMON ops callback by adding a
> set_samples boolean argument. This allows the callback to optionally handle
> sampling address setup to minimize unnecessary region iteration.
>
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index d8251c83e6e55..3c2a4d673bf62 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.
>
> [Severity: Low]
> This isn't a bug, but will this use of '&set' in include/linux/damon.h
> generate a kernel-doc build warning?
>
> The '&' prefix in kernel-doc is strictly reserved for referencing structures,
> enumerations, typedefs, or unions. Using it to emphasize the verb 'set' is
> invalid syntax.
Good finding. It is a typo. I will fix in the next revision.
>
> [ ... ]
>
> > 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;
>
> [Severity: Medium]
> Does damon_pa_apply_probes() in mm/damon/paddr.c need to use the new
> set_samples parameter?
>
> According to the updated API documentation, this callback should conditionally
> set r->sampling_addr if set_samples is true. However, the parameter appears to
> be ignored in the function body in this commit, which could lead to incorrect
> or stale sampling if a future caller passes true.
Yes, the later patch in this series will do that.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=2
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 01/19] mm/damon/core: introduce damon_probe->weight SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:21 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
` (15 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread* Re: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested
2026-07-09 14:05 ` [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
@ 2026-07-09 14:21 ` sashiko-bot
2026-07-09 14:48 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:21 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] Integer overflow in `r->probe_hits[]` counter when `aggr_samples` exceeds 255.
--
--- Patch [3]: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested ---
commit 81ba2fab5bb28d260739f7b8c81129066d602897
Author: SJ Park <sj@kernel.org>
mm/damon/paddr: set samples in apply_probes() if requested
This commit updates the apply_probe() callback implementation in DAMON_PADDR
so that it respects the set_samples parameter.
> 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));
[Severity: High]
This is a pre-existing issue, but does this code overflow probe_hits[]?
Looking at include/linux/damon.h, probe_hits is defined as an unsigned char
array:
struct damon_region {
...
unsigned char probe_hits[DAMON_MAX_PROBES];
...
};
Immediately following the code in this diff, damon_pa_apply_probes()
unconditionally increments the counter:
folio = damon_get_folio(PHYS_PFN(pa));
damon_for_each_probe(p, ctx) {
if (damon_pa_filter_pass(pa, folio, p))
r->probe_hits[i]++;
i++;
}
If a user configures DAMON via sysfs with an aggr_interval and sample_interval
ratio that yields more than 255 samples per aggregation interval (for example,
aggr_interval=100000us and sample_interval=100us), this counter will silently
wrap around.
Could this cause heavily accessed regions to wrap to a small number and appear
cold, potentially leading DAMOS to make incorrect memory eviction or migration
decisions?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=3
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested
2026-07-09 14:21 ` sashiko-bot
@ 2026-07-09 14:48 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:48 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:21:02 +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] Integer overflow in `r->probe_hits[]` counter when `aggr_samples` exceeds 255.
> --
>
> --- Patch [3]: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested ---
> commit 81ba2fab5bb28d260739f7b8c81129066d602897
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/paddr: set samples in apply_probes() if requested
>
> This commit updates the apply_probe() callback implementation in DAMON_PADDR
> so that it respects the set_samples parameter.
>
> > 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));
>
> [Severity: High]
> This is a pre-existing issue, but does this code overflow probe_hits[]?
>
> Looking at include/linux/damon.h, probe_hits is defined as an unsigned char
> array:
>
> struct damon_region {
> ...
> unsigned char probe_hits[DAMON_MAX_PROBES];
> ...
> };
>
> Immediately following the code in this diff, damon_pa_apply_probes()
> unconditionally increments the counter:
>
> folio = damon_get_folio(PHYS_PFN(pa));
> damon_for_each_probe(p, ctx) {
> if (damon_pa_filter_pass(pa, folio, p))
> r->probe_hits[i]++;
> i++;
> }
>
> If a user configures DAMON via sysfs with an aggr_interval and sample_interval
> ratio that yields more than 255 samples per aggregation interval (for example,
> aggr_interval=100000us and sample_interval=100us), this counter will silently
> wrap around.
>
> Could this cause heavily accessed regions to wrap to a small number and appear
> cold, potentially leading DAMOS to make incorrect memory eviction or migration
> decisions?
Known low priority issue.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=3
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (2 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:16 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 05/19] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
` (14 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 3c2a4d673bf62..4e1845ce83870 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 9fbea6204520b..9f6cdd810bd8a 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3692,7 +3692,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] 38+ messages in thread* Re: [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-09 14:05 ` [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
@ 2026-07-09 14:16 ` sashiko-bot
2026-07-09 14:49 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:16 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `damon_pa_apply_probes` callback unconditionally returns 0 instead of computing and returning the maximum probe hits weighted sum when `return_max_wsum` is true, violating the updated API contract.
--
--- Patch [4]: [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum ---
commit ed82d24ec75fdb639533062e293614cb1cebe866
Author: SJ Park <sj@kernel.org>
mm/damon/core: ask apply_probe() to return max probe hits weighted sum
This commit updates the apply_probes() callback protocol in the DAMON
operations to optionally return the maximum probe hits weighted sum. This
value is intended to help the core layer adjust regions in the future.
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 3c2a4d673bf62..4e1845ce83870 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
[ ... ]
> 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: Medium]
The kernel-doc for apply_probes states it should return the maximum probe hits
weighted sum if return_max_wsum is true.
However, damon_pa_apply_probes() unconditionally returns 0 here without
computing the weighted sum.
Will this cause the core layer to receive an incorrect threshold of 0 when it
starts passing return_max_wsum = true in the future?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
2026-07-09 14:16 ` sashiko-bot
@ 2026-07-09 14:49 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:49 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:16:38 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `damon_pa_apply_probes` callback unconditionally returns 0 instead of computing and returning the maximum probe hits weighted sum when `return_max_wsum` is true, violating the updated API contract.
> --
>
> --- Patch [4]: [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum ---
> commit ed82d24ec75fdb639533062e293614cb1cebe866
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: ask apply_probe() to return max probe hits weighted sum
>
> This commit updates the apply_probes() callback protocol in the DAMON
> operations to optionally return the maximum probe hits weighted sum. This
> value is intended to help the core layer adjust regions in the future.
>
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 3c2a4d673bf62..4e1845ce83870 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
>
> [ ... ]
>
> > 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: Medium]
> The kernel-doc for apply_probes states it should return the maximum probe hits
> weighted sum if return_max_wsum is true.
>
> However, damon_pa_apply_probes() unconditionally returns 0 here without
> computing the weighted sum.
>
> Will this cause the core layer to receive an incorrect threshold of 0 when it
> starts passing return_max_wsum = true in the future?
This is intentional. The later patch in this series will implement it.
>
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=4
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 05/19] mm/damon/core: implement damon_probe_hits_wsum()
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (3 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 06/19] mm/damon/paddr: respect return_max_wsum SJ Park
` (13 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 4e1845ce83870..516fefaf54dcc 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 9f6cdd810bd8a..fea97399b5754 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -393,6 +393,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 weighted sum of.
+ * @last: if the request is for last-window aggregated 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] 38+ messages in thread* [RFC PATCH v1.2 06/19] mm/damon/paddr: respect return_max_wsum
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (4 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 05/19] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs() SJ Park
` (12 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread* [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs()
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (5 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 06/19] mm/damon/paddr: respect return_max_wsum SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:32 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits SJ Park
` (11 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
Use of abs() in damon_merge_regions_of() could cause a silent integer
overflow since the macro casts unsigned int to signed int. It is
unlikely to have such a large value for nr_accesses. Even though it
happens, the user impact is just degraded monitoring results. Users
showing bad monitoring results for weird setup is quite trivial. But
the code is obviously wrong. Use abs_diff() instead.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260705213817.100841-1-sj@kernel.org/
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index fea97399b5754..4c7e8a12a9b67 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3261,7 +3261,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
struct damon_region *r, *prev = NULL, *next;
damon_for_each_region_safe(r, next, t) {
- if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
+ if (abs_diff(r->nr_accesses, r->last_nr_accesses) > thres)
r->age = 0;
else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
r->age = 0;
@@ -3269,7 +3269,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
r->age++;
if (prev && prev->ar.end == r->ar.start &&
- abs(prev->nr_accesses - r->nr_accesses) <= thres &&
+ abs_diff(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
--
2.47.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs()
2026-07-09 14:05 ` [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs() SJ Park
@ 2026-07-09 14:32 ` sashiko-bot
2026-07-09 14:54 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:32 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Potential integer overflow in weighted average calculations within `damon_merge_two_regions()` on 32-bit architectures.
- [High] Infinite loop in `kdamond_merge_regions()` causing soft lockup when `aggr_interval` is large.
- [High] Multiple erroneous `r->age` updates per aggregation interval during region merging.
--
--- Patch [7]: [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs() ---
commit f6ab818d018c3ba8c682c6e198e98ba7a8a69b83
Author: SJ Park <sj@kernel.org>
mm/damon/core: use abs_diff() instead of abs()
Use of abs() in damon_merge_regions_of() could cause a silent integer
overflow. This patch replaces it with abs_diff().
Link: https://lore.kernel.org/20260705213817.100841-1-sj@kernel.org/
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index fea97399b5754..4c7e8a12a9b67 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3261,7 +3261,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> struct damon_region *r, *prev = NULL, *next;
>
> damon_for_each_region_safe(r, next, t) {
> - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> + if (abs_diff(r->nr_accesses, r->last_nr_accesses) > thres)
> r->age = 0;
> else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> r->age = 0;
> @@ -3269,7 +3269,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> r->age++;
[Severity: High]
This isn't a bug introduced by this patch, but does this logic apply multiple
age updates per aggregation interval if damon_merge_regions_of() is called
multiple times?
The function damon_merge_regions_of() is called inside a do-while loop in
kdamond_merge_regions():
mm/damon/core.c:kdamond_merge_regions() {
...
do {
nr_regions = 0;
damon_for_each_target(t, c) {
damon_merge_regions_of(t, threshold, sz_limit);
nr_regions += damon_nr_regions(t);
}
threshold = max(1, threshold * 2);
} while (nr_regions > c->attrs.max_nr_regions &&
threshold / 2 < max_thres);
...
}
Since last_nr_accesses isn't updated until after this loop completes, if
merging fails to reduce nr_regions sufficiently, the loop repeats. Could this
cause the same access counts to be evaluated repeatedly, artificially aging or
resetting memory multiple times within a single aggregation interval?
[Severity: High]
This is a pre-existing issue, but can the loop in kdamond_merge_regions() run
infinitely if max_thres is extremely large?
Looking at the same snippet above, max_thres is an unsigned int calculated
from aggr_interval. If max_thres is >= 2,147,483,648 (for example, due to a
large aggr_interval), doubling threshold will eventually overflow a 32-bit
unsigned int to 0.
Then max(1, 0) resets it to 1. Since threshold / 2 (which is 0) will always
be less than max_thres, could this cause a soft lockup of the kdamond thread?
>
> if (prev && prev->ar.end == r->ar.start &&
> - abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> + abs_diff(prev->nr_accesses, r->nr_accesses) <= thres &&
> damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
> damon_merge_two_regions(t, prev, r);
[Severity: High]
This is a pre-existing issue, but can the size-weighted average calculations
in damon_merge_two_regions() overflow on 32-bit architectures?
mm/damon/core.c:damon_merge_two_regions() {
...
unsigned long sz_l = damon_sz_region(l), sz_r = damon_sz_region(r);
l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
(sz_l + sz_r);
l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
...
}
If sz_l is large (e.g., 100 MB or ~104,857,600 bytes) and nr_accesses or age
is moderate (e.g., 50), their product (~5.24 billion) exceeds the maximum
32-bit unsigned integer value (~4.29 billion).
Since unsigned long is 32 bits on those systems, could this silently overflow
and irreversibly corrupt the merged region's statistics?
> else
> prev = r;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=7
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs()
2026-07-09 14:32 ` sashiko-bot
@ 2026-07-09 14:54 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:54 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:32:09 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Potential integer overflow in weighted average calculations within `damon_merge_two_regions()` on 32-bit architectures.
> - [High] Infinite loop in `kdamond_merge_regions()` causing soft lockup when `aggr_interval` is large.
> - [High] Multiple erroneous `r->age` updates per aggregation interval during region merging.
> --
>
> --- Patch [7]: [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs() ---
> commit f6ab818d018c3ba8c682c6e198e98ba7a8a69b83
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: use abs_diff() instead of abs()
>
> Use of abs() in damon_merge_regions_of() could cause a silent integer
> overflow. This patch replaces it with abs_diff().
>
> Link: https://lore.kernel.org/20260705213817.100841-1-sj@kernel.org/
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index fea97399b5754..4c7e8a12a9b67 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3261,7 +3261,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> > struct damon_region *r, *prev = NULL, *next;
> >
> > damon_for_each_region_safe(r, next, t) {
> > - if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
> > + if (abs_diff(r->nr_accesses, r->last_nr_accesses) > thres)
> > r->age = 0;
> > else if ((r->nr_accesses == 0) != (r->last_nr_accesses == 0))
> > r->age = 0;
> > @@ -3269,7 +3269,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> > r->age++;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this logic apply multiple
> age updates per aggregation interval if damon_merge_regions_of() is called
> multiple times?
>
> The function damon_merge_regions_of() is called inside a do-while loop in
> kdamond_merge_regions():
>
> mm/damon/core.c:kdamond_merge_regions() {
> ...
> do {
> nr_regions = 0;
> damon_for_each_target(t, c) {
> damon_merge_regions_of(t, threshold, sz_limit);
> nr_regions += damon_nr_regions(t);
> }
> threshold = max(1, threshold * 2);
> } while (nr_regions > c->attrs.max_nr_regions &&
> threshold / 2 < max_thres);
> ...
> }
>
> Since last_nr_accesses isn't updated until after this loop completes, if
> merging fails to reduce nr_regions sufficiently, the loop repeats. Could this
> cause the same access counts to be evaluated repeatedly, artificially aging or
> resetting memory multiple times within a single aggregation interval?
Sounds making sense. Apparently not fit to be fixed in this series, though. I
will further check and separately work on this.
>
> [Severity: High]
> This is a pre-existing issue, but can the loop in kdamond_merge_regions() run
> infinitely if max_thres is extremely large?
>
> Looking at the same snippet above, max_thres is an unsigned int calculated
> from aggr_interval. If max_thres is >= 2,147,483,648 (for example, due to a
> large aggr_interval), doubling threshold will eventually overflow a 32-bit
> unsigned int to 0.
>
> Then max(1, 0) resets it to 1. Since threshold / 2 (which is 0) will always
> be less than max_thres, could this cause a soft lockup of the kdamond thread?
>
> >
> > if (prev && prev->ar.end == r->ar.start &&
> > - abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> > + abs_diff(prev->nr_accesses, r->nr_accesses) <= thres &&
> > damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
> > damon_merge_two_regions(t, prev, r);
Seems a good finding. Not related to this series, as Sashiko also says. I
will further check separatelt work on this.
>
> [Severity: High]
> This is a pre-existing issue, but can the size-weighted average calculations
> in damon_merge_two_regions() overflow on 32-bit architectures?
>
> mm/damon/core.c:damon_merge_two_regions() {
> ...
> unsigned long sz_l = damon_sz_region(l), sz_r = damon_sz_region(r);
>
> l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
> (sz_l + sz_r);
> l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
> ...
> }
>
> If sz_l is large (e.g., 100 MB or ~104,857,600 bytes) and nr_accesses or age
> is moderate (e.g., 50), their product (~5.24 billion) exceeds the maximum
> 32-bit unsigned integer value (~4.29 billion).
>
> Since unsigned long is 32 bits on those systems, could this silently overflow
> and irreversibly corrupt the merged region's statistics?
Known low priority issue.
>
> > else
> > prev = r;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=7
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (6 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 07/19] mm/damon/core: use abs_diff() instead of abs() SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:32 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 09/19] mm/damon/core: disallow probe_hits overflow on attrs only monitoring SJ Park
` (10 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 | 49 +++++++++++++++++++++++++++++--------
mm/damon/tests/core-kunit.h | 13 ++++++++--
2 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 4c7e8a12a9b67..7d49420ea26c4 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.
@@ -3248,6 +3253,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
*
@@ -3256,24 +3271,38 @@ 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_diff(r->nr_accesses, r->last_nr_accesses) > thres)
+ unsigned int score, last_score, diff;
+
+ score = damon_merge_score(r, false, ctx, use_probe_hits);
+ last_score = damon_merge_score(r, true, ctx, use_probe_hits);
+
+ if (abs_diff(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_diff(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;
+ diff = abs_diff(score, damon_merge_score(prev, false, ctx,
+ use_probe_hits));
+ if (diff > 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;
}
}
@@ -3306,7 +3335,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 d97280270c27b..6ad73559dd8ea 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] 38+ messages in thread* Re: [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits
2026-07-09 14:05 ` [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits SJ Park
@ 2026-07-09 14:32 ` sashiko-bot
2026-07-09 14:56 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:32 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] Region `age` is incorrectly updated multiple times per aggregation interval and evaluated against a stale `last_score` when the `kdamond_merge_regions` retry loop is triggered.
--
--- Patch [8]: [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits ---
commit 938ea22fc6c1fec3c909339503398aa70ea047c8
Author: SJ Park <sj@kernel.org>
mm/damon/core: extend merge function to work with probe hits
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.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3256,24 +3271,38 @@ 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_diff(r->nr_accesses, r->last_nr_accesses) > thres)
> + unsigned int score, last_score, diff;
> +
> + score = damon_merge_score(r, false, ctx, use_probe_hits);
> + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> +
> + if (abs_diff(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++;
[Severity: High]
This is a pre-existing issue, but does this logic in damon_merge_regions_of()
incorrectly increment or reset r->age multiple times per aggregation interval?
When the number of regions exceeds max_nr_regions, kdamond_merge_regions()
repeatedly calls damon_merge_regions_of() with doubled thresholds. During
these retries, r->age is incremented multiple times within a single
aggregation interval.
Furthermore, because damon_merge_two_regions() updates the current
nr_accesses and probe_hits but fails to merge their historical counterparts
(last_nr_accesses and last_probe_hits), these values become stale.
On subsequent iterations of the retry loop, damon_merge_score() computes
last_score using these stale unmerged values. This can cause
abs_diff(score, last_score) to spuriously exceed the threshold, resetting
r->age to 0 inappropriately.
Could this cause DAMOS age-based schemes to trigger prematurely or fail to
trigger entirely?
> - if (prev && prev->ar.end == r->ar.start &&
> - abs_diff(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;
> + diff = abs_diff(score, damon_merge_score(prev, false, ctx,
> + use_probe_hits));
> + if (diff > 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;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=8
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits
2026-07-09 14:32 ` sashiko-bot
@ 2026-07-09 14:56 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:56 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:32:15 +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] Region `age` is incorrectly updated multiple times per aggregation interval and evaluated against a stale `last_score` when the `kdamond_merge_regions` retry loop is triggered.
> --
>
> --- Patch [8]: [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits ---
> commit 938ea22fc6c1fec3c909339503398aa70ea047c8
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: extend merge function to work with probe hits
>
> 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.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
>
> [ ... ]
>
> > @@ -3256,24 +3271,38 @@ 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_diff(r->nr_accesses, r->last_nr_accesses) > thres)
> > + unsigned int score, last_score, diff;
> > +
> > + score = damon_merge_score(r, false, ctx, use_probe_hits);
> > + last_score = damon_merge_score(r, true, ctx, use_probe_hits);
> > +
> > + if (abs_diff(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++;
>
> [Severity: High]
> This is a pre-existing issue, but does this logic in damon_merge_regions_of()
> incorrectly increment or reset r->age multiple times per aggregation interval?
>
> When the number of regions exceeds max_nr_regions, kdamond_merge_regions()
> repeatedly calls damon_merge_regions_of() with doubled thresholds. During
> these retries, r->age is incremented multiple times within a single
> aggregation interval.
>
> Furthermore, because damon_merge_two_regions() updates the current
> nr_accesses and probe_hits but fails to merge their historical counterparts
> (last_nr_accesses and last_probe_hits), these values become stale.
>
> On subsequent iterations of the retry loop, damon_merge_score() computes
> last_score using these stale unmerged values. This can cause
> abs_diff(score, last_score) to spuriously exceed the threshold, resetting
> r->age to 0 inappropriately.
>
> Could this cause DAMOS age-based schemes to trigger prematurely or fail to
> trigger entirely?
I already promised to separately work on this.
>
> > - if (prev && prev->ar.end == r->ar.start &&
> > - abs_diff(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;
> > + diff = abs_diff(score, damon_merge_score(prev, false, ctx,
> > + use_probe_hits));
> > + if (diff > 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;
> > }
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=8
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 09/19] mm/damon/core: disallow probe_hits overflow on attrs only monitoring
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (7 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 08/19] mm/damon/core: extend merge function to work with probe hits SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow SJ Park
` (9 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
When any damon_probe->weight is set, DAMON will do only probe
monitoring. probe_hits is 'unsigned char'. It could overflow when the
aggregation interval is larger than the sampling interval times 256.
damon_as_probe_weights() always return false, so such overflow cannot
happen. Even if it happens, it only degrades the monitoring results.
That said, the overflow is not intentional. It is better to be
prevented as long as the cost is not expensive. Disallow the overflow
by adding a validation logic on the core layer parameters validation
function.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7d49420ea26c4..4f1425e56950b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1332,6 +1332,19 @@ static void damos_set_filters_default_reject(struct damos *s)
damos_filters_default_reject(&s->ops_filters);
}
+static bool damon_valid_probe_params(struct damon_ctx *ctx)
+{
+ unsigned long sample_interval;
+
+ if (!damon_has_probe_weights(ctx))
+ return true;
+
+ sample_interval = ctx->attrs.sample_interval ? : 1;
+ if (ctx->attrs.aggr_interval / sample_interval > U8_MAX)
+ return false;
+ return true;
+}
+
/*
* damos_commit_dests() - Copy migration destinations from @src to @dst.
* @dst: Destination structure to update.
@@ -1736,6 +1749,9 @@ static int __damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
}
}
+ if (!damon_valid_probe_params(src))
+ return -EINVAL;
+
err = damon_commit_schemes(dst, src);
if (err)
return err;
--
2.47.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (8 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 09/19] mm/damon/core: disallow probe_hits overflow on attrs only monitoring SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:36 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 11/19] mm/damon/core: disable access monitoring when probe weights are set SJ Park
` (8 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
damon_probe_hits_wsum() could overflow in weird setups. Users could set
the weight unreasonably high. They could also set the aggregation
interval unreasonably high compared to the sampling interval. Such user
setup is unlikely. Even if such setup is used,
damon_has_probe_weights() always returns false, so the overflow cannot
happen. The function may be completed in future, though. Even if the
overflow happens, the consequence is degraded monitoring results for the
unreasonable setup. It is just a trivial user experience issue.
It is still better to be prevented unless the cost is expensive. Avoid
the overflow by adding the parameter validation in the core layer
parameters validation function.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 4f1425e56950b..b585e4bf035f1 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1335,6 +1335,9 @@ static void damos_set_filters_default_reject(struct damos *s)
static bool damon_valid_probe_params(struct damon_ctx *ctx)
{
unsigned long sample_interval;
+ unsigned char max_probe_hits;
+ struct damon_probe *probe;
+ unsigned int wsum, wsum_to_add;
if (!damon_has_probe_weights(ctx))
return true;
@@ -1342,6 +1345,18 @@ static bool damon_valid_probe_params(struct damon_ctx *ctx)
sample_interval = ctx->attrs.sample_interval ? : 1;
if (ctx->attrs.aggr_interval / sample_interval > U8_MAX)
return false;
+
+ /* invlaid if probe hits weighted sum can overflow */
+ max_probe_hits = damon_nr_samples_per_aggr(&ctx->attrs);
+ wsum = 0;
+ damon_for_each_probe(probe, ctx) {
+ if (probe->weight > UINT_MAX / max_probe_hits)
+ return false;
+ wsum_to_add = probe->weight * max_probe_hits;
+ if (UINT_MAX - wsum < wsum_to_add)
+ return false;
+ wsum += wsum_to_add;
+ }
return true;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow
2026-07-09 14:05 ` [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow SJ Park
@ 2026-07-09 14:36 ` sashiko-bot
2026-07-09 14:58 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:36 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Spelling mistake in comment.
--
--- Patch [10]: [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow ---
commit 3e76c3dde55500288f3eccb730acd26964bfdad2
Author: SJ Park <sj@kernel.org>
mm/damon/core: validate params for probe hits weighted sum overflow
This commit adds parameter validation to prevent probe hits weighted sum
overflow. It ensures that the combination of probe weights and aggregation
intervals does not exceed the maximum allowed values.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 4f1425e56950b..b585e4bf035f1 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -1342,6 +1345,18 @@ static bool damon_valid_probe_params(struct damon_ctx *ctx)
> sample_interval = ctx->attrs.sample_interval ? : 1;
> if (ctx->attrs.aggr_interval / sample_interval > U8_MAX)
> return false;
> +
> + /* invlaid if probe hits weighted sum can overflow */
[Severity: Low]
This isn't a bug, but there is a typo in this comment ("invlaid" instead of
"invalid").
> + max_probe_hits = damon_nr_samples_per_aggr(&ctx->attrs);
> + wsum = 0;
> + damon_for_each_probe(probe, ctx) {
> + if (probe->weight > UINT_MAX / max_probe_hits)
> + return false;
> + wsum_to_add = probe->weight * max_probe_hits;
> + if (UINT_MAX - wsum < wsum_to_add)
> + return false;
> + wsum += wsum_to_add;
> + }
> return true;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=10
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow
2026-07-09 14:36 ` sashiko-bot
@ 2026-07-09 14:58 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:58 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:36:51 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Spelling mistake in comment.
> --
>
> --- Patch [10]: [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow ---
> commit 3e76c3dde55500288f3eccb730acd26964bfdad2
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: validate params for probe hits weighted sum overflow
>
> This commit adds parameter validation to prevent probe hits weighted sum
> overflow. It ensures that the combination of probe weights and aggregation
> intervals does not exceed the maximum allowed values.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 4f1425e56950b..b585e4bf035f1 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -1342,6 +1345,18 @@ static bool damon_valid_probe_params(struct damon_ctx *ctx)
> > sample_interval = ctx->attrs.sample_interval ? : 1;
> > if (ctx->attrs.aggr_interval / sample_interval > U8_MAX)
> > return false;
> > +
> > + /* invlaid if probe hits weighted sum can overflow */
>
> [Severity: Low]
> This isn't a bug, but there is a typo in this comment ("invlaid" instead of
> "invalid").
Good finding. I will fix in the next revision.
>
> > + max_probe_hits = damon_nr_samples_per_aggr(&ctx->attrs);
> > + wsum = 0;
> > + damon_for_each_probe(probe, ctx) {
> > + if (probe->weight > UINT_MAX / max_probe_hits)
> > + return false;
> > + wsum_to_add = probe->weight * max_probe_hits;
> > + if (UINT_MAX - wsum < wsum_to_add)
> > + return false;
> > + wsum += wsum_to_add;
> > + }
> > return true;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=10
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 11/19] mm/damon/core: disable access monitoring when probe weights are set
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (9 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 10/19] mm/damon/core: validate params for probe hits weighted sum overflow SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if " SJ Park
` (7 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 b585e4bf035f1..e250b7080708d 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3763,17 +3763,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] 38+ messages in thread* [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (10 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 11/19] mm/damon/core: disable access monitoring when probe weights are set SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:48 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 13/19] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
` (6 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 e250b7080708d..f1be748e9a674 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3777,7 +3777,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] 38+ messages in thread* Re: [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-09 14:05 ` [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if " SJ Park
@ 2026-07-09 14:48 ` sashiko-bot
2026-07-09 15:00 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:48 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] When access checks are disabled, `max_nr_accesses` is not updated but is still used for the region merge threshold. `apply_probes()` is incorrectly called without requesting or capturing the maximum weighted sum.
- [Medium] Setting probe weights on a context whose operations do not implement `.apply_probes` (e.g., `vaddr`) silently disables all monitoring.
--
--- Patch [12]: [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if probe weights are set ---
commit ffce494dcbbb1125634a81b14be56b2f89aefe7e
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 e250b7080708d..f1be748e9a674 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3777,7 +3777,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: Medium]
When access checks are disabled, check_accesses() is bypassed and
max_nr_accesses is not updated.
Later in this function, kdamond_merge_regions() uses max_nr_accesses to
calculate the merge threshold:
if (time_after_eq(ctx->passed_sample_intervals,
next_aggregation_sis)) {
kdamond_merge_regions(ctx,
max_nr_accesses / 10,
sz_limit);
Since apply_probes() is called with return_max_wsum=false, it doesn't
update or return a max weighted sum. Will kdamond_merge_regions() use an
invalid or stale threshold (often 0), breaking the merge algorithm?
[Severity: Medium]
If a user sets probe weights on a context whose operations do not implement
.apply_probes (e.g., vaddr), access_check_disabled will be true and
check_accesses() will be bypassed.
Since .apply_probes is not implemented, this block is also skipped.
Could this completely disable all monitoring for the target with no error
feedback to the user?
Validation in damon_valid_probe_params() successfully validates this state:
mm/damon/core.c:damon_valid_probe_params() {
...
if (!damon_has_probe_weights(ctx))
return true;
...
}
Should damon_valid_probe_params() check if ctx->ops.apply_probes is supported?
>
> if (time_after_eq(ctx->passed_sample_intervals,
> next_aggregation_sis)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=12
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if probe weights are set
2026-07-09 14:48 ` sashiko-bot
@ 2026-07-09 15:00 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 15:00 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:48:55 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] When access checks are disabled, `max_nr_accesses` is not updated but is still used for the region merge threshold. `apply_probes()` is incorrectly called without requesting or capturing the maximum weighted sum.
> - [Medium] Setting probe weights on a context whose operations do not implement `.apply_probes` (e.g., `vaddr`) silently disables all monitoring.
> --
>
> --- Patch [12]: [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if probe weights are set ---
> commit ffce494dcbbb1125634a81b14be56b2f89aefe7e
> 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 e250b7080708d..f1be748e9a674 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3777,7 +3777,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: Medium]
> When access checks are disabled, check_accesses() is bypassed and
> max_nr_accesses is not updated.
>
> Later in this function, kdamond_merge_regions() uses max_nr_accesses to
> calculate the merge threshold:
>
> if (time_after_eq(ctx->passed_sample_intervals,
> next_aggregation_sis)) {
> kdamond_merge_regions(ctx,
> max_nr_accesses / 10,
> sz_limit);
>
> Since apply_probes() is called with return_max_wsum=false, it doesn't
> update or return a max weighted sum. Will kdamond_merge_regions() use an
> invalid or stale threshold (often 0), breaking the merge algorithm?
That will be implemented by a later patch of this series.
>
> [Severity: Medium]
> If a user sets probe weights on a context whose operations do not implement
> .apply_probes (e.g., vaddr), access_check_disabled will be true and
> check_accesses() will be bypassed.
>
> Since .apply_probes is not implemented, this block is also skipped.
>
> Could this completely disable all monitoring for the target with no error
> feedback to the user?
>
> Validation in damon_valid_probe_params() successfully validates this state:
>
> mm/damon/core.c:damon_valid_probe_params() {
> ...
> if (!damon_has_probe_weights(ctx))
> return true;
> ...
> }
>
> Should damon_valid_probe_params() check if ctx->ops.apply_probes is supported?
This is an intentional design. The user should config DAMON correctly.
>
> >
> > if (time_after_eq(ctx->passed_sample_intervals,
> > next_aggregation_sis)) {
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=12
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 13/19] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn()
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (11 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 12/19] mm/damon/core: set samples in apply_probes() if " SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
` (5 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 f1be748e9a674..8837b2129baa6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3730,7 +3730,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);
@@ -3764,6 +3763,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;
@@ -3775,7 +3775,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);
@@ -3783,7 +3783,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] 38+ messages in thread* [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (12 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 13/19] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:49 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() SJ Park
` (4 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 8837b2129baa6..12c66edecd783 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3763,7 +3763,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;
@@ -3776,9 +3777,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] 38+ messages in thread* Re: [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-09 14:05 ` [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
@ 2026-07-09 14:49 ` sashiko-bot
2026-07-09 15:02 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:49 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing validation for `apply_probes` callback when probe weights are configured leads to complete DAMON region collapse.
--
--- Patch [14]: [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set ---
commit a4d0e22d5d4d741ec6086d8f49fefbab2c3b57bd
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 8837b2129baa6..12c66edecd783 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3776,9 +3777,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;
> + }
[Severity: High]
If a configuration has probe weights enabled (so access_check_disabled is
true) but the active operations do not implement the apply_probes callback
(like vaddr or fvaddr), does max_merge_score remain 0 here?
It appears that skipping both check_accesses() and apply_probes() leaves
max_merge_score at 0. Later, when kdamond_merge_regions() is called with a
threshold of 0, and all regions have 0 probe hits, damon_merge_regions_of()
would see a difference of 0 between adjacent regions.
Could this lead to aggressive merging of all regions up to sz_limit,
effectively collapsing the fine-grained region layout?
Should there be a validation check in damon_valid_probe_params() to reject
configurations where probe weights are set but ctx->ops.apply_probes is
missing?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=14
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set
2026-07-09 14:49 ` sashiko-bot
@ 2026-07-09 15:02 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 15:02 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:49:01 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing validation for `apply_probes` callback when probe weights are configured leads to complete DAMON region collapse.
> --
>
> --- Patch [14]: [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set ---
> commit a4d0e22d5d4d741ec6086d8f49fefbab2c3b57bd
> 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 8837b2129baa6..12c66edecd783 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
>
> [ ... ]
>
> > @@ -3776,9 +3777,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;
> > + }
>
> [Severity: High]
> If a configuration has probe weights enabled (so access_check_disabled is
> true) but the active operations do not implement the apply_probes callback
> (like vaddr or fvaddr), does max_merge_score remain 0 here?
>
> It appears that skipping both check_accesses() and apply_probes() leaves
> max_merge_score at 0. Later, when kdamond_merge_regions() is called with a
> threshold of 0, and all regions have 0 probe hits, damon_merge_regions_of()
> would see a difference of 0 between adjacent regions.
>
> Could this lead to aggressive merging of all regions up to sz_limit,
> effectively collapsing the fine-grained region layout?
>
> Should there be a validation check in damon_valid_probe_params() to reject
> configurations where probe weights are set but ctx->ops.apply_probes is
> missing?
No, this is an intentional design. The user should correctly config DAMON.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=14
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight()
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (13 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 14/19] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:56 ` sashiko-bot
2026-07-09 14:05 ` [RFC PATCH v1.2 16/19] mm/damon/sysfs: implement probe/weight file SJ Park
` (3 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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 12c66edecd783..6661d4bfab5b9 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] 38+ messages in thread* Re: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight()
2026-07-09 14:05 ` [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() SJ Park
@ 2026-07-09 14:56 ` sashiko-bot
2026-07-09 15:03 ` SJ Park
0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-09 14:56 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Enabling probe weights for DAMON operations that do not implement `apply_probes` (e.g., VADDR and FVADDR) silently disables memory monitoring and region aggregation.
--
--- Patch [15]: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() ---
commit 13bb6c9724d49615512e0e4f353c5d02210d2c11
Author: SJ Park <sj@kernel.org>
mm/damon/core: implement damon_has_probe_weight()
This commit completes the implementation of damon_has_probe_weights()
to return true if any probe has a non-zero weight. This allows DAMON
to detect probe weights and use them for monitoring.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 12c66edecd783..6661d4bfab5b9 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 this silently break monitoring for operation sets that do not implement
apply_probes?
When this function returns true, access_check_disabled evaluates to true in
kdamond_fn():
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 (!access_check_disabled && ctx->ops.check_accesses)
max_merge_score = ctx->ops.check_accesses(ctx);
if (ctx->ops.apply_probes) {
...
}
...
}
If a user configures probe weights for operation sets that lack apply_probes,
all access checks are skipped. Since apply_probes is NULL, no memory access
information is gathered at all. This causes regions to endlessly report 0
accesses and incorrectly merge up to the maximum limit.
Should __damon_commit_ctx() or damon_valid_probe_params() validate that
ctx->ops.apply_probes is non-NULL when probe weights are used, returning
-EINVAL to prevent this failure?
> return false;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=15
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight()
2026-07-09 14:56 ` sashiko-bot
@ 2026-07-09 15:03 ` SJ Park
0 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 15:03 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 09 Jul 2026 14:56:52 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Enabling probe weights for DAMON operations that do not implement `apply_probes` (e.g., VADDR and FVADDR) silently disables memory monitoring and region aggregation.
> --
>
> --- Patch [15]: [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() ---
> commit 13bb6c9724d49615512e0e4f353c5d02210d2c11
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: implement damon_has_probe_weight()
>
> This commit completes the implementation of damon_has_probe_weights()
> to return true if any probe has a non-zero weight. This allows DAMON
> to detect probe weights and use them for monitoring.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 12c66edecd783..6661d4bfab5b9 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 this silently break monitoring for operation sets that do not implement
> apply_probes?
>
> When this function returns true, access_check_disabled evaluates to true in
> kdamond_fn():
>
> 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 (!access_check_disabled && ctx->ops.check_accesses)
> max_merge_score = ctx->ops.check_accesses(ctx);
>
> if (ctx->ops.apply_probes) {
> ...
> }
> ...
> }
>
> If a user configures probe weights for operation sets that lack apply_probes,
> all access checks are skipped. Since apply_probes is NULL, no memory access
> information is gathered at all. This causes regions to endlessly report 0
> accesses and incorrectly merge up to the maximum limit.
>
> Should __damon_commit_ctx() or damon_valid_probe_params() validate that
> ctx->ops.apply_probes is non-NULL when probe weights are used, returning
> -EINVAL to prevent this failure?
This is an intentional design. Users should correctly set DAMON.
>
> > return false;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709140600.90950-1-sj@kernel.org?part=15
Thanks,
SJ
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v1.2 16/19] mm/damon/sysfs: implement probe/weight file
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (14 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 15/19] mm/damon/core: implement damon_has_probe_weight() SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 17/19] Docs/mm/damon/design: document attrs-only monitoring SJ Park
` (2 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread* [RFC PATCH v1.2 17/19] Docs/mm/damon/design: document attrs-only monitoring
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (15 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 16/19] mm/damon/sysfs: implement probe/weight file SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 18/19] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 19/19] Docs/ABI/damon: document probe weight file SJ Park
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread* [RFC PATCH v1.2 18/19] Docs/admin-guide/mm/damon/usage: document weight sysfs file
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (16 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 17/19] Docs/mm/damon/design: document attrs-only monitoring SJ Park
@ 2026-07-09 14:05 ` SJ Park
2026-07-09 14:05 ` [RFC PATCH v1.2 19/19] Docs/ABI/damon: document probe weight file SJ Park
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread* [RFC PATCH v1.2 19/19] Docs/ABI/damon: document probe weight file
2026-07-09 14:05 [RFC PATCH v1.2 00/19] mm/damon: introduce data attributes only monitoring SJ Park
` (17 preceding siblings ...)
2026-07-09 14:05 ` [RFC PATCH v1.2 18/19] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
@ 2026-07-09 14:05 ` SJ Park
18 siblings, 0 replies; 38+ messages in thread
From: SJ Park @ 2026-07-09 14:05 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] 38+ messages in thread