mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/damon: skip folios already on the migration target node
@ 2026-10-28 13:05 Aveline Noir
  2026-10-29  1:01 ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: Aveline Noir @ 2026-10-28 13:05 UTC (permalink / raw)
  To: SJ Park, Andrew Morton
  Cc: damon, linux-mm, linux-kernel, Bijan Tabatabai, Bijan Tabatabai

While measuring DAMOS migration overhead, I found that the target_nid
path isolates folios already on the target node, then puts them back on
the LRU without migrating them.

Skip them before isolation in vaddr and paddr, as the vaddr explicit
destinations path already does.

Signed-off-by: Aveline Noir <jm5905938@gmail.com>

---
DAMON KUnit and selftests all passed. In a synthetic same-node fvaddr
test on a two-node x86-64 KVM guest (8 interleaved runs per kernel),
guest-kernel cycles fell by about 29%
with 4 KiB regions and 90% with 16 MiB regions. Median kdamond CPU
time fell by about 76% with 16 MiB regions, with no clear change
with 4 KiB regions.
---
 mm/damon/paddr.c |  2 ++
 mm/damon/vaddr.c | 22 ++++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 2cfdc356b415..5b773012a876 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -368,6 +368,8 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
 		else
 			*sz_filter_passed += folio_size(folio) / addr_unit;
 
+		if (folio_nid(folio) == s->target_nid)
+			goto put_folio;
 		if (!folio_isolate_lru(folio))
 			goto put_folio;
 		node_stat_add_folio(folio, NR_ISOLATED_ANON +
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b549496ea8e2..cd71fb1ac209 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -786,14 +786,15 @@ struct damos_va_migrate_private {
  */
 static void damos_va_migrate_dests_add(struct folio *folio,
 		struct vm_area_struct *vma, unsigned long addr,
-		struct damos_migrate_dests *dests,
+		struct damos *s,
 		struct list_head *migration_lists)
 {
+	struct damos_migrate_dests *dests = &s->migrate_dests;
 	pgoff_t ilx;
 	int order;
 	unsigned int target;
 	unsigned int weight_total = 0;
-	int i;
+	int i, nid;
 
 	/*
 	 * If dests is empty, there is only one migration list corresponding
@@ -801,7 +802,8 @@ static void damos_va_migrate_dests_add(struct folio *folio,
 	 */
 	if (!dests->nr_dests) {
 		i = 0;
-		goto isolate;
+		nid = s->target_nid;
+		goto check_nid;
 	}
 
 	order = folio_order(folio);
@@ -821,12 +823,13 @@ static void damos_va_migrate_dests_add(struct folio *folio,
 			break;
 		target -= dests->weight_arr[i];
 	}
+	nid = dests->node_id_arr[i];
 
+check_nid:
 	/* If the folio is already in the right node, don't do anything */
-	if (folio_nid(folio) == dests->node_id_arr[i])
+	if (folio_nid(folio) == nid)
 		return;
 
-isolate:
 	if (!folio_isolate_lru(folio))
 		return;
 	node_stat_add_folio(folio, NR_ISOLATED_ANON +
@@ -840,7 +843,6 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 	struct damos_va_migrate_private *priv = walk->private;
 	struct list_head *migration_lists = priv->migration_lists;
 	struct damos *s = priv->scheme;
-	struct damos_migrate_dests *dests = &s->migrate_dests;
 	struct folio *folio;
 	spinlock_t *ptl;
 	pte_t *start_pte, *pte, ptent;
@@ -858,8 +860,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 			goto huge_out;
 		if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
 			goto huge_out;
-		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
-				migration_lists);
+		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
+					   migration_lists);
 huge_out:
 		spin_unlock(ptl);
 		return 0;
@@ -883,8 +885,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 			continue;
 		if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
 			continue;
-		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
-				migration_lists);
+		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
+					   migration_lists);
 		page_idx = folio_page_idx(folio, pte_page(ptent));
 		nr = folio_nr_pages(folio) - page_idx;
 	}

base-commit: d865f2677c44cb2171b833d6775cf692756fa8be
-- 
2.56.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/damon: skip folios already on the migration target node
  2026-10-28 13:05 [PATCH] mm/damon: skip folios already on the migration target node Aveline Noir
@ 2026-10-29  1:01 ` SJ Park
  2026-10-29  1:06   ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-10-29  1:01 UTC (permalink / raw)
  To: Aveline Noir
  Cc: SJ Park, Andrew Morton, damon, linux-mm, linux-kernel,
	Bijan Tabatabai, Bijan Tabatabai

On Thu, 29 Oct 2026 00:05:46 +1100 Aveline Noir <jm5905938@gmail.com> wrote:

> While measuring DAMOS migration overhead, I found that the target_nid
> path isolates folios already on the target node, then puts them back on
> the LRU without migrating them.
> 
> Skip them before isolation in vaddr and paddr, as the vaddr explicit
> destinations path already does.

Nice catch!

> 
> Signed-off-by: Aveline Noir <jm5905938@gmail.com>

Other than below trivial nit,

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

> 
> ---
> DAMON KUnit and selftests all passed. In a synthetic same-node fvaddr
> test on a two-node x86-64 KVM guest (8 interleaved runs per kernel),
> guest-kernel cycles fell by about 29%
> with 4 KiB regions and 90% with 16 MiB regions. Median kdamond CPU
> time fell by about 76% with 16 MiB regions, with no clear change
> with 4 KiB regions.

Thank you for sharing the test result.  We don't want to add too much test
results in the commit message.  But the performance part above is informative
and concise, so I think it deserves to be in the commit message.

> ---
>  mm/damon/paddr.c |  2 ++
>  mm/damon/vaddr.c | 22 ++++++++++++----------
>  2 files changed, 14 insertions(+), 10 deletions(-)
[...]
> @@ -858,8 +860,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
>  			goto huge_out;
>  		if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
>  			goto huge_out;
> -		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> -				migration_lists);
> +		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
> +					   migration_lists);

Let's not align parameters on parentheses but just give two tabs indentation.

>  huge_out:
>  		spin_unlock(ptl);
>  		return 0;
> @@ -883,8 +885,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
>  			continue;
>  		if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
>  			continue;
> -		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> -				migration_lists);
> +		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
> +					   migration_lists);

Ditto.

Assuming you accept my humble suggestions above, I applied  this patch to
damon/next [1] tree, together with my above suggestions.  I will repost it for
mm.git inclusion within a week.  So, no action from your side is needed for
now.  Let me know if you think changes to the plan or discussions are needed.

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


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/damon: skip folios already on the migration target node
  2026-10-29  1:01 ` SJ Park
@ 2026-10-29  1:06   ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-10-29  1:06 UTC (permalink / raw)
  To: SJ Park
  Cc: Aveline Noir, Andrew Morton, damon, linux-mm, linux-kernel,
	Bijan Tabatabai, Bijan Tabatabai

One more trivial nit.  Let's use 'mm/damon/{p,v}addr:' as the subject prefix.

On Wed, 28 Oct 2026 18:01:43 -0700 SJ Park <sj@kernel.org> wrote:

> On Thu, 29 Oct 2026 00:05:46 +1100 Aveline Noir <jm5905938@gmail.com> wrote:
> 
> > While measuring DAMOS migration overhead, I found that the target_nid
> > path isolates folios already on the target node, then puts them back on
> > the LRU without migrating them.
> > 
> > Skip them before isolation in vaddr and paddr, as the vaddr explicit
> > destinations path already does.
> 
> Nice catch!
> 
> > 
> > Signed-off-by: Aveline Noir <jm5905938@gmail.com>
> 
> Other than below trivial nit,
> 
> Reviewed-by: SJ Park <sj@kernel.org>
> 
> > 
> > ---
> > DAMON KUnit and selftests all passed. In a synthetic same-node fvaddr
> > test on a two-node x86-64 KVM guest (8 interleaved runs per kernel),
> > guest-kernel cycles fell by about 29%
> > with 4 KiB regions and 90% with 16 MiB regions. Median kdamond CPU
> > time fell by about 76% with 16 MiB regions, with no clear change
> > with 4 KiB regions.
> 
> Thank you for sharing the test result.  We don't want to add too much test
> results in the commit message.  But the performance part above is informative
> and concise, so I think it deserves to be in the commit message.
> 
> > ---
> >  mm/damon/paddr.c |  2 ++
> >  mm/damon/vaddr.c | 22 ++++++++++++----------
> >  2 files changed, 14 insertions(+), 10 deletions(-)
> [...]
> > @@ -858,8 +860,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  			goto huge_out;
> >  		if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
> >  			goto huge_out;
> > -		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> > -				migration_lists);
> > +		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
> > +					   migration_lists);
> 
> Let's not align parameters on parentheses but just give two tabs indentation.
> 
> >  huge_out:
> >  		spin_unlock(ptl);
> >  		return 0;
> > @@ -883,8 +885,8 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  			continue;
> >  		if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
> >  			continue;
> > -		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> > -				migration_lists);
> > +		damos_va_migrate_dests_add(folio, walk->vma, addr, s,
> > +					   migration_lists);
> 
> Ditto.
> 
> Assuming you accept my humble suggestions above, I applied  this patch to
> damon/next [1] tree, together with my above suggestions.  I will repost it for
> mm.git inclusion within a week.  So, no action from your side is needed for
> now.  Let me know if you think changes to the plan or discussions are needed.

I also added the suggested subject prefix in damon/next tree.  Please feel free
to let me know if you have an opinion.

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


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-29  1:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 13:05 [PATCH] mm/damon: skip folios already on the migration target node Aveline Noir
2026-10-29  1:01 ` SJ Park
2026-10-29  1:06   ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®