DAMON generates monitoring results snapshots for every sampling
interval.  DAMOS applies given schemes on the regions of the snapshots,
for every apply interval of the scheme.

DAMOS stat informs a given scheme has tried to how many memory entities
and applied, in the region and byte level.  In some use cases including
user-space oriented tuning and investigations, it is useful to know that
in the DAMON-snapshot level.  Introduce a new stat, namely nr_snapshots
for DAMON core API callers.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h |  3 +++
 mm/damon/core.c       | 13 ++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 3813373a9200..1d8a1515e75a 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -330,6 +330,8 @@ struct damos_watermarks {
  * @sz_ops_filter_passed:
  *		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.
  *
  * "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
@@ -355,6 +357,7 @@ struct damos_stat {
 	unsigned long sz_applied;
 	unsigned long sz_ops_filter_passed;
 	unsigned long qt_exceeds;
+	unsigned long nr_snapshots;
 };
 
 /**
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 53fb988b6a0d..6f3328b29a65 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -157,6 +157,12 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
 	damon_free_region(r);
 }
 
+static bool damon_is_last_region(struct damon_region *r,
+		struct damon_target *t)
+{
+	return list_is_last(&t->regions_list, &r->list);
+}
+
 /*
  * Check whether a region is intersecting an address range
  *
@@ -1949,10 +1955,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (damos_skip_charged_region(t, &r, s, c->min_sz_region))
 			continue;
 
-		if (!damos_valid_target(c, t, r, s))
-			continue;
+		if (damos_valid_target(c, t, r, s))
+			damos_apply_scheme(c, t, r, s);
 
-		damos_apply_scheme(c, t, r, s);
+		if (damon_is_last_region(r, t))
+			s->stat.nr_snapshots++;
 	}
 }
 
-- 
2.47.3

On Tue, 16 Dec 2025 00:01:14 -0800 SeongJae Park <sj@kernel.org> wrote:

> DAMON generates monitoring results snapshots for every sampling
> interval.  DAMOS applies given schemes on the regions of the snapshots,
> for every apply interval of the scheme.
> 
> DAMOS stat informs a given scheme has tried to how many memory entities
> and applied, in the region and byte level.  In some use cases including
> user-space oriented tuning and investigations, it is useful to know that
> in the DAMON-snapshot level.  Introduce a new stat, namely nr_snapshots
> for DAMON core API callers.

Hi everyone,

I ran mm-new through the AI review prompts and this one was flagged:

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -157,6 +157,12 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
>  	damon_free_region(r);
>  }
>
> +static bool damon_is_last_region(struct damon_region *r,
> +		struct damon_target *t)
> +{
> +	return list_is_last(&t->regions_list, &r->list);
                           ^^^^^^^^^^^^^^^^  ^^^^^^^^

Are the arguments to list_is_last() swapped here? The kernel's
list_is_last() expects (entry, head) but this passes (head, entry).

Looking at include/linux/list.h:

    static inline int list_is_last(const struct list_head *list,
                                   const struct list_head *head)
    {
        return list->next == head;
    }

The current code checks if t->regions_list.next == &r->list, which will
be true when r is the first region in the list, not the last.

Compare with the existing damon_last_region() in include/linux/damon.h
which correctly uses &t->regions_list as the head:

    static inline struct damon_region *damon_last_region(struct damon_target *t)
    {
        return list_last_entry(&t->regions_list, struct damon_region, list);
    }

Should this be:

    return list_is_last(&r->list, &t->regions_list);

> +}
> +

[ ... ]

> @@ -1978,10 +1984,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,

[ ... ]

> +		if (damon_is_last_region(r, t))
> +			s->stat.nr_snapshots++;

If the list_is_last() arguments are indeed swapped, this will increment
nr_snapshots when processing the first region of each target rather than
the last, causing the stat to be counted once per target instead of once
per complete snapshot.

>  	}
>  }

On Wed, 14 Jan 2026 04:29:56 -0800 Chris Mason <clm@meta.com> wrote:

> On Tue, 16 Dec 2025 00:01:14 -0800 SeongJae Park <sj@kernel.org> wrote:
> 
> > DAMON generates monitoring results snapshots for every sampling
> > interval.  DAMOS applies given schemes on the regions of the snapshots,
> > for every apply interval of the scheme.
> > 
> > DAMOS stat informs a given scheme has tried to how many memory entities
> > and applied, in the region and byte level.  In some use cases including
> > user-space oriented tuning and investigations, it is useful to know that
> > in the DAMON-snapshot level.  Introduce a new stat, namely nr_snapshots
> > for DAMON core API callers.
> 
> Hi everyone,
> 
> I ran mm-new through the AI review prompts and this one was flagged:

Thank you Chris, this is a very helpful and great gift for the community!

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -157,6 +157,12 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
> >  	damon_free_region(r);
> >  }
> >
> > +static bool damon_is_last_region(struct damon_region *r,
> > +		struct damon_target *t)
> > +{
> > +	return list_is_last(&t->regions_list, &r->list);
>                            ^^^^^^^^^^^^^^^^  ^^^^^^^^
> 
> Are the arguments to list_is_last() swapped here? The kernel's
> list_is_last() expects (entry, head) but this passes (head, entry).
> 
> Looking at include/linux/list.h:
> 
>     static inline int list_is_last(const struct list_head *list,
>                                    const struct list_head *head)
>     {
>         return list->next == head;
>     }
> 
> The current code checks if t->regions_list.next == &r->list, which will
> be true when r is the first region in the list, not the last.
> 
> Compare with the existing damon_last_region() in include/linux/damon.h
> which correctly uses &t->regions_list as the head:
> 
>     static inline struct damon_region *damon_last_region(struct damon_target *t)
>     {
>         return list_last_entry(&t->regions_list, struct damon_region, list);
>     }
> 
> Should this be:
> 
>     return list_is_last(&r->list, &t->regions_list);
> 
> > +}
> > +
> 
> [ ... ]
> 
> > @@ -1978,10 +1984,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> 
> [ ... ]
> 
> > +		if (damon_is_last_region(r, t))
> > +			s->stat.nr_snapshots++;
> 
> If the list_is_last() arguments are indeed swapped, this will increment
> nr_snapshots when processing the first region of each target rather than
> the last, causing the stat to be counted once per target instead of once
> per complete snapshot.

AI is all correct.  Note that this is not making user-visible issues, though it
is clearly a bug that better to be fixed.  I explain the reason in below fixup
patch.

Andrew, the commit to fix is in mm-unstable.  Could you please squash below
attaching fixup to the commit?


Thanks,
SJ

[...]
=== >8 ===
From 551b89fd05f0c40c824d2a5c0ca6c2a63766fbab Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Wed, 14 Jan 2026 06:57:38 -0800
Subject: [PATCH] mm/damon/core: fix wrong list_is_last() call in
 damons_is_last_region()

damon_is_last_region() is calling list_is_last() with swapped arguments.
As a result, it works like damon_is_first_region().  Not that this is
not making user-visible impact, because the function is only used to
increment nr_snapshots stat once per DAMOS' regions iteration, and the
stat is exposed users only after the iteration is completed.  That's why
it passed a user space test.  Nonetheless, the bug is a bug and could
cause real user issues in future.  Fix it by correcting the order of
arguments for list_lis_last().

Reported-by: Chris Mason <clm@meta.com>
Closes: https://lore.kernel.org/20260114122959.1164957-1-clm@meta.com
Fixes: bbb3a45f1923 ("mm/damon/core: introduce nr_snapshots damos stat") # mm-unstable
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 385900733a0b..5d7e58cbd9fe 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -190,7 +190,7 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
 static bool damon_is_last_region(struct damon_region *r,
 		struct damon_target *t)
 {
-	return list_is_last(&t->regions_list, &r->list);
+	return list_is_last(&r->list, &t->regions_list);
 }
 
 /*
-- 
2.47.3