[PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[Daemyung Kang]

DAMOS migration accepts an unsigned int weight for each destination,
but sums the weights into an unsigned int.  If the total wraps to zero,
no folio is migrated.  Other wrapped totals select the wrong node.

Sum weights into a u64 once per migration walk.  Keep the existing
32-bit divisor path for totals up to UINT_MAX and use a 64-bit remainder
for larger totals.  Add KUnit coverage for the actual summing path and
for destination selection at the overflow boundary.

Fixes: 19c1dc15c859 ("mm/damon/vaddr: use damos->migrate_dests in migrate_{hot,cold}")
Signed-off-by: Daemyung Kang <charsyam@gmail.com>
---
 mm/damon/tests/vaddr-kunit.h | 35 +++++++++++++++++++
 mm/damon/vaddr.c             | 65 +++++++++++++++++++++++-------------
 2 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h
index d61b503..f67f1a0 100644
--- a/mm/damon/tests/vaddr-kunit.h
+++ b/mm/damon/tests/vaddr-kunit.h
@@ -266,12 +266,47 @@ static void damon_test_apply_three_regions4(struct kunit *test)
 			new_three_regions, expected, ARRAY_SIZE(expected));
 }
 
+static void damon_test_migrate_dests_idx(struct kunit *test)
+{
+	unsigned int weights[] = { UINT_MAX, 1 };
+	struct damos_migrate_dests dests = {
+		.weight_arr = weights,
+		.nr_dests = ARRAY_SIZE(weights),
+	};
+	u64 total = damos_va_migrate_weight_total(&dests);
+
+	KUNIT_EXPECT_EQ(test, total, (u64)UINT_MAX + 1);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests, 0, total), 0);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests,
+			UINT_MAX, total), 1);
+
+	weights[1] = 2;
+	total = damos_va_migrate_weight_total(&dests);
+	KUNIT_EXPECT_EQ(test, total, (u64)UINT_MAX + 2);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests,
+			UINT_MAX, total), 1);
+
+	weights[0] = 0;
+	weights[1] = 0;
+	total = damos_va_migrate_weight_total(&dests);
+	KUNIT_EXPECT_EQ(test, total, 0ULL);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests, 0, total), -1);
+
+	weights[0] = 1;
+	weights[1] = 3;
+	total = damos_va_migrate_weight_total(&dests);
+	KUNIT_EXPECT_EQ(test, total, 4ULL);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests, 1, total), 1);
+	KUNIT_EXPECT_EQ(test, damos_va_migrate_dests_idx(&dests, 4, total), 0);
+}
+
 static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_three_regions_in_vmas),
 	KUNIT_CASE(damon_test_apply_three_regions1),
 	KUNIT_CASE(damon_test_apply_three_regions2),
 	KUNIT_CASE(damon_test_apply_three_regions3),
 	KUNIT_CASE(damon_test_apply_three_regions4),
+	KUNIT_CASE(damon_test_migrate_dests_idx),
 	{},
 };
 
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index d5dde97..279dbc6 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -7,6 +7,7 @@
 
 #include <linux/highmem.h>
 #include <linux/hugetlb.h>
+#include <linux/math64.h>
 #include <linux/mman.h>
 #include <linux/mmu_notifier.h>
 #include <linux/page_idle.h>
@@ -774,8 +775,41 @@ static bool damos_va_filter_out(struct damos *scheme, struct folio *folio,
 struct damos_va_migrate_private {
 	struct list_head *migration_lists;
 	struct damos *scheme;
+	u64 weight_total;
 };
 
+static u64 damos_va_migrate_weight_total(struct damos_migrate_dests *dests)
+{
+	u64 total = 0;
+	size_t i;
+
+	for (i = 0; i < dests->nr_dests; i++)
+		total += dests->weight_arr[i];
+	return total;
+}
+
+static int damos_va_migrate_dests_idx(struct damos_migrate_dests *dests,
+		pgoff_t ilx, u64 weight_total)
+{
+	u64 target;
+	int i;
+
+	if (!weight_total)
+		return -1;
+
+	if (weight_total <= UINT_MAX)
+		target = ilx % (unsigned int)weight_total;
+	else
+		div64_u64_rem(ilx, weight_total, &target);
+
+	for (i = 0; i < dests->nr_dests; i++) {
+		if (target < dests->weight_arr[i])
+			return i;
+		target -= dests->weight_arr[i];
+	}
+	return -1;
+}
+
 /*
  * Place the given folio in the migration_list corresponding to where the folio
  * should be migrated.
@@ -784,13 +818,11 @@ 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 list_head *migration_lists)
+		struct damos_va_migrate_private *priv)
 {
+	struct damos_migrate_dests *dests = &priv->scheme->migrate_dests;
 	pgoff_t ilx;
 	int order;
-	unsigned int target;
-	unsigned int weight_total = 0;
 	int i;
 
 	/*
@@ -806,20 +838,10 @@ static void damos_va_migrate_dests_add(struct folio *folio,
 	ilx = vma_start_pgoff(vma) >> order;
 	ilx += linear_page_delta(vma, addr) >> order;
 
-	for (i = 0; i < dests->nr_dests; i++)
-		weight_total += dests->weight_arr[i];
-
-	/* If the total weights are somehow 0, don't migrate at all */
-	if (!weight_total)
+	i = damos_va_migrate_dests_idx(dests, ilx, priv->weight_total);
+	if (i < 0)
 		return;
 
-	target = ilx % weight_total;
-	for (i = 0; i < dests->nr_dests; i++) {
-		if (target < dests->weight_arr[i])
-			break;
-		target -= dests->weight_arr[i];
-	}
-
 	/* If the folio is already in the right node, don't do anything */
 	if (folio_nid(folio) == dests->node_id_arr[i])
 		return;
@@ -829,16 +851,14 @@ static void damos_va_migrate_dests_add(struct folio *folio,
 		return;
 	node_stat_add_folio(folio, NR_ISOLATED_ANON +
 			folio_is_file_lru(folio));
-	list_add(&folio->lru, &migration_lists[i]);
+	list_add(&folio->lru, &priv->migration_lists[i]);
 }
 
 static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
 		unsigned long next, struct mm_walk *walk)
 {
 	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;
@@ -856,8 +876,7 @@ 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, priv);
 huge_out:
 		spin_unlock(ptl);
 		return 0;
@@ -881,8 +900,7 @@ 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, priv);
 		page_idx = folio_page_idx(folio, pte_page(ptent));
 		nr = folio_nr_pages(folio) - page_idx;
 	}
@@ -958,6 +976,7 @@ static unsigned long damos_va_migrate(struct damon_target *target,
 	use_target_nid = dests->nr_dests == 0;
 	nr_dests = use_target_nid ? 1 : dests->nr_dests;
 	priv.scheme = s;
+	priv.weight_total = damos_va_migrate_weight_total(dests);
 	priv.migration_lists = kmalloc_objs(*priv.migration_lists, nr_dests);
 	if (!priv.migration_lists)
 		return 0;
-- 
2.43.0

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[Aveline Noir]

Hi Daemyung,

On Tue, 27 Oct 2026 12:54:31 +0900 Daemyung Kang wrote:

> Sum weights into a u64 once per migration walk.  Keep the existing
> 32-bit divisor path for totals up to UINT_MAX and use a 64-bit remainder
> for larger totals.

Tested locally. Looks good to me :)

Tested in an x86_64 QEMU guest; both the buggy and corrected behavior
were verified.

Reviewed-by: Aveline Noir <jm5905938@gmail.com>
Tested-by: Aveline Noir <jm5905938@gmail.com>

Thanks,
Aveline Noir

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[SJ Park]

Hello Daemyung,

On Tue, 27 Oct 2026 12:54:31 +0900 Daemyung Kang <charsyam@gmail.com> wrote:

> DAMOS migration accepts an unsigned int weight for each destination,
> but sums the weights into an unsigned int.  If the total wraps to zero,
> no folio is migrated.  Other wrapped totals select the wrong node.

Thank you for finding the issue.

But, is it realistic to have the weight_total overflow?  Have you found the
issue on real usage?  Or, theoretical?

> 
> Sum weights into a u64 once per migration walk.  Keep the existing
> 32-bit divisor path for totals up to UINT_MAX and use a 64-bit remainder
> for larger totals.  Add KUnit coverage for the actual summing path and
> for destination selection at the overflow boundary.

Do we really need to support that user inputs?  Unless we have a very real use
case that require such huge weight values, I'd prefer not.

How about just clearly notify users those are invalid parameters?  We could do
so by adding weight_total overflow check to the central parameters validation
path: damos_commit_dests().

> 
> Fixes: 19c1dc15c859 ("mm/damon/vaddr: use damos->migrate_dests in migrate_{hot,cold}")

Should we Cc: stable?


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[CharSyam]

Hi, SJ.

Thanks for your reply.

1. I found this by code inspection, not from a real-world bug report.
I consider it a correctness issue for valid inputs.
2. I would prefer a u64 total over rejecting those inputs, since the
interface does not document a limit on the sum of weights.
3. Given the lack of a reported real-world impact, I do not think Cc:
stable is needed.

Thanks.
Daemyung

2026년 10월 27일 (화) 오후 10:54, SJ Park <sj@kernel.org>님이 작성:
>
> Hello Daemyung,
>
> On Tue, 27 Oct 2026 12:54:31 +0900 Daemyung Kang <charsyam@gmail.com> wrote:
>
> > DAMOS migration accepts an unsigned int weight for each destination,
> > but sums the weights into an unsigned int.  If the total wraps to zero,
> > no folio is migrated.  Other wrapped totals select the wrong node.
>
> Thank you for finding the issue.
>
> But, is it realistic to have the weight_total overflow?  Have you found the
> issue on real usage?  Or, theoretical?
>
> >
> > Sum weights into a u64 once per migration walk.  Keep the existing
> > 32-bit divisor path for totals up to UINT_MAX and use a 64-bit remainder
> > for larger totals.  Add KUnit coverage for the actual summing path and
> > for destination selection at the overflow boundary.
>
> Do we really need to support that user inputs?  Unless we have a very real use
> case that require such huge weight values, I'd prefer not.
>
> How about just clearly notify users those are invalid parameters?  We could do
> so by adding weight_total overflow check to the central parameters validation
> path: damos_commit_dests().
>
> >
> > Fixes: 19c1dc15c859 ("mm/damon/vaddr: use damos->migrate_dests in migrate_{hot,cold}")
>
> Should we Cc: stable?
>
>
> Thanks,
> SJ
>
> [...]

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[SJ Park]

Apparently Bijan's micron email is no more working.  I added Bijan's gmail to
Cc list.

On Wed, 28 Oct 2026 08:57:06 +0900 CharSyam <charsyam@gmail.com> wrote:

> Hi, SJ.
> 
> Thanks for your reply.

You're welcome!

> 
> 1. I found this by code inspection, not from a real-world bug report.
> I consider it a correctness issue for valid inputs.

Thank you for clarifying.

> 2. I would prefer a u64 total over rejecting those inputs, since the
> interface does not document a limit on the sum of weights.

But I doubt the value of documenting it.  I wonder if real users will find the
behavior at all.  I think it would be not too late to consider documenting it
after we get an issue report from a real world user.

Even just keeping the current behavior looks not really bad to me, as long as
it is easy to test and only shoot the foot of users who didn't run test for
their weird inputs.  We can revisit after a real issue report is delivered.

What do you think?

> 3. Given the lack of a reported real-world impact, I do not think Cc:
> stable is needed.

Makes sense, I agree.

Please avoid top-posting [1] from the next time.

[1] https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[CharSyam]

Hi, SJ

> Even just keeping the current behavior looks not really bad to me, as long as
> it is easy to test and only shoot the foot of users who didn't run test for
> their weird inputs.  We can revisit after a real issue report is delivered.
>
> What do you think?

 I agree. This is a correctness issue, but I am not aware of any
real-world impact,
so I do not think it needs to be fixed now. We can revisit it if a
user reports a problem.

> Please avoid top-posting [1] from the next time.

Sorry about that. I will reply inline from now on.

Thanks,
Daemyung

Re: [PATCH] mm/damon/vaddr: preserve migration weights above UINT_MAX

[SJ Park]

On Wed, 28 Oct 2026 23:44:28 +0900 CharSyam <charsyam@gmail.com> wrote:

> Hi, SJ
> 
> > Even just keeping the current behavior looks not really bad to me, as long as
> > it is easy to test and only shoot the foot of users who didn't run test for
> > their weird inputs.  We can revisit after a real issue report is delivered.
> >
> > What do you think?
> 
>  I agree. This is a correctness issue, but I am not aware of any
> real-world impact,
> so I do not think it needs to be fixed now. We can revisit it if a
> user reports a problem.

I agree, let's do so.  I will consider adding the input validity check with low
priority, since it might help us getting a real issue report sooner.

> 
> > Please avoid top-posting [1] from the next time.
> 
> Sorry about that. I will reply inline from now on.

No worries!


Thanks,
SJ

[...]