[PATCH 0/2] mm/damon: fix the temporal goal tuner's size quota conversion

[PATCH 0/2] mm/damon: fix the temporal goal tuner's size quota conversion

[Donggeun Yoo]

damos_goal_tune_esz_bp_temporal() hands the size quota to
damos_set_effective_quota() through quota->esz_bp in basis points, and the
multiply that gets it there is unchecked.  On 32-bit it wraps from 429497
bytes on, and wraps to exactly zero at the 1 GiB that
Documentation/admin-guide/mm/damon/usage.rst uses as its example, since
1 GiB * 10000 is 2500 * 2^32.  A zero effective quota makes
damos_quota_is_full() true on the first test of every charge window, so the
scheme applies nothing for as long as the goal is unachieved.

Patch 1 saturates the conversion.  Patch 2 pins the boundary in the core
kunit suite, where it fails without patch 1 on any word size.

DAMON kunit: 36 tests, all passing with both patches; 35 passing and
damos_test_esz_goal_temporal failing with patch 2 alone.  The DAMON
selftests do not reach the changed function: a build with a print at its
entry stayed silent through all fifteen of them and fired as soon as a
scheme was configured with the temporal tuner.

Donggeun Yoo (2):
  mm/damon/core: prevent size quota overflow in the temporal goal tuner
  mm/damon/tests/core-kunit: test the temporal tuner's size quota
    conversion

 mm/damon/core.c             |  6 +++---
 mm/damon/tests/core-kunit.h | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)


base-commit: 498ee28e5ec4727f829507c4a1bde3ab1b7704cd
-- 
2.53.0

[PATCH v1 1/2] mm/damon/core: prevent size quota overflow in the temporal goal tuner

[Donggeun Yoo]

damos_goal_tune_esz_bp_temporal() converts the scheme's size quota into
basis points with "quota->esz_bp = quota->sz * 10000", both unsigned long,
and damos_set_effective_quota() divides the result back by 10000.
quotas/bytes is unbounded; bytes_store() hands it to kstrtoul() as is.

On 32-bit the product wraps for any size quota above ULONG_MAX / 10000,
that is 429496 bytes.  Documentation/admin-guide/mm/damon/usage.rst
instructs "echo $((1024*1024*1024)) > quotas/bytes", and 1 GiB * 10000 is
2500 * 2^32, so that documented value wraps to exactly zero; 256 MiB and
every multiple of it do the same.  quota->esz then becomes zero while the
goal is not achieved, the trailing "if (quota->sz && quota->sz < esz)" can
only lower esz further, and damos_quota_is_full() is true on the first test
of every charge window, so the scheme applies nothing and the goal is never
approached.  Other sizes are wrong without being zero: 500000 yields 70503.

Saturate to ULONG_MAX, which is what the same function already writes for a
scheme with no size quota.  Widening esz_bp instead would reach the consist
tuner, which runs the same field through damon_feed_loop_next_input(),
unsigned long in and out; bounding the multiply keeps the change to this
branch.  On 32-bit a large size quota then behaves like no size quota
rather than like a dead scheme.

Fixes: af738a6a00c1 ("mm/damon/core: introduce DAMOS_QUOTA_GOAL_TUNER_TEMPORAL")
Cc: <stable@vger.kernel.org> # 7.1.x
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
Measured on i386 under QEMU: one paddr context with a stat scheme, the
temporal goal tuner, and one unachieved user_input goal.  Each size is
written to quotas/bytes, the kdamond is started, and
quotas/effective_bytes is read back after
update_schemes_effective_quotas.

  quotas/bytes    effective_bytes    effective_bytes
                  before             after
  4096            4096               4096
  429496          429496             429496
  429497          0                  429496
  268435456       0                  429496
  1073741824      0                  429496
  500000          70503              429496
  4294967295      429495             429496
  0               429496             429496

Everything the conversion can hold is unchanged, and 429496 is what the
no-size-quota row already produced before the patch.

Patch 2 pins the same boundary at ULONG_MAX / 10000 and so runs on any
word size.  Without this patch it fails on x86_64:

  # damos_test_esz_goal_temporal: EXPECTATION FAILED at mm/damon/tests/core-kunit.h:1959
  Expected s.quota.esz == max_sz, but
      s.quota.esz == 0 (0x0)
      max_sz == 1844674407370955 (0x68db8bac710cb)

 mm/damon/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 2258b72da7a7..5ec476cef4db 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3274,10 +3274,10 @@ static void damos_goal_tune_esz_bp_temporal(struct damon_ctx *c,
 
 	if (score >= 10000)
 		quota->esz_bp = 0;
-	else if (quota->sz)
-		quota->esz_bp = quota->sz * 10000;
-	else
+	else if (!quota->sz || quota->sz > ULONG_MAX / 10000)
 		quota->esz_bp = ULONG_MAX;
+	else
+		quota->esz_bp = quota->sz * 10000;
 }
 
 /*
-- 
2.53.0

[PATCH v1 2/2] mm/damon/tests/core-kunit: test the temporal tuner's size quota conversion

[Donggeun Yoo]

damos_goal_tune_esz_bp_temporal() encodes the size quota in basis points,
so the conversion is exact only up to ULONG_MAX / 10000.  Pin the three
sizes around that boundary: the largest one that fits, the first one that
does not, and ULONG_MAX.

Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
 mm/damon/tests/core-kunit.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 5ff0436c5844..4460dce0352c 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1929,6 +1929,40 @@ static void damon_test_rand(struct kunit *test)
 	}
 }
 
+static void damos_test_esz_goal_temporal(struct kunit *test)
+{
+	unsigned long max_sz = ULONG_MAX / 10000;
+	struct damos_quota_goal goal = {
+		.metric = DAMOS_QUOTA_USER_INPUT,
+		.target_value = 10000,
+		.current_value = 0,
+	};
+	struct damon_ctx *ctx;
+	struct damos s;
+
+	ctx = damon_new_ctx();
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+
+	memset(&s, 0, sizeof(s));
+	INIT_LIST_HEAD(&s.quota.goals);
+	list_add(&goal.list, &s.quota.goals);
+	s.quota.goal_tuner = DAMOS_QUOTA_GOAL_TUNER_TEMPORAL;
+
+	s.quota.sz = max_sz;
+	damos_set_effective_quota(ctx, &s);
+	KUNIT_EXPECT_EQ(test, s.quota.esz, max_sz);
+
+	s.quota.sz = max_sz + 1;
+	damos_set_effective_quota(ctx, &s);
+	KUNIT_EXPECT_EQ(test, s.quota.esz, max_sz);
+
+	s.quota.sz = ULONG_MAX;
+	damos_set_effective_quota(ctx, &s);
+	KUNIT_EXPECT_EQ(test, s.quota.esz, max_sz);
+
+	damon_destroy_ctx(ctx);
+}
+
 static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_target),
 	KUNIT_CASE(damon_test_regions),
@@ -1965,6 +1999,7 @@ static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_is_last_region),
 	KUNIT_CASE(damon_test_walk_control_obsolete),
 	KUNIT_CASE(damon_test_rand),
+	KUNIT_CASE(damos_test_esz_goal_temporal),
 	{},
 };
 
-- 
2.53.0

Re: [PATCH v1 1/2] mm/damon/core: prevent size quota overflow in the temporal goal tuner

[SJ Park]

Hi Donggeun,


Thank you for this patch.

On Sat, 19 Sep 2026 16:13:23 +0900 Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

> damos_goal_tune_esz_bp_temporal() converts the scheme's size quota into
> basis points with "quota->esz_bp = quota->sz * 10000", both unsigned long,
> and damos_set_effective_quota() divides the result back by 10000.
> quotas/bytes is unbounded; bytes_store() hands it to kstrtoul() as is.
> 
> On 32-bit the product wraps for any size quota above ULONG_MAX / 10000,
> that is 429496 bytes.  Documentation/admin-guide/mm/damon/usage.rst
> instructs "echo $((1024*1024*1024)) > quotas/bytes", and 1 GiB * 10000 is
> 2500 * 2^32, so that documented value wraps to exactly zero; 256 MiB and
> every multiple of it do the same.  quota->esz then becomes zero while the
> goal is not achieved, the trailing "if (quota->sz && quota->sz < esz)" can
> only lower esz further, and damos_quota_is_full() is true on the first test
> of every charge window, so the scheme applies nothing and the goal is never
> approached.  Other sizes are wrong without being zero: 500000 yields 70503.

For 32-bit machines, we have addr_unit parameter.  I believe use of it could
effectively solve this kind of issues.  Correct me if I'm wrong.

There could be cases that addr_unit cannot help, though.  Particularly, if I
remember correctly, 'addr_unit' works for only paddr.  Also it doesn't fix all
theoretical corner cases.  Even on 64 bit machines, same problem exists in
theory.  So I think this change is worthy to have.  But I think it is better to
mention existence of addr_unit and why it is not the perfect solution in the
commit message.

Maybe it is worthy to add the comment on the user documents, too.

> 
> Saturate to ULONG_MAX, which is what the same function already writes for a
> scheme with no size quota.  Widening esz_bp instead would reach the consist
> tuner, which runs the same field through damon_feed_loop_next_input(),
> unsigned long in and out; bounding the multiply keeps the change to this
> branch.  On 32-bit a large size quota then behaves like no size quota
> rather than like a dead scheme.
> 
> Fixes: af738a6a00c1 ("mm/damon/core: introduce DAMOS_QUOTA_GOAL_TUNER_TEMPORAL")
> Cc: <stable@vger.kernel.org> # 7.1.x
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> Measured on i386 under QEMU: one paddr context with a stat scheme, the
> temporal goal tuner, and one unachieved user_input goal.  Each size is
> written to quotas/bytes, the kdamond is started, and
> quotas/effective_bytes is read back after
> update_schemes_effective_quotas.
> 
>   quotas/bytes    effective_bytes    effective_bytes
>                   before             after
>   4096            4096               4096
>   429496          429496             429496
>   429497          0                  429496
>   268435456       0                  429496
>   1073741824      0                  429496
>   500000          70503              429496
>   4294967295      429495             429496
>   0               429496             429496
> 
> Everything the conversion can hold is unchanged, and 429496 is what the
> no-size-quota row already produced before the patch.
> 
> Patch 2 pins the same boundary at ULONG_MAX / 10000 and so runs on any
> word size.  Without this patch it fails on x86_64:
> 
>   # damos_test_esz_goal_temporal: EXPECTATION FAILED at mm/damon/tests/core-kunit.h:1959
>   Expected s.quota.esz == max_sz, but
>       s.quota.esz == 0 (0x0)
>       max_sz == 1844674407370955 (0x68db8bac710cb)
> 
>  mm/damon/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 2258b72da7a7..5ec476cef4db 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3274,10 +3274,10 @@ static void damos_goal_tune_esz_bp_temporal(struct damon_ctx *c,
>  
>  	if (score >= 10000)
>  		quota->esz_bp = 0;
> -	else if (quota->sz)
> -		quota->esz_bp = quota->sz * 10000;
> -	else
> +	else if (!quota->sz || quota->sz > ULONG_MAX / 10000)
>  		quota->esz_bp = ULONG_MAX;
> +	else
> +		quota->esz_bp = quota->sz * 10000;
>  }

In my humble opinion, this could be easier to read in below way:

'''
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3524,7 +3524,7 @@ static void damos_goal_tune_esz_bp_temporal(struct damon_ctx *c,

        if (score >= 10000)
                quota->esz_bp = 0;
-       else if (quota->sz)
+       else if (quota->sz && quota->sz <= ULONG_MAX / 10000)
                quota->esz_bp = quota->sz * 10000;
        else
                quota->esz_bp = ULONG_MAX;
'''

What do you think?


Thanks,
SJ

[...]

Re: [PATCH v1 2/2] mm/damon/tests/core-kunit: test the temporal tuner's size quota conversion

[SJ Park]

On Sat, 19 Sep 2026 16:13:24 +0900 Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

> damos_goal_tune_esz_bp_temporal() encodes the size quota in basis points,
> so the conversion is exact only up to ULONG_MAX / 10000.  Pin the three
> sizes around that boundary: the largest one that fits, the first one that
> does not, and ULONG_MAX.
> 
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
>  mm/damon/tests/core-kunit.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 5ff0436c5844..4460dce0352c 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -1929,6 +1929,40 @@ static void damon_test_rand(struct kunit *test)
>  	}
>  }
>  
> +static void damos_test_esz_goal_temporal(struct kunit *test)
> +{
> +	unsigned long max_sz = ULONG_MAX / 10000;
> +	struct damos_quota_goal goal = {
> +		.metric = DAMOS_QUOTA_USER_INPUT,
> +		.target_value = 10000,
> +		.current_value = 0,
> +	};

Let's use damon_new_scheme() and damos_new_quota_goal() unless it makes code
too complicated.

> +	struct damon_ctx *ctx;
> +	struct damos s;
> +
> +	ctx = damon_new_ctx();
> +	KUNIT_ASSERT_NOT_NULL(test, ctx);
> +
> +	memset(&s, 0, sizeof(s));

> +	INIT_LIST_HEAD(&s.quota.goals);
> +	list_add(&goal.list, &s.quota.goals);

Let's use existing helper, damos_add_quota_goal().

> +	s.quota.goal_tuner = DAMOS_QUOTA_GOAL_TUNER_TEMPORAL;
> +
> +	s.quota.sz = max_sz;

Meaning of 'max_sz' is bit confusing.  Why don't you use 'ULONG_MAX / 10000'
here?

[...]


Other than above, looks good to me.


Thanks,
SJ

Re: [PATCH v1 1/2] mm/damon/core: prevent size quota overflow in the temporal goal tuner

[Donggeun Yoo]

Hi SJ,

Thank you for the review, and for pointing me at addr_unit -- I did not
know about it.

On Sat, 19 Sep 2026 09:55:08 -0700 SJ Park <sj@kernel.org> wrote:
> For 32-bit machines, we have addr_unit parameter.  I believe use of it could
> effectively solve this kind of issues.  Correct me if I'm wrong.

You are right that it helps, and quite a lot: with a larger unit the same
amount of memory is written to quotas/bytes as a smaller number, so the
boundary is much harder to reach in practice.  It does not move the
boundary itself, though, since the multiply is on the value written
whatever the unit means.  And as you say, only paddr implements addr_unit,
so vaddr and fvaddr get nothing from it.

The 64-bit case turned out to be a little more than theoretical too.
bytes_store() takes whatever kstrtoul() parses, so a quotas/bytes above
1844674407370955 wraps there with no scaling involved.

v2 says both in the changelog, and your suggestion about the user
documents became a third patch.  I also took your form of the guard; it is
a one-line diff and it reads better.

  https://lore.kernel.org/damon/20260920023111.2466265-1-donggeunyoo.kernel@gmail.com/

Thanks,
Donggeun

Re: [PATCH v1 2/2] mm/damon/tests/core-kunit: test the temporal tuner's size quota conversion

[Donggeun Yoo]

Hi SJ,

Thank you.  All three applied in v2: damon_new_scheme() for the scheme,
damos_new_quota_goal() with damos_add_quota_goal() for the goal, and
ULONG_MAX / 10000 written out in place of max_sz -- the name really was
confusing.

  https://lore.kernel.org/damon/20260920023111.2466265-3-donggeunyoo.kernel@gmail.com/

Thanks,
Donggeun