[PATCH v2 0/2] mm/damon: Add damos_stat support for vaddr

[PATCH v2 0/2] mm/damon: Add damos_stat support for vaddr

[Yueyang Pan]

Previously damos_stat only supoort paddr. This patch set adds support 
for damos_stat for vaddr. Also all different types of filters are 
supported. 

Functionality Test
==================
I wrote a small test program which allocates 10GB of DRAM, use 
madvise(MADV_HUGEPAGE) to convert the base pages to 2MB huge pages
Then my program does the following things in order:
1. Write sequentially to the whole 10GB region
2. Read the first 5GB region sequentially for 10 times
3. Sleep 5s
4. Read the second 5GB region sequentially for 10 times

With a proper damon setting, we are expected to see df-passed to be 10GB
and hot region move around with the read

$ # Start damon record
$sudo ./damo/damo start "./my_test/test" --monitoring_intervals 100ms\
1s 2s

$ # damon report
$sudo ./damo/damo report access --snapshot_damos_filter allow \
hugepage_size 2MiB 2MiB
    heatmap:
    # min/max temperatures: -600,000,000, 100,001,000, column size: 137.352 MiB
    intervals: sample 100 ms aggr 1 s (max access hz 10)
    # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
    df-pass:
    # min/max temperatures: -400,000,000, 100,001,000, column size: 128.031 MiB
    0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 6 s           df-passed 0 B
    1   addr 127.608 TiB  size 877.664 MiB access 3.000 hz age 0 ns          df-passed 878.000 MiB
    2   addr 127.609 TiB  size 219.418 MiB access 2.000 hz age 0 ns          df-passed 220.000 MiB
    3   addr 127.609 TiB  size 316.613 MiB access 1.000 hz age 1 s           df-passed 316.000 MiB
    4   addr 127.609 TiB  size 474.922 MiB access 1.000 hz age 1 s           df-passed 476.000 MiB
    5   addr 127.610 TiB  size 407.188 MiB access 1.000 hz age 0 ns          df-passed 406.000 MiB
    6   addr 127.610 TiB  size 610.781 MiB access 1.000 hz age 0 ns          df-passed 612.000 MiB
    7   addr 127.611 TiB  size 697.309 MiB access 0 hz   age 0 ns          df-passed 696.000 MiB
    8   addr 127.611 TiB  size 77.480 MiB  access 1.000 hz age 0 ns          df-passed 78.000 MiB
    9   addr 127.611 TiB  size 573.102 MiB access 1.000 hz age 0 ns          df-passed 574.000 MiB
    10  addr 127.612 TiB  size 245.617 MiB access 2.000 hz age 0 ns          df-passed 246.000 MiB
    11  addr 127.612 TiB  size 295.102 MiB access 1.000 hz age 1 s           df-passed 294.000 MiB
    12  addr 127.612 TiB  size 295.105 MiB access 1.000 hz age 1 s           df-passed 296.000 MiB
    13  addr 127.613 TiB  size 67.172 MiB  access 1.000 hz age 1 s           df-passed 66.000 MiB
    14  addr 127.613 TiB  size 604.570 MiB access 0 hz   age 1 s           df-passed 606.000 MiB
    15  addr 127.613 TiB  size 389.578 MiB access 0 hz   age 4 s           df-passed 388.000 MiB
    16  addr 127.614 TiB  size 259.719 MiB access 0 hz   age 4 s           df-passed 260.000 MiB
    17  addr 127.614 TiB  size 817.941 MiB access 0 hz   age 4 s           df-passed 818.000 MiB
    18  addr 127.615 TiB  size 204.488 MiB access 0 hz   age 4 s           df-passed 204.000 MiB
    19  addr 127.615 TiB  size 730.902 MiB access 0 hz   age 4 s           df-passed 732.000 MiB
    20  addr 127.616 TiB  size 182.727 MiB access 0 hz   age 4 s           df-passed 182.000 MiB
    21  addr 127.616 TiB  size 926.824 MiB access 0 hz   age 2 s           df-passed 928.000 MiB
    22  addr 127.617 TiB  size 102.984 MiB access 0 hz   age 2 s           df-passed 102.000 MiB
    23  addr 127.617 TiB  size 86.527 MiB  access 0 hz   age 2 s           df-passed 86.000 MiB
    24  addr 127.617 TiB  size 778.777 MiB access 0 hz   age 2 s           df-passed 776.000 MiB
    25  addr 127.999 TiB  size 132.000 KiB access 0 hz   age 6 s           df-passed 0 B
    memory bw estimate: 6.524 GiB per second  df-passed: 6.527 GiB per second
    total size: 10.731 GiB  df-passed 10.000 GiB
    record DAMON intervals: sample 100 ms, aggr 1 s


$ # damon report again
$sudo ./damo/damo report access --snapshot_damos_filter allow \
hugepage_size 2MiB 2MiB
    heatmap:
    # min/max temperatures: -1,100,000,000, 2,000, column size: 137.352 MiB
    intervals: sample 100 ms aggr 1 s (max access hz 10)
    # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
    df-pass:
    # min/max temperatures: -900,000,000, 2,000, column size: 128.031 MiB
    0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 11 s          df-passed 0 B
    1   addr 127.608 TiB  size 579.715 MiB access 2.000 hz age 0 ns          df-passed 580.000 MiB
    2   addr 127.608 TiB  size 144.930 MiB access 2.000 hz age 0 ns          df-passed 146.000 MiB
    3   addr 127.608 TiB  size 452.453 MiB access 2.000 hz age 0 ns          df-passed 452.000 MiB
    4   addr 127.609 TiB  size 113.117 MiB access 1.000 hz age 0 ns          df-passed 114.000 MiB
    5   addr 127.609 TiB  size 182.367 MiB access 2.000 hz age 0 ns          df-passed 182.000 MiB
    6   addr 127.609 TiB  size 182.371 MiB access 2.000 hz age 0 ns          df-passed 182.000 MiB
    7   addr 127.609 TiB  size 350.488 MiB access 1.000 hz age 0 ns          df-passed 350.000 MiB
    8   addr 127.610 TiB  size 525.738 MiB access 1.000 hz age 0 ns          df-passed 526.000 MiB
    9   addr 127.610 TiB  size 401.352 MiB access 1.000 hz age 0 ns          df-passed 402.000 MiB
    10  addr 127.611 TiB  size 100.340 MiB access 1.000 hz age 0 ns          df-passed 100.000 MiB
    11  addr 127.611 TiB  size 19.523 MiB  access 0 hz   age 0 ns          df-passed 20.000 MiB
    12  addr 127.611 TiB  size 175.727 MiB access 0 hz   age 0 ns          df-passed 176.000 MiB
    13  addr 127.611 TiB  size 106.629 MiB access 0 hz   age 0 ns          df-passed 106.000 MiB
    14  addr 127.611 TiB  size 959.676 MiB access 0 hz   age 0 ns          df-passed 960.000 MiB
    15  addr 127.612 TiB  size 424.469 MiB access 1.000 hz age 0 ns          df-passed 424.000 MiB
    16  addr 127.612 TiB  size 424.469 MiB access 1.000 hz age 0 ns          df-passed 424.000 MiB
    17  addr 127.613 TiB  size 201.648 MiB access 0 hz   age 6 s           df-passed 202.000 MiB
    18  addr 127.613 TiB  size 806.609 MiB access 0 hz   age 6 s           df-passed 806.000 MiB
    19  addr 127.614 TiB  size 862.125 MiB access 0 hz   age 9 s           df-passed 862.000 MiB
    20  addr 127.614 TiB  size 215.535 MiB access 0 hz   age 9 s           df-passed 216.000 MiB
    21  addr 127.615 TiB  size 104.500 MiB access 0 hz   age 9 s           df-passed 104.000 MiB
    22  addr 127.615 TiB  size 940.523 MiB access 0 hz   age 9 s           df-passed 942.000 MiB
    23  addr 127.616 TiB  size 640.281 MiB access 0 hz   age 7 s           df-passed 640.000 MiB
    24  addr 127.616 TiB  size 426.855 MiB access 0 hz   age 7 s           df-passed 426.000 MiB
    25  addr 127.617 TiB  size 90.105 MiB  access 0 hz   age 7 s           df-passed 90.000 MiB
    26  addr 127.617 TiB  size 810.965 MiB access 0 hz   age 7 s           df-passed 808.000 MiB
    27  addr 127.999 TiB  size 132.000 KiB access 0 hz   age 11 s          df-passed 0 B
    memory bw estimate: 5.297 GiB per second  df-passed: 5.297 GiB per second
    total size: 10.731 GiB  df-passed 10.000 GiB
    record DAMON intervals: sample 100 ms, aggr 1 s

As you can see the total df-passed region is 10GiB and the hot region
moves as the seq read keeps going

Revision History
================
Changes from v1 [1]:
- Follow David's advise to combine *pmd_entry() and *pte_entry() into
  one. Also remove manually setting walk->action
- Use vma_normal_page and vma_normal_page_pmd instead of damon_get_folio
  to remove redundant folio_get and folio_put
- Follow SJ's advise to only move damon_pa_scheme_has_filter to
  ops-common
- Change the command used in cover-letter for more natural illustration

[1] https://lore.kernel.org/all/cover.1753794408.git.pyyjason@gmail.com/

Yueyang Pan (2):
  mm/damon: Move has filter to ops-common
  mm/damon: Add damos_stat support for vaddr

 mm/damon/ops-common.c |   9 ++++
 mm/damon/ops-common.h |   2 +
 mm/damon/paddr.c      |  11 +----
 mm/damon/vaddr.c      | 105 +++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 116 insertions(+), 11 deletions(-)

-- 
2.47.3

[PATCH v2 1/2] mm/damon: Move has filter to ops-common

[Yueyang Pan]

This patch moves damon_pa_scheme_has_filter to ops-common. renaming
to damon_scheme_has_filter.
Doing so allows us to reuse its logic in the vaddr version
of DAMOS_STAT

Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
---
 mm/damon/ops-common.c |  9 +++++++++
 mm/damon/ops-common.h |  2 ++
 mm/damon/paddr.c      | 11 +----------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index 99321ff5cb92..3ebfa356ca46 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -412,3 +412,12 @@ unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid)
 
 	return nr_migrated;
 }
+
+bool damon_scheme_has_filter(struct damos *s)
+{
+	struct damos_filter *f;
+
+	damos_for_each_ops_filter(f, s)
+		return true;
+	return false;
+}
diff --git a/mm/damon/ops-common.h b/mm/damon/ops-common.h
index 61ad54aaf256..8d5c5c7631ac 100644
--- a/mm/damon/ops-common.h
+++ b/mm/damon/ops-common.h
@@ -21,3 +21,5 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
 
 bool damos_folio_filter_match(struct damos_filter *filter, struct folio *folio);
 unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid);
+
+bool damon_scheme_has_filter(struct damos *s);
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 53a55c5114fb..daeceed981a0 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -262,22 +262,13 @@ static unsigned long damon_pa_migrate(struct damon_region *r, struct damos *s,
 	return applied * PAGE_SIZE;
 }
 
-static bool damon_pa_scheme_has_filter(struct damos *s)
-{
-	struct damos_filter *f;
-
-	damos_for_each_ops_filter(f, s)
-		return true;
-	return false;
-}
-
 static unsigned long damon_pa_stat(struct damon_region *r, struct damos *s,
 		unsigned long *sz_filter_passed)
 {
 	unsigned long addr;
 	struct folio *folio;
 
-	if (!damon_pa_scheme_has_filter(s))
+	if (!damon_scheme_has_filter(s))
 		return 0;
 
 	addr = r->ar.start;
-- 
2.47.3

[PATCH v2 2/2] mm/damon: Add damos_stat support for vaddr

[Yueyang Pan]

This patch adds support for damos_stat in virtual address space.
It leverages the walk_page_range to walk the page table and gets
the folio from page table. The last folio scanned is stored in
damos->last_applied to prevent double counting.

Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
---
 mm/damon/vaddr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 87e825349bdf..6ed919e817e2 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -890,6 +890,109 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	return applied * PAGE_SIZE;
 }
 
+struct damos_va_stat_private {
+	struct damos *scheme;
+	unsigned long *sz_filter_passed;
+};
+
+static inline bool damon_va_invalid_damos_folio(struct folio *folio, struct damos *s)
+{
+	return !folio || folio == s->last_applied;
+}
+
+static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
+		unsigned long next, struct mm_walk *walk)
+{
+	struct damos_va_stat_private *priv = walk->private;
+	struct damos *s = priv->scheme;
+	unsigned long *sz_filter_passed = priv->sz_filter_passed;
+	struct vm_area_struct *vma = walk->vma;
+	struct folio *folio;
+	spinlock_t *ptl;
+	pte_t *start_pte, *pte, ptent;
+	int nr;
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	if (pmd_trans_huge(*pmd)) {
+		pmd_t pmde;
+
+		ptl = pmd_trans_huge_lock(pmd, vma);
+		if (!ptl)
+			return 0;
+		pmde = pmdp_get(pmd);
+		if (!pmd_present(pmde))
+			goto huge_unlock;
+
+		folio = vm_normal_folio_pmd(vma, addr, pmde);
+
+		if (damon_va_invalid_damos_folio(folio, s))
+			goto huge_unlock;
+
+		if (!damos_va_filter_out(s, folio, vma, addr, NULL, pmd))
+			*sz_filter_passed += folio_size(folio);
+		s->last_applied = folio;
+
+huge_unlock:
+		spin_unlock(ptl);
+		return 0;
+	}
+#endif
+	start_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+	if (!start_pte)
+		return 0;
+
+	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
+		nr = 1;
+		ptent = ptep_get(pte);
+
+		if (pte_none(ptent) || !pte_present(ptent))
+			continue;
+
+		folio = vm_normal_folio(vma, addr, ptent);
+
+		if (damon_va_invalid_damos_folio(folio, s))
+			continue;
+
+		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
+			*sz_filter_passed += folio_size(folio);
+		nr = folio_nr_pages(folio);
+		s->last_applied = folio;
+	}
+
+	pte_unmap_unlock(start_pte, ptl);
+
+	return 0;
+}
+
+static unsigned long damos_va_stat(struct damon_target *target,
+		struct damon_region *r, struct damos *s,
+		unsigned long *sz_filter_passed)
+{
+
+	struct damos_va_stat_private priv;
+	struct mm_struct *mm;
+	struct mm_walk_ops walk_ops = {
+		.pmd_entry = damos_va_stat_pmd_entry,
+		.walk_lock = PGWALK_RDLOCK,
+	};
+
+	priv.scheme = s;
+	priv.sz_filter_passed = sz_filter_passed;
+
+	if (!damon_scheme_has_filter(s))
+		return 0;
+
+	mm = damon_get_mm(target);
+	if (!mm)
+		return 0;
+
+	mmap_read_lock(mm);
+	walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
+	mmap_read_unlock(mm);
+	mmput(mm);
+	return 0;
+}
+
 static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
 		struct damon_target *t, struct damon_region *r,
 		struct damos *scheme, unsigned long *sz_filter_passed)
@@ -916,7 +1019,7 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
 	case DAMOS_MIGRATE_COLD:
 		return damos_va_migrate(t, r, scheme, sz_filter_passed);
 	case DAMOS_STAT:
-		return 0;
+		return damos_va_stat(t, r, scheme, sz_filter_passed);
 	default:
 		/*
 		 * DAMOS actions that are not yet supported by 'vaddr'.
-- 
2.47.3

Re: [PATCH v2 1/2] mm/damon: Move has filter to ops-common

[SeongJae Park]

On Wed, 30 Jul 2025 10:19:55 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:

> This patch moves damon_pa_scheme_has_filter to ops-common. renaming
> to damon_scheme_has_filter.
> Doing so allows us to reuse its logic in the vaddr version
> of DAMOS_STAT
> 
> Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
> ---
>  mm/damon/ops-common.c |  9 +++++++++
>  mm/damon/ops-common.h |  2 ++
>  mm/damon/paddr.c      | 11 +----------
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 99321ff5cb92..3ebfa356ca46 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -412,3 +412,12 @@ unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid)
>  
>  	return nr_migrated;
>  }
> +
> +bool damon_scheme_has_filter(struct damos *s)
> +{
> +	struct damos_filter *f;
> +
> +	damos_for_each_ops_filter(f, s)
> +		return true;
> +	return false;
> +}

I should have tell this earlier, sorry.  I now think it would be good to have
ops-common.c own prefix, since this namee makes me expect the function is on
DAMON core layer.  Also, I use normal 'grep' and ctags at the best, and maybe
I'm not the only one that that lazy at learning new tools.

Following the weird and none-public naming convention we have on DAMON,
damos_ops_ for DAMOS-related functions and damon_ops_ for monitoring-related
functions would be the prefix for ops-common.c.

So, what about renaming this to damos_ops_has_filter() if we have a chance to
revision this once again?

> diff --git a/mm/damon/ops-common.h b/mm/damon/ops-common.h
> index 61ad54aaf256..8d5c5c7631ac 100644
> --- a/mm/damon/ops-common.h
> +++ b/mm/damon/ops-common.h
> @@ -21,3 +21,5 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
>  
>  bool damos_folio_filter_match(struct damos_filter *filter, struct folio *folio);
>  unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid);
> +
> +bool damon_scheme_has_filter(struct damos *s);
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 53a55c5114fb..daeceed981a0 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -262,22 +262,13 @@ static unsigned long damon_pa_migrate(struct damon_region *r, struct damos *s,
>  	return applied * PAGE_SIZE;
>  }
>  
> -static bool damon_pa_scheme_has_filter(struct damos *s)
> -{
> -	struct damos_filter *f;
> -
> -	damos_for_each_ops_filter(f, s)
> -		return true;
> -	return false;
> -}
> -
>  static unsigned long damon_pa_stat(struct damon_region *r, struct damos *s,
>  		unsigned long *sz_filter_passed)
>  {
>  	unsigned long addr;
>  	struct folio *folio;
>  
> -	if (!damon_pa_scheme_has_filter(s))
> +	if (!damon_scheme_has_filter(s))
>  		return 0;
>  
>  	addr = r->ar.start;
> -- 
> 2.47.3

Otherwise, all looks good to me.


Thanks,
SJ

Re: [PATCH v2 2/2] mm/damon: Add damos_stat support for vaddr

[SeongJae Park]

On Wed, 30 Jul 2025 10:19:56 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:

> This patch adds support for damos_stat in virtual address space.
> It leverages the walk_page_range to walk the page table and gets
> the folio from page table. The last folio scanned is stored in
> damos->last_applied to prevent double counting.
> 
> Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
> ---
>  mm/damon/vaddr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 87e825349bdf..6ed919e817e2 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -890,6 +890,109 @@ static unsigned long damos_va_migrate(struct damon_target *target,
>  	return applied * PAGE_SIZE;
>  }
>  
> +struct damos_va_stat_private {
> +	struct damos *scheme;
> +	unsigned long *sz_filter_passed;
> +};
> +
> +static inline bool damon_va_invalid_damos_folio(struct folio *folio, struct damos *s)

Weirdly DAMON code usually keeps the 80 columns limit.  Could you please break
down this line?

Also, the name feels long to me.  What about damos_va_invalid_folio()?

> +{
> +	return !folio || folio == s->last_applied;
> +}
> +
> +static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
> +		unsigned long next, struct mm_walk *walk)
> +{
> +	struct damos_va_stat_private *priv = walk->private;
> +	struct damos *s = priv->scheme;
> +	unsigned long *sz_filter_passed = priv->sz_filter_passed;
> +	struct vm_area_struct *vma = walk->vma;
> +	struct folio *folio;
> +	spinlock_t *ptl;
> +	pte_t *start_pte, *pte, ptent;
> +	int nr;
> +
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	if (pmd_trans_huge(*pmd)) {
> +		pmd_t pmde;
> +
> +		ptl = pmd_trans_huge_lock(pmd, vma);
> +		if (!ptl)
> +			return 0;
> +		pmde = pmdp_get(pmd);
> +		if (!pmd_present(pmde))
> +			goto huge_unlock;
> +
> +		folio = vm_normal_folio_pmd(vma, addr, pmde);
> +
> +		if (damon_va_invalid_damos_folio(folio, s))
> +			goto huge_unlock;
> +
> +		if (!damos_va_filter_out(s, folio, vma, addr, NULL, pmd))
> +			*sz_filter_passed += folio_size(folio);
> +		s->last_applied = folio;
> +
> +huge_unlock:
> +		spin_unlock(ptl);
> +		return 0;
> +	}
> +#endif
> +	start_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> +	if (!start_pte)
> +		return 0;
> +
> +	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
> +		nr = 1;
> +		ptent = ptep_get(pte);
> +
> +		if (pte_none(ptent) || !pte_present(ptent))
> +			continue;
> +
> +		folio = vm_normal_folio(vma, addr, ptent);
> +
> +		if (damon_va_invalid_damos_folio(folio, s))
> +			continue;
> +
> +		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
> +			*sz_filter_passed += folio_size(folio);
> +		nr = folio_nr_pages(folio);
> +		s->last_applied = folio;
> +	}
> +
> +	pte_unmap_unlock(start_pte, ptl);
> +

No strong opinion, but I'd like to drop above two blank lines (one after the
for loop, and one after pte_unmap_unlock() if you don't mind.

> +	return 0;
> +}
> +
> +static unsigned long damos_va_stat(struct damon_target *target,
> +		struct damon_region *r, struct damos *s,
> +		unsigned long *sz_filter_passed)
> +{
> +

Seems this is unnecessary blank line.  Let's remove it.

> +	struct damos_va_stat_private priv;
> +	struct mm_struct *mm;
> +	struct mm_walk_ops walk_ops = {
> +		.pmd_entry = damos_va_stat_pmd_entry,
> +		.walk_lock = PGWALK_RDLOCK,
> +	};
> +
> +	priv.scheme = s;
> +	priv.sz_filter_passed = sz_filter_passed;
> +
> +	if (!damon_scheme_has_filter(s))
> +		return 0;
> +
> +	mm = damon_get_mm(target);
> +	if (!mm)
> +		return 0;
> +
> +	mmap_read_lock(mm);
> +	walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
> +	mmap_read_unlock(mm);
> +	mmput(mm);
> +	return 0;
> +}
> +
>  static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
>  		struct damon_target *t, struct damon_region *r,
>  		struct damos *scheme, unsigned long *sz_filter_passed)
> @@ -916,7 +1019,7 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
>  	case DAMOS_MIGRATE_COLD:
>  		return damos_va_migrate(t, r, scheme, sz_filter_passed);
>  	case DAMOS_STAT:
> -		return 0;
> +		return damos_va_stat(t, r, scheme, sz_filter_passed);
>  	default:
>  		/*
>  		 * DAMOS actions that are not yet supported by 'vaddr'.
> -- 
> 2.47.3


Thanks,
SJ

Re: [PATCH v2 0/2] mm/damon: Add damos_stat support for vaddr

[SeongJae Park]

On Wed, 30 Jul 2025 10:19:54 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:

> Previously damos_stat only supoort paddr. This patch set adds support 
> for damos_stat for vaddr. Also all different types of filters are 
> supported. 
> 
> Functionality Test
> ==================
> I wrote a small test program which allocates 10GB of DRAM, use 
> madvise(MADV_HUGEPAGE) to convert the base pages to 2MB huge pages
> Then my program does the following things in order:
> 1. Write sequentially to the whole 10GB region
> 2. Read the first 5GB region sequentially for 10 times
> 3. Sleep 5s
> 4. Read the second 5GB region sequentially for 10 times
> 
> With a proper damon setting, we are expected to see df-passed to be 10GB
> and hot region move around with the read
> 
> $ # Start damon record
> $sudo ./damo/damo start "./my_test/test" --monitoring_intervals 100ms\
> 1s 2s

Above command is not really doing record, so the first line would better to be
updated, say, "Start DAMON" ?

Also, it is easier to read if you give prefix spaces for quoting commands as
well, to make it consistent with their outputs.  Fo for example, above could be
something like below.

    $ # Start DAMON
    $ sudo ./damo/damo start ...

> 
> $ # damon report
> $sudo ./damo/damo report access --snapshot_damos_filter allow \
> hugepage_size 2MiB 2MiB

Again, I'd prefer having prefix spaces for above commands consistent to the
following output.

Also, maybe the first comment could be more detailed, say, "Show
DAMON-generated access pattern snapshot" ?

>     heatmap:
>     # min/max temperatures: -600,000,000, 100,001,000, column size: 137.352 MiB
>     intervals: sample 100 ms aggr 1 s (max access hz 10)
>     # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
>     df-pass:
>     # min/max temperatures: -400,000,000, 100,001,000, column size: 128.031 MiB
>     0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 6 s           df-passed 0 B
>     1   addr 127.608 TiB  size 877.664 MiB access 3.000 hz age 0 ns          df-passed 878.000 MiB
>     2   addr 127.609 TiB  size 219.418 MiB access 2.000 hz age 0 ns          df-passed 220.000 MiB
>     3   addr 127.609 TiB  size 316.613 MiB access 1.000 hz age 1 s           df-passed 316.000 MiB
[...]
>     memory bw estimate: 6.524 GiB per second  df-passed: 6.527 GiB per second
>     total size: 10.731 GiB  df-passed 10.000 GiB
>     record DAMON intervals: sample 100 ms, aggr 1 s
> 
> 
> $ # damon report again
> $sudo ./damo/damo report access --snapshot_damos_filter allow \
> hugepage_size 2MiB 2MiB

Ditto.

>     heatmap:
>     # min/max temperatures: -1,100,000,000, 2,000, column size: 137.352 MiB
>     intervals: sample 100 ms aggr 1 s (max access hz 10)
>     # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
>     df-pass:
>     # min/max temperatures: -900,000,000, 2,000, column size: 128.031 MiB
>     0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 11 s          df-passed 0 B
>     1   addr 127.608 TiB  size 579.715 MiB access 2.000 hz age 0 ns          df-passed 580.000 MiB
>     2   addr 127.608 TiB  size 144.930 MiB access 2.000 hz age 0 ns          df-passed 146.000 MiB
>     3   addr 127.608 TiB  size 452.453 MiB access 2.000 hz age 0 ns          df-passed 452.000 MiB
[...]
>     memory bw estimate: 5.297 GiB per second  df-passed: 5.297 GiB per second
>     total size: 10.731 GiB  df-passed 10.000 GiB
>     record DAMON intervals: sample 100 ms, aggr 1 s
> 
> As you can see the total df-passed region is 10GiB and the hot region
> moves as the seq read keeps going
> 
> Revision History
> ================
> Changes from v1 [1]:
> - Follow David's advise to combine *pmd_entry() and *pte_entry() into
>   one. Also remove manually setting walk->action
> - Use vma_normal_page and vma_normal_page_pmd instead of damon_get_folio
>   to remove redundant folio_get and folio_put
> - Follow SJ's advise to only move damon_pa_scheme_has_filter to
>   ops-common
> - Change the command used in cover-letter for more natural illustration
> 
> [1] https://lore.kernel.org/all/cover.1753794408.git.pyyjason@gmail.com/
> 
> Yueyang Pan (2):
>   mm/damon: Move has filter to ops-common
>   mm/damon: Add damos_stat support for vaddr

I also left a few trivial comments to the patches.  So I find no real concerns
but asking for a few trivial cleanups.  Could you please make the changes with
next version, after waiting a day for giving others to chime in for things I
may missed?


Thanks,
SJ

[...]

Re: [PATCH v2 2/2] mm/damon: Add damos_stat support for vaddr

[YUEYANG PAN]

On Wed, Jul 30, 2025 at 10:56:52AM -0700, SeongJae Park wrote:
> On Wed, 30 Jul 2025 10:19:56 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:
> 
> > This patch adds support for damos_stat in virtual address space.
> > It leverages the walk_page_range to walk the page table and gets
> > the folio from page table. The last folio scanned is stored in
> > damos->last_applied to prevent double counting.
> > 
> > Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
> > ---
> >  mm/damon/vaddr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 104 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 87e825349bdf..6ed919e817e2 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -890,6 +890,109 @@ static unsigned long damos_va_migrate(struct damon_target *target,
> >  	return applied * PAGE_SIZE;
> >  }
> >  
> > +struct damos_va_stat_private {
> > +	struct damos *scheme;
> > +	unsigned long *sz_filter_passed;
> > +};
> > +
> > +static inline bool damon_va_invalid_damos_folio(struct folio *folio, struct damos *s)
> 
> Weirdly DAMON code usually keeps the 80 columns limit.  Could you please break
> down this line?
> 
> Also, the name feels long to me.  What about damos_va_invalid_folio()?
> 

Thanks. I will fix it in the next version.

> > +{
> > +	return !folio || folio == s->last_applied;
> > +}
> > +
> > +static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
> > +		unsigned long next, struct mm_walk *walk)
> > +{
> > +	struct damos_va_stat_private *priv = walk->private;
> > +	struct damos *s = priv->scheme;
> > +	unsigned long *sz_filter_passed = priv->sz_filter_passed;
> > +	struct vm_area_struct *vma = walk->vma;
> > +	struct folio *folio;
> > +	spinlock_t *ptl;
> > +	pte_t *start_pte, *pte, ptent;
> > +	int nr;
> > +
> > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > +	if (pmd_trans_huge(*pmd)) {
> > +		pmd_t pmde;
> > +
> > +		ptl = pmd_trans_huge_lock(pmd, vma);
> > +		if (!ptl)
> > +			return 0;
> > +		pmde = pmdp_get(pmd);
> > +		if (!pmd_present(pmde))
> > +			goto huge_unlock;
> > +
> > +		folio = vm_normal_folio_pmd(vma, addr, pmde);
> > +
> > +		if (damon_va_invalid_damos_folio(folio, s))
> > +			goto huge_unlock;
> > +
> > +		if (!damos_va_filter_out(s, folio, vma, addr, NULL, pmd))
> > +			*sz_filter_passed += folio_size(folio);
> > +		s->last_applied = folio;
> > +
> > +huge_unlock:
> > +		spin_unlock(ptl);
> > +		return 0;
> > +	}
> > +#endif
> > +	start_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> > +	if (!start_pte)
> > +		return 0;
> > +
> > +	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
> > +		nr = 1;
> > +		ptent = ptep_get(pte);
> > +
> > +		if (pte_none(ptent) || !pte_present(ptent))
> > +			continue;
> > +
> > +		folio = vm_normal_folio(vma, addr, ptent);
> > +
> > +		if (damon_va_invalid_damos_folio(folio, s))
> > +			continue;
> > +
> > +		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
> > +			*sz_filter_passed += folio_size(folio);
> > +		nr = folio_nr_pages(folio);
> > +		s->last_applied = folio;
> > +	}
> > +
> > +	pte_unmap_unlock(start_pte, ptl);
> > +
> 
> No strong opinion, but I'd like to drop above two blank lines (one after the
> for loop, and one after pte_unmap_unlock() if you don't mind.
> 

Will remove it in the next version.

> > +	return 0;
> > +}
> > +
> > +static unsigned long damos_va_stat(struct damon_target *target,
> > +		struct damon_region *r, struct damos *s,
> > +		unsigned long *sz_filter_passed)
> > +{
> > +
> 
> Seems this is unnecessary blank line.  Let's remove it.

Will remove it in the next version.

> 
> > +	struct damos_va_stat_private priv;
> > +	struct mm_struct *mm;
> > +	struct mm_walk_ops walk_ops = {
> > +		.pmd_entry = damos_va_stat_pmd_entry,
> > +		.walk_lock = PGWALK_RDLOCK,
> > +	};
> > +
> > +	priv.scheme = s;
> > +	priv.sz_filter_passed = sz_filter_passed;
> > +
> > +	if (!damon_scheme_has_filter(s))
> > +		return 0;
> > +
> > +	mm = damon_get_mm(target);
> > +	if (!mm)
> > +		return 0;
> > +
> > +	mmap_read_lock(mm);
> > +	walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
> > +	mmap_read_unlock(mm);
> > +	mmput(mm);
> > +	return 0;
> > +}
> > +
> >  static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
> >  		struct damon_target *t, struct damon_region *r,
> >  		struct damos *scheme, unsigned long *sz_filter_passed)
> > @@ -916,7 +1019,7 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
> >  	case DAMOS_MIGRATE_COLD:
> >  		return damos_va_migrate(t, r, scheme, sz_filter_passed);
> >  	case DAMOS_STAT:
> > -		return 0;
> > +		return damos_va_stat(t, r, scheme, sz_filter_passed);
> >  	default:
> >  		/*
> >  		 * DAMOS actions that are not yet supported by 'vaddr'.
> > -- 
> > 2.47.3
> 
> 
> Thanks,
> SJ

Best Wishes
Pan

Re: [PATCH v2 1/2] mm/damon: Move has filter to ops-common

[YUEYANG PAN]

On Wed, Jul 30, 2025 at 10:45:06AM -0700, SeongJae Park wrote:
> On Wed, 30 Jul 2025 10:19:55 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:
> 
> > This patch moves damon_pa_scheme_has_filter to ops-common. renaming
> > to damon_scheme_has_filter.
> > Doing so allows us to reuse its logic in the vaddr version
> > of DAMOS_STAT
> > 
> > Signed-off-by: Yueyang Pan <pyyjason@gmail.com>
> > ---
> >  mm/damon/ops-common.c |  9 +++++++++
> >  mm/damon/ops-common.h |  2 ++
> >  mm/damon/paddr.c      | 11 +----------
> >  3 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index 99321ff5cb92..3ebfa356ca46 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> > @@ -412,3 +412,12 @@ unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid)
> >  
> >  	return nr_migrated;
> >  }
> > +
> > +bool damon_scheme_has_filter(struct damos *s)
> > +{
> > +	struct damos_filter *f;
> > +
> > +	damos_for_each_ops_filter(f, s)
> > +		return true;
> > +	return false;
> > +}
> 
> I should have tell this earlier, sorry.  I now think it would be good to have
> ops-common.c own prefix, since this namee makes me expect the function is on
> DAMON core layer.  Also, I use normal 'grep' and ctags at the best, and maybe
> I'm not the only one that that lazy at learning new tools.
> 
> Following the weird and none-public naming convention we have on DAMON,
> damos_ops_ for DAMOS-related functions and damon_ops_ for monitoring-related
> functions would be the prefix for ops-common.c.
> 
> So, what about renaming this to damos_ops_has_filter() if we have a chance to
> revision this once again?

Thanks. I will fix it in the next version.

> 
> > diff --git a/mm/damon/ops-common.h b/mm/damon/ops-common.h
> > index 61ad54aaf256..8d5c5c7631ac 100644
> > --- a/mm/damon/ops-common.h
> > +++ b/mm/damon/ops-common.h
> > @@ -21,3 +21,5 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
> >  
> >  bool damos_folio_filter_match(struct damos_filter *filter, struct folio *folio);
> >  unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid);
> > +
> > +bool damon_scheme_has_filter(struct damos *s);
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 53a55c5114fb..daeceed981a0 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -262,22 +262,13 @@ static unsigned long damon_pa_migrate(struct damon_region *r, struct damos *s,
> >  	return applied * PAGE_SIZE;
> >  }
> >  
> > -static bool damon_pa_scheme_has_filter(struct damos *s)
> > -{
> > -	struct damos_filter *f;
> > -
> > -	damos_for_each_ops_filter(f, s)
> > -		return true;
> > -	return false;
> > -}
> > -
> >  static unsigned long damon_pa_stat(struct damon_region *r, struct damos *s,
> >  		unsigned long *sz_filter_passed)
> >  {
> >  	unsigned long addr;
> >  	struct folio *folio;
> >  
> > -	if (!damon_pa_scheme_has_filter(s))
> > +	if (!damon_scheme_has_filter(s))
> >  		return 0;
> >  
> >  	addr = r->ar.start;
> > -- 
> > 2.47.3
> 
> Otherwise, all looks good to me.
> 
> 
> Thanks,
> SJ

Best Wishes
Pan