[PATCH v2 0/3] docs/mm/damon/design: add explanation of nr_snapshots

[PATCH v2 0/3] docs/mm/damon/design: add explanation of nr_snapshots

[Liew Rui Yan]

Add an explanation of nr_snapshots to avoid misunderstandings.

Changes from v1:
- v1: https://lore.kernel.org/damon/20260811101841.804596-1-aethernet65535@gmail.com
- Patch 1 word order adjustment: Repositioned "completely" ("tried to be
  completely applied" -> "completely tried to be applied").
- Patch 2: Use two spaces between sentences.
- Patch 3: Add the "Reviewed-by:" of SJ, because he added this in v1.
- Patch 3: Make some minor modifications, Patch 3 can be applied after
  Patch 2.

Liew Rui Yan (3):
  docs/mm/damon/design: accurate semantics of nr_snapshots
  docs/mm/damon/design: difference between watermarks and nr_snapshots
  docs/mm/damon/design: fix typo of max_nr_snapshots

 Documentation/mm/damon/design.rst | 7 ++++---
 include/linux/damon.h             | 5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots

[Liew Rui Yan]

Change "tried to be applied" -> "completely tried to be applied" to
maintain consistency between the documentation and the code.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 Documentation/mm/damon/design.rst | 4 ++--
 include/linux/damon.h             | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index aed6cb1cf483..1739aeec6eb9 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -846,8 +846,8 @@ scheme's execution.
 - ``nr_applied``: Total number of regions that the scheme is applied.
 - ``sz_applied``: Total size of regions that the scheme is applied.
 - ``qt_exceeds``: Total number of times the quota of the scheme has exceeded.
-- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is tried to
-  be applied.
+- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is
+  completely tried to be applied.
 - ``max_nr_snapshots``: Upper limit of ``nr_snapshots``.
 
 "A scheme is tried to be applied to a region" means DAMOS core logic determined
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 0c8b7ddef9ab..33914594fb48 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -357,7 +357,8 @@ struct damos_watermarks {
  *		Total bytes that passed ops layer-handled DAMOS filters.
  * @qt_exceeds: Total number of times the quota of the scheme has exceeded.
  * @nr_snapshots:
- *		Total number of DAMON snapshots that the scheme has tried.
+ *		Total number of DAMON snapshots that the scheme is completely
+ *		tried to be applied.
  *
  * "Tried an action to a region" in this context means the DAMOS core logic
  * determined the region as eligible to apply the action.  The access pattern
-- 
2.55.0

Re: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots

[sashiko-bot]

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

Pre-existing issues:
- [High] The commit updates documentation to match buggy code behavior rather than fixing the underlying logical flaws in the `nr_snapshots` counter. `nr_snapshots` incorrectly increments per target instead of per snapshot, and skips entirely when quotas are full.
--

--- Patch [1]: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots ---
commit 11b4a9c760b881c063ff5e1eec0248b087d6a31f
Author: Liew Rui Yan <aethernet65535@gmail.com>

docs/mm/damon/design: accurate semantics of nr_snapshots

This commit updates the DAMON design documentation and header comments
to align the description of nr_snapshots with the current behavior
implemented in the code.

> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> index aed6cb1cf4831..1739aeec6eb95 100644
> --- a/Documentation/mm/damon/design.rst
> +++ b/Documentation/mm/damon/design.rst
> @@ -846,8 +846,8 @@ scheme's execution.
[ ... ]
>  - ``qt_exceeds``: Total number of times the quota of the scheme has exceeded.
> -- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is tried to
> -  be applied.
> +- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is
> +  completely tried to be applied.
>  - ``max_nr_snapshots``: Upper limit of ``nr_snapshots``.

[Severity: High]
This is a pre-existing issue, but does updating the documentation to match
the code behavior paper over an underlying logical flaw in the nr_snapshots
counter?

Looking at damon_do_apply_schemes() in mm/damon/core.c, the counter updates
like this:

mm/damon/core.c:damon_do_apply_schemes() {
    ...
        /* Check the quota */
        if (damos_quota_is_full(quota, c->min_region_sz))
            continue;
    ...
        if (damon_is_last_region(r, t))
            s->stat.nr_snapshots++;
    ...
}

Because this increments at the last region of a target, if multiple targets
are present, does the counter incorrectly increment per target instead of
per snapshot?

Additionally, if a restrictive quota fills up before the last region of a
target is processed, the early continue skips the counter increment entirely.
This could mean a scheme configured with a max_nr_snapshots limit might
never reach its limit.

Should the underlying code be fixed to increment once per snapshot regardless
of quotas and targets, rather than updating the documentation to reflect this
behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812104002.15225-1-aethernet65535@gmail.com?part=1

Re: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots

[SJ Park]

On Wed, 12 Aug 2026 10:47:19 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The commit updates documentation to match buggy code behavior rather than fixing the underlying logical flaws in the `nr_snapshots` counter. `nr_snapshots` incorrectly increments per target instead of per snapshot, and skips entirely when quotas are full.
> --
> 
> --- Patch [1]: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots ---
> commit 11b4a9c760b881c063ff5e1eec0248b087d6a31f
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> docs/mm/damon/design: accurate semantics of nr_snapshots
> 
> This commit updates the DAMON design documentation and header comments
> to align the description of nr_snapshots with the current behavior
> implemented in the code.
> 
> > diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> > index aed6cb1cf4831..1739aeec6eb95 100644
> > --- a/Documentation/mm/damon/design.rst
> > +++ b/Documentation/mm/damon/design.rst
> > @@ -846,8 +846,8 @@ scheme's execution.
> [ ... ]
> >  - ``qt_exceeds``: Total number of times the quota of the scheme has exceeded.
> > -- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is tried to
> > -  be applied.
> > +- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is
> > +  completely tried to be applied.
> >  - ``max_nr_snapshots``: Upper limit of ``nr_snapshots``.
> 
> [Severity: High]
> This is a pre-existing issue, but does updating the documentation to match
> the code behavior paper over an underlying logical flaw in the nr_snapshots
> counter?
> 
> Looking at damon_do_apply_schemes() in mm/damon/core.c, the counter updates
> like this:
> 
> mm/damon/core.c:damon_do_apply_schemes() {
>     ...
>         /* Check the quota */
>         if (damos_quota_is_full(quota, c->min_region_sz))
>             continue;
>     ...
>         if (damon_is_last_region(r, t))
>             s->stat.nr_snapshots++;
>     ...
> }
> 
> Because this increments at the last region of a target, if multiple targets
> are present, does the counter incorrectly increment per target instead of
> per snapshot?
> 
> Additionally, if a restrictive quota fills up before the last region of a
> target is processed, the early continue skips the counter increment entirely.
> This could mean a scheme configured with a max_nr_snapshots limit might
> never reach its limit.
> 
> Should the underlying code be fixed to increment once per snapshot regardless
> of quotas and targets, rather than updating the documentation to reflect this
> behavior?

I show no problem at the current behavior.  Let's keep it.  If it really
confuses some real human, we could add clarifications to documents.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812104002.15225-1-aethernet65535@gmail.com?part=1


Thanks,
SJ

[PATCH v2 2/3] docs/mm/damon/design: difference between watermarks and nr_snapshots

[Liew Rui Yan]

Explain the difference between nr_snapshots reaches max_nr_snapshots and
watermarks.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 Documentation/mm/damon/design.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 939298f2a9b9..1fcf0b8c044b 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -872,7 +872,8 @@ the action to the region will fail.
 
 Unlike normal stats, ``max_nr_snapshots`` is set by users.  If it is set as
 non-zero and ``nr_snapshots`` be same to or greater than ``nr_snapshots``, the
-scheme is deactivated.
+scheme is deactivated.  Note that, unlike watermarks, even if a scheme's
+``nr_snapshots`` reaches ``max_nr_snapshots``, monitoring will not stop.
 
 To know how user-space can read the stats via :ref:`DAMON sysfs interface
 <sysfs_interface>`, refer to :ref:s`stats <sysfs_stats>` part of the
-- 
2.55.0

Re: [PATCH v2 2/3] docs/mm/damon/design: difference between watermarks and nr_snapshots

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but `nr_snapshots` is incorrectly incremented per target instead of per snapshot, and skips incrementing entirely if the scheme bypasses the final region of a target.
--

--- Patch [2]: [PATCH v2 2/3] docs/mm/damon/design: difference between watermarks and nr_snapshots ---
commit af16039fca7bbda3290262311f6015d553f6c65f
Author: Liew Rui Yan <aethernet65535@gmail.com>

docs/mm/damon/design: difference between watermarks and nr_snapshots

Explain the difference between nr_snapshots reaches max_nr_snapshots and
watermarks.

> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> index 1739aeec6eb95..e7977f005ac06 100644
> --- a/Documentation/mm/damon/design.rst
> +++ b/Documentation/mm/damon/design.rst
> @@ -872,7 +872,8 @@ the action to the region will fail.
>  
>  Unlike normal stats, ``max_nr_snapshots`` is set by users.  If it is set as
>  non-zero and ``nr_snapshots`` be same to or greater than ``nr_snapshots``, the
> -scheme is deactivated.
> +scheme is deactivated.  Note that, unlike watermarks, even if a scheme's
> +``nr_snapshots`` reaches ``max_nr_snapshots``, monitoring will not stop.

[Severity: High]
This isn't a bug introduced by this patch, but while documenting the
nr_snapshots behavior, I noticed an issue in how nr_snapshots is incremented
in the core code.

Looking at damon_do_apply_schemes() in mm/damon/core.c, nr_snapshots is
incremented per target instead of per snapshot:

		if (damos_quota_is_full(quota, c->min_region_sz))
			continue;

		if (damos_skip_charged_region(t, r, s, c->min_region_sz))
			continue;
...
		if (damon_is_last_region(r, t))
			s->stat.nr_snapshots++;

Could this cause the scheme to incorrectly increment multiple times per
snapshot if there are multiple monitoring targets?

Also, if damos_quota_is_full() or damos_skip_charged_region() triggers a
continue on the target's last region, does this bypass the increment entirely?

If a scheme frequently exhausts its quota, it seems nr_snapshots might never
increment, causing the scheme to run forever without reaching
max_nr_snapshots.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812104002.15225-1-aethernet65535@gmail.com?part=2

Re: [PATCH v2 2/3] docs/mm/damon/design: difference between watermarks and nr_snapshots

[SJ Park]

On Wed, 12 Aug 2026 10:47:25 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but `nr_snapshots` is incorrectly incremented per target instead of per snapshot, and skips incrementing entirely if the scheme bypasses the final region of a target.
> --
> 
> --- Patch [2]: [PATCH v2 2/3] docs/mm/damon/design: difference between watermarks and nr_snapshots ---
> commit af16039fca7bbda3290262311f6015d553f6c65f
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> docs/mm/damon/design: difference between watermarks and nr_snapshots
> 
> Explain the difference between nr_snapshots reaches max_nr_snapshots and
> watermarks.
> 
> > diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> > index 1739aeec6eb95..e7977f005ac06 100644
> > --- a/Documentation/mm/damon/design.rst
> > +++ b/Documentation/mm/damon/design.rst
> > @@ -872,7 +872,8 @@ the action to the region will fail.
> >  
> >  Unlike normal stats, ``max_nr_snapshots`` is set by users.  If it is set as
> >  non-zero and ``nr_snapshots`` be same to or greater than ``nr_snapshots``, the
> > -scheme is deactivated.
> > +scheme is deactivated.  Note that, unlike watermarks, even if a scheme's
> > +``nr_snapshots`` reaches ``max_nr_snapshots``, monitoring will not stop.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while documenting the
> nr_snapshots behavior, I noticed an issue in how nr_snapshots is incremented
> in the core code.
> 
> Looking at damon_do_apply_schemes() in mm/damon/core.c, nr_snapshots is
> incremented per target instead of per snapshot:
> 
> 		if (damos_quota_is_full(quota, c->min_region_sz))
> 			continue;
> 
> 		if (damos_skip_charged_region(t, r, s, c->min_region_sz))
> 			continue;
> ...
> 		if (damon_is_last_region(r, t))
> 			s->stat.nr_snapshots++;
> 
> Could this cause the scheme to incorrectly increment multiple times per
> snapshot if there are multiple monitoring targets?
> 
> Also, if damos_quota_is_full() or damos_skip_charged_region() triggers a
> continue on the target's last region, does this bypass the increment entirely?
> 
> If a scheme frequently exhausts its quota, it seems nr_snapshots might never
> increment, causing the scheme to run forever without reaching
> max_nr_snapshots.

I don't find a problem of the current behavior.  Let me know if I'm missing
something.  Let's keep it as-is otherwise.  We could add clarifications to
documents if this really confuses humans.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812104002.15225-1-aethernet65535@gmail.com?part=2


Thanks,
SJ

[PATCH v2 3/3] docs/mm/damon/design: fix typo of max_nr_snapshots

[Liew Rui Yan]

Fixes a typo (nr_snapshots -> max_nr_snapshots) and corrects a grammar
error.

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
---
 Documentation/mm/damon/design.rst | 4 ++--
 include/linux/damon.h             | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index e7977f005ac0..d1dd9050ebf4 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -871,8 +871,8 @@ action is ``pageout`` while all pages of the region are unreclaimable, applying
 the action to the region will fail.
 
 Unlike normal stats, ``max_nr_snapshots`` is set by users.  If it is set as
-non-zero and ``nr_snapshots`` be same to or greater than ``nr_snapshots``, the
-scheme is deactivated.  Note that, unlike watermarks, even if a scheme's
+non-zero and ``nr_snapshots`` equals or is greater than ``max_nr_snapshots``,
+the scheme is deactivated.  Note that, unlike watermarks, even if a scheme's
 ``nr_snapshots`` reaches ``max_nr_snapshots``, monitoring will not stop.
 
 To know how user-space can read the stats via :ref:`DAMON sysfs interface
diff --git a/include/linux/damon.h b/include/linux/damon.h
index cbdf5f77978e..4b0d2d2e4ea4 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -549,7 +549,7 @@ struct damos_migrate_dests {
  *
  * After applying the &action to each region, &stat is updated.
  *
- * If &max_nr_snapshots is set as non-zero and &stat.nr_snapshots be same to or
+ * If &max_nr_snapshots is set as non-zero and &stat.nr_snapshots equals or is
  * greater than it, the scheme is deactivated.
  */
 struct damos {
-- 
2.55.0

Re: [PATCH v2 3/3] docs/mm/damon/design: fix typo of max_nr_snapshots

[sashiko-bot]

> Fixes a typo (nr_snapshots -> max_nr_snapshots) and corrects a grammar
> error.
> 
> Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812104002.15225-1-aethernet65535@gmail.com?part=3

Re: [PATCH v2 0/3] docs/mm/damon/design: add explanation of nr_snapshots

[SJ Park]

On Wed, 12 Aug 2026 18:39:59 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Add an explanation of nr_snapshots to avoid misunderstandings.

Thank you for this series.

For the series:

Reviewed-by: SJ Park <sj@kernel.org>

I applied  this series to damon/next [1] tree.  We are now quite close to next
merge window.  We (mm community) want to focus on making mm.git more stabilized
and therefore ready for the next merge window, rather than adding more changes
that are not really urgent.  I understand this series is not really urgent.
Hence, Andrew might not add this patch until next -rc1 release.  In the case, I
will request adding this to mm.git after next -rc1 release.  So, no action from
your side is needed for now.  Let me know if you think this is really  urgent.

[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]