[RFC PATCH 0/2] mm/damon: fix nr_snapshots semantics and max_nr_snapshots check coverage

[RFC PATCH 0/2] mm/damon: fix nr_snapshots semantics and max_nr_snapshots check coverage

[Liew Rui Yan]

Both patches address the DAMOS nr_snapshots/max_nr_snapshots
deactivation mechanism.

- Patch 1: fixes the nr_snapshots counting semantics.
- Patch 2: unifies deactivation checks to prevent busy-loop.

Note: Patch 1 should be applied before Patch 2, as damos_is_deactivated()
relies on correct nr_snapshots counting.

Liew Rui Yan (2):
  mm/damon: fix nr_snapshots counting using tried_applied flag
  mm/damon: skip deactivated schemes in watermark checks and add
    fallback sleep

 include/linux/damon.h |  2 ++
 mm/damon/core.c       | 42 ++++++++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.55.0

[PATCH 1/2] mm/damon: fix nr_snapshots counting using tried_applied flag

[Liew Rui Yan]

Currently, nr_snapshots is only incremented when damon_is_last_region()
returns true. This is semantically incorrect because nr_snapshots should
count the number of DAMON snapshots (aggregation intervals) where the
scheme was tried to be applied, as documented in design.rst.

The "tried to be applied" means the scheme passed all guard checks
including access pattern, quotas, watermarks, and max_nr_snapshots
filters. The old damon_is_last_region() check does not accurately
reflect this semantic.

To fix this, add a tried_applied flag to 'struct damos' that is set
whenever a scheme passes all guard checks and is considered for
application in damon_do_apply_schemes(). Then, increment nr_snapshots
based on this flag in kdamond_apply_schemes() after the walk completes.
This ensures nr_snapshots accurately reflects the number of snapshots
where the scheme was actually tried.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 include/linux/damon.h |  2 ++
 mm/damon/core.c       | 12 ++++--------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 0c8b7ddef9ab..01e899a2150c 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -573,6 +573,8 @@ struct damos {
 	unsigned long next_apply_sis;
 	/* informs if ongoing DAMOS walk for this scheme is finished */
 	bool walk_completed;
+	/* informs if damos is tried applied in this phase */
+	bool tried_applied;
 	/*
 	 * If the current region in the filtering stage is allowed by core
 	 * layer-handled filters.  If true, operations layer allows it, too.
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..7230483e771f 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -398,12 +398,6 @@ static void damon_destroy_region(struct damon_region *r,
 	damon_free_region(r);
 }
 
-static bool damon_is_last_region(struct damon_region *r,
-		struct damon_target *t)
-{
-	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.
@@ -2668,8 +2662,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (damos_valid_target(c, r, s))
 			damos_apply_scheme(c, t, r, s);
 
-		if (damon_is_last_region(r, t))
-			s->stat.nr_snapshots++;
+		s->tried_applied = true;
 	}
 }
 
@@ -3249,6 +3242,9 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 	damon_for_each_scheme(s, c) {
 		if (time_before(c->passed_sample_intervals, s->next_apply_sis))
 			continue;
+		if (s->tried_applied)
+			s->stat.nr_snapshots++;
+		s->tried_applied = false;
 		damos_walk_complete(c, s);
 		damos_set_next_apply_sis(s, c);
 		s->last_applied = NULL;
-- 
2.55.0

[PATCH 2/2] mm/damon: skip deactivated schemes in watermark checks and add fallback sleep

[Liew Rui Yan]

According to DAMOS design documentation, a scheme is deactivated when
nr_snapshots reaches max_nr_snapshots. However, the previous
kdamond_wait_activation() still checked the schemes which nr_snapshots
reached max_nr_snapshots.

This caused an issue - when all schemes were deactivated due to
max_nr_snapshots, but their watermarks were still satisfied,
damos_wmark_wait_us() would return 0, causing kdamond_wait_activation()
to return 0 (activated). The main loop would then continue without
sleeping, leading to unnecessary overhead since all schemes would be
skipped in damon_do_apply_schemes().

To fix this:
- Add a damos_is_deactivated() helper that checks both wmarks.activated
  and max_nr_snapshots conditions.
- Replace the wmarks.activated-only checks with damos_is_deactivated()
  in both damon_do_apply_schemes() and kdamond_apply_schemes().
- In kdamond_wait_activation(), skip deactivated schemes when calculating
  the minimum wait time.
- When no active schemes remain, sleep for sample_interval as a fallback
  (consistent with pause behavior) instead of returning 0.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 mm/damon/core.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7230483e771f..57d1a21a8a0a 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2633,6 +2633,17 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
 	damos_update_stat(s, sz, sz_applied, sz_ops_filter_passed);
 }
 
+static bool damos_is_deactivated(struct damos *s)
+{
+	if (!s->wmarks.activated)
+		return true;
+	if (s->max_nr_snapshots &&
+			s->max_nr_snapshots <= s->stat.nr_snapshots)
+		return true;
+
+	return false;
+}
+
 static void damon_do_apply_schemes(struct damon_ctx *c,
 				   struct damon_target *t,
 				   struct damon_region *r)
@@ -2645,7 +2656,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (time_before(c->passed_sample_intervals, s->next_apply_sis))
 			continue;
 
-		if (!s->wmarks.activated)
+		if (damos_is_deactivated(s))
 			continue;
 
 		/* Check the quota */
@@ -2655,10 +2666,6 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (damos_skip_charged_region(t, r, s, c->min_region_sz))
 			continue;
 
-		if (s->max_nr_snapshots &&
-				s->max_nr_snapshots <= s->stat.nr_snapshots)
-			continue;
-
 		if (damos_valid_target(c, r, s))
 			damos_apply_scheme(c, t, r, s);
 
@@ -3220,7 +3227,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		if (time_before(c->passed_sample_intervals, s->next_apply_sis))
 			continue;
 
-		if (!s->wmarks.activated)
+		if (damos_is_deactivated(s))
 			continue;
 
 		has_schemes_to_apply = true;
@@ -3704,16 +3711,23 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
 
 	while (!kdamond_need_stop(ctx)) {
 		damon_for_each_scheme(s, ctx) {
+			if (s->max_nr_snapshots &&
+				s->max_nr_snapshots <= s->stat.nr_snapshots)
+				continue;
+
 			wait_time = damos_wmark_wait_us(s);
 			if (!init_wait_time || wait_time < min_wait_time) {
 				init_wait_time = true;
 				min_wait_time = wait_time;
 			}
 		}
-		if (!min_wait_time)
+		if (!min_wait_time && init_wait_time)
 			return 0;
 
-		kdamond_usleep(min_wait_time);
+		if (min_wait_time)
+			kdamond_usleep(min_wait_time);
+		else
+			kdamond_usleep(ctx->attrs.sample_interval);
 
 		kdamond_call(ctx, false);
 		if (ctx->maybe_corrupted)
-- 
2.55.0

Re: [PATCH 1/2] mm/damon: fix nr_snapshots counting using tried_applied flag

[SJ Park]

On Fri,  7 Aug 2026 17:35:25 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Currently, nr_snapshots is only incremented when damon_is_last_region()
> returns true. This is semantically incorrect because nr_snapshots should
> count the number of DAMON snapshots (aggregation intervals) where the
> scheme was tried to be applied, as documented in design.rst.

I don't think the current implementation is wrong.  Maybe the documentation is
better to be updated to "...that the scheme has __completely__ tried".  Let me
know if I'm missing something, or this implementaion has some practical
problems.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon: skip deactivated schemes in watermark checks and add fallback sleep

[SJ Park]

Hello Liew,

On Fri,  7 Aug 2026 17:35:26 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> According to DAMOS design documentation, a scheme is deactivated when
> nr_snapshots reaches max_nr_snapshots. However, the previous
> kdamond_wait_activation() still checked the schemes which nr_snapshots
> reached max_nr_snapshots.
> 
> This caused an issue - when all schemes were deactivated due to
> max_nr_snapshots, but their watermarks were still satisfied,
> damos_wmark_wait_us() would return 0, causing kdamond_wait_activation()
> to return 0 (activated). The main loop would then continue without
> sleeping, leading to unnecessary overhead since all schemes would be
> skipped in damon_do_apply_schemes().

This is an intended implementation.

When all schemes are deactivated by watermarks, DAMON stops monitoring.  It was
implemented in the way because we didn't want DAMON consumes system resource in
the case.  But, later it turned out DAMON's resource consumption is really
negligible.  Rather, it is turned out that it makes DAMON runs with stale
history when it is activated again.  Particularly, regions have 'age' and their
start/end addresses that was emerged before the deactivation.  Those are
meaningless and could even cause wrong DAMOS decisions.

We don't want that anymore.  For a case the user really want DAMON completely
stops, we introduced 'pause'.  For max_nr_snapshot-based deactivation, we don't
intend to completely stop DAMON.

Maybe the documentation can be updated to further clarify this.


Thanks,
SJ

[...]

Re: [RFC PATCH 0/2] mm/damon: fix nr_snapshots semantics and max_nr_snapshots check coverage

[SJ Park]

Hello Liew,

On Fri,  7 Aug 2026 17:35:24 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Both patches address the DAMOS nr_snapshots/max_nr_snapshots
> deactivation mechanism.
> 
> - Patch 1: fixes the nr_snapshots counting semantics.
> - Patch 2: unifies deactivation checks to prevent busy-loop.

Thank you for sharing this patch series.  However, I don't think the patches
are fixing real problems.  Rather, it seems the poor documentation has confused
you.  Sorry for writing the documentation in the confusing way.  I commented to
each patch for details.  If you want, please feel free to send documentation
update patches.  I will also try to find a time to do that if nobody does.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon: skip deactivated schemes in watermark checks and add fallback sleep

[Liew Rui Yan]

Hi SJ,

On Fri,  7 Aug 2026 07:07:39 -0700 SJ Park <sj@kernel.org> wrote:

> This is an intended implementation.
> 
> When all schemes are deactivated by watermarks, DAMON stops monitoring.  It was
> implemented in the way because we didn't want DAMON consumes system resource in
> the case.  But, later it turned out DAMON's resource consumption is really
> negligible.  Rather, it is turned out that it makes DAMON runs with stale
> history when it is activated again.  Particularly, regions have 'age' and their
> start/end addresses that was emerged before the deactivation.  Those are
> meaningless and could even cause wrong DAMOS decisions.
> 
> We don't want that anymore.  For a case the user really want DAMON completely
> stops, we introduced 'pause'.  For max_nr_snapshot-based deactivation, we don't
> intend to completely stop DAMON.
> 
> Maybe the documentation can be updated to further clarify this.

Thank you for your detailed explanation! I also glad to hear that
DAMON's monitoring's resource consumption is negligible, since I plan to
add a contexts/<N>/always_monitoring parameter later on, allowing users
to decide for themselves whether to keep monitoring even all schemes is
deactivated by watermarks.

Best regards,
Rui Yan

Re: [RFC PATCH 0/2] mm/damon: fix nr_snapshots semantics and max_nr_snapshots check coverage

[Liew Rui Yan]

Hi SJ,

On Fri,  7 Aug 2026 07:12:13 -0700 SJ Park <sj@kernel.org> wrote:

> Thank you for sharing this patch series.  However, I don't think the patches
> are fixing real problems.  Rather, it seems the poor documentation has confused
> you.  Sorry for writing the documentation in the confusing way.  I commented to
> each patch for details.  If you want, please feel free to send documentation
> update patches.  I will also try to find a time to do that if nobody does.

I will update those documentations! Thank you again for your detailed
explanations.

Re: [RFC PATCH 0/2] mm/damon: fix nr_snapshots semantics and max_nr_snapshots check coverage

[SJ Park]

On Sat,  8 Aug 2026 00:19:00 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Hi SJ,
> 
> On Fri,  7 Aug 2026 07:12:13 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > Thank you for sharing this patch series.  However, I don't think the patches
> > are fixing real problems.  Rather, it seems the poor documentation has confused
> > you.  Sorry for writing the documentation in the confusing way.  I commented to
> > each patch for details.  If you want, please feel free to send documentation
> > update patches.  I will also try to find a time to do that if nobody does.
> 
> I will update those documentations! Thank you again for your detailed
> explanations.

Thank you, looking forward to it!


Thanks,
SJ

Re: [PATCH 2/2] mm/damon: skip deactivated schemes in watermark checks and add fallback sleep

[SJ Park]

On Sat,  8 Aug 2026 00:18:40 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Hi SJ,
> 
> On Fri,  7 Aug 2026 07:07:39 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > This is an intended implementation.
> > 
> > When all schemes are deactivated by watermarks, DAMON stops monitoring.  It was
> > implemented in the way because we didn't want DAMON consumes system resource in
> > the case.  But, later it turned out DAMON's resource consumption is really
> > negligible.  Rather, it is turned out that it makes DAMON runs with stale
> > history when it is activated again.  Particularly, regions have 'age' and their
> > start/end addresses that was emerged before the deactivation.  Those are
> > meaningless and could even cause wrong DAMOS decisions.
> > 
> > We don't want that anymore.  For a case the user really want DAMON completely
> > stops, we introduced 'pause'.  For max_nr_snapshot-based deactivation, we don't
> > intend to completely stop DAMON.
> > 
> > Maybe the documentation can be updated to further clarify this.
> 
> Thank you for your detailed explanation! I also glad to hear that
> DAMON's monitoring's resource consumption is negligible, since I plan to
> add a contexts/<N>/always_monitoring parameter later on, allowing users
> to decide for themselves whether to keep monitoring even all schemes is
> deactivated by watermarks.

Unless it is for your real use case, I'd not encourage improving watermarks.

Here are contexts.  Watermarks was initially developed for DAMON_RECLAIM.  But
I found it is difficult to be extended for other use cases.  Also it has the
aobve mentioned problem: losing the monitoring results that was converged over
time.

I'm planning to develop an alternative superior feature and deprecate
watermarks over time.  The plan is not having a high priority because I don't
show real requirement of that.  If anyone wants it for their real use case,
please let me know so that I can prioritize.


Thanks,
SJ

[...]