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

[PATCH v2 0/3] 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 bounds the conversion.  Patch 2 pins the boundary in the core kunit
suite, where it fails without patch 1 on any word size.  Patch 3 documents
the ceiling that remains once the conversion is bounded: the basis-point
form cannot represent a size quota above ULONG_MAX / 10000, so a larger one
is handled as if no size quota were set.

Changes since v1
(https://lore.kernel.org/damon/20260919071324.1583280-1-donggeunyoo.kernel@gmail.com/):
- patch 1: bound the existing multiply arm rather than reorder the two
  arms, per SJ Park's review.  Same behavior, one changed line.
- patch 1: address addr_unit -- it mitigates but does not solve this, and
  the 64-bit boundary is reachable without any scaling.
- patch 2: build the scheme and the goal with damon_new_scheme(),
  damos_new_quota_goal() and damos_add_quota_goal() instead of a stack
  damos and an open-coded list_add(); drop the max_sz local.
- patch 3: new, also suggested by SJ Park.

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

Donggeun Yoo (3):
  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
  Docs/admin-guide/mm/damon/usage: document the temporal tuner's quota
    limit

 Documentation/admin-guide/mm/damon/usage.rst |  7 +++
 mm/damon/core.c                              |  2 +-
 mm/damon/tests/core-kunit.h                  | 48 ++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)


base-commit: 498ee28e5ec4727f829507c4a1bde3ab1b7704cd
-- 
2.53.0

[PATCH v2 1/3] 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.

While the addr_unit parameter effectively mitigates the overflow risk by
scaling down the values written to quotas/bytes, it does not fundamentally
solve the issue.  Theoretically, an overflow can still occur if the scaled
value is exceptionally large.  Furthermore, because addr_unit is exclusive
to the paddr operations set, vaddr and fvaddr contexts remain fully
exposed to this overflow since they take unscaled raw byte values.

The 64-bit boundary is reachable without any scaling: bytes_store() takes
whatever kstrtoul() parses, so a quotas/bytes above 1844674407370955 wraps
the multiply there too.

Bound the conversion, so a size quota it cannot represent falls to the
ULONG_MAX the 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.  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:1970
  Expected s->quota.esz == (~0UL) / 10000, but
      s->quota.esz == 0 (0x0)
  # damos_test_esz_goal_temporal: EXPECTATION FAILED at mm/damon/tests/core-kunit.h:1974
  Expected s->quota.esz == (~0UL) / 10000, but
      s->quota.esz == 1844674407370954 (0x68db8bac710ca)

 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 2258b72da7a78..16d4145379a2b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3274,7 +3274,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;
-- 
2.53.0

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

[SJ Park]

On Sun, 20 Sep 2026 11:31:09 +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,

Let's break commit message lines with 72 columns limit.

> 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.

Why 256 MiB, not 429,496 bytes?

> 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.

So, the way to work around is updating the size quota to smaller value,
correct?

> Other sizes are wrong without being zero: 500000 yields 70503.
> 
> While the addr_unit parameter effectively mitigates the overflow risk by
> scaling down the values written to quotas/bytes, it does not fundamentally
> solve the issue.  Theoretically, an overflow can still occur if the scaled
> value is exceptionally large.  Furthermore, because addr_unit is exclusive
> to the paddr operations set, vaddr and fvaddr contexts remain fully
> exposed to this overflow since they take unscaled raw byte values.
> 
> The 64-bit boundary is reachable without any scaling: bytes_store() takes
> whatever kstrtoul() parses, so a quotas/bytes above 1844674407370955 wraps
> the multiply there too.

But why a sane user would set such huge number?

Because this patch Cc stable@, let's make super clear about the user impact so
that people don't unnecessarily be scared.  Please mention when the issue can
happen.  It can happen only in certain user setups that probably untested.
Since the consequence is easy to detect (scheme makes no progress always), the
setup is likely untested.  Please also mention it does not cause critical
problems like crashes or leaks, and can be easily worked around by updating the
size quota.

> 
> Bound the conversion, so a size quota it cannot represent falls to the
> ULONG_MAX the function already writes for a scheme with no size quota.

I don't understand the above sentence.  Is the grammar correct?

> 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.  On
> 32-bit a large size quota then behaves like no size quota rather than like
> a dead scheme.

I don't quite understand above.  could you please elaborate?

> 
> 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:1970
>   Expected s->quota.esz == (~0UL) / 10000, but
>       s->quota.esz == 0 (0x0)
>   # damos_test_esz_goal_temporal: EXPECTATION FAILED at mm/damon/tests/core-kunit.h:1974
>   Expected s->quota.esz == (~0UL) / 10000, but
>       s->quota.esz == 1844674407370954 (0x68db8bac710ca)
> 
>  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 2258b72da7a78..16d4145379a2b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3274,7 +3274,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;

Code change looks good to me.


Thanks,
SJ

[...]

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

[Donggeun Yoo]

Thank you for the detailed review.

On Sun, 20 Sep 2026 03:37:13 -0700 SJ Park <sj@kernel.org> wrote:
> Let's break commit message lines with 72 columns limit.

Done in v3, here and in 2/3.

> Why 256 MiB, not 429,496 bytes?

They are two different numbers.  429496 is where the multiply starts
to wrap, and every size above it is wrong.  256 MiB is where the
wrapped value lands on exactly zero: 256 MiB * 10000 is 625 * 2^32, so
on 32-bit the product wraps around 625 times and ends at zero.

> So, the way to work around is updating the size quota to smaller value,
> correct?

Correct.  v3 says so.

> But why a sane user would set such huge number?

They would not, which is why v3 no longer argues the 64-bit case in its
own paragraph.  The threshold is now stated once, as part of what it
takes to reach the bug: above ULONG_MAX / 10000, which is 429496 bytes
on 32-bit and 1844674407370955 on 64-bit.

> Because this patch Cc stable@, let's make super clear about the user
> impact [...]

v3:

  Triggering this needs a scheme with a quota goal, the temporal goal
  tuner, and a size quota above ULONG_MAX / 10000 -- 429496 bytes on
  32-bit, 1844674407370955 on 64-bit -- so it is unlikely to be hit on
  a tested setup.  Nothing is corrupted and nothing leaks.  The scheme
  makes no progress for as long as the goal is unachieved, which is
  easy to notice, and writing a smaller size quota restores it.

> > Bound the conversion, so a size quota it cannot represent falls to the
> > ULONG_MAX the function already writes for a scheme with no size quota.
>
> I don't understand the above sentence.  Is the grammar correct?

You are right that it is hard to read.  The sentence was too long and
tried to say two things at once.  v3:

  Bound the multiply.  A size quota too large to convert now takes the
  same ULONG_MAX branch as a scheme with no size quota, so the
  effective quota becomes ULONG_MAX / 10000 instead of a wrapped value.

> > Widening esz_bp instead would reach the consist tuner [...]
>
> I don't quite understand above.  could you please elaborate?

Sorry, that paragraph was not clear.  It was meant to explain why I did
not simply widen the type.

The other way to fix the overflow is to make esz_bp wider than unsigned
long, u64 for example.  But esz_bp is not used only here.  The consist
tuner keeps its own value in the same field, and
damos_goal_tune_esz_bp_consist() passes it to
damon_feed_loop_next_input(), which takes unsigned long and returns
unsigned long.  So widening esz_bp means widening that function too,
and the consist tuner would change for a problem it does not have.
Bounding the multiply is one line, and only the temporal tuner is
touched.

I dropped the paragraph in v3.  It argues against a fix nobody
proposed, so it only makes the changelog harder to read.

Thanks,
Donggeun

[PATCH v2 2/3] 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 | 48 +++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 5ff0436c58441..a0604788bc638 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1929,6 +1929,53 @@ static void damon_test_rand(struct kunit *test)
 	}
 }
 
+static void damos_test_esz_goal_temporal(struct kunit *test)
+{
+	struct damos_access_pattern pattern = {};
+	struct damos_watermarks wmarks = {};
+	struct damos_quota quota = {
+		.goal_tuner = DAMOS_QUOTA_GOAL_TUNER_TEMPORAL,
+	};
+	struct damos_quota_goal *goal;
+	struct damon_ctx *ctx;
+	struct damos *s;
+
+	ctx = damon_new_ctx();
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+
+	s = damon_new_scheme(&pattern, DAMOS_STAT, 0, &quota, &wmarks,
+			NUMA_NO_NODE);
+	if (!s) {
+		damon_destroy_ctx(ctx);
+		kunit_skip(test, "scheme alloc fail");
+	}
+	damon_add_scheme(ctx, s);
+
+	goal = damos_new_quota_goal(DAMOS_QUOTA_USER_INPUT, 10000);
+	if (!goal) {
+		damon_destroy_ctx(ctx);
+		kunit_skip(test, "quota goal alloc fail");
+	}
+	goal->current_value = 0;
+	damos_add_quota_goal(&s->quota, goal);
+
+	/* The largest size quota the basis-point conversion can hold. */
+	s->quota.sz = ULONG_MAX / 10000;
+	damos_set_effective_quota(ctx, s);
+	KUNIT_EXPECT_EQ(test, s->quota.esz, ULONG_MAX / 10000);
+
+	/* Any larger one saturates instead of wrapping. */
+	s->quota.sz = ULONG_MAX / 10000 + 1;
+	damos_set_effective_quota(ctx, s);
+	KUNIT_EXPECT_EQ(test, s->quota.esz, ULONG_MAX / 10000);
+
+	s->quota.sz = ULONG_MAX;
+	damos_set_effective_quota(ctx, s);
+	KUNIT_EXPECT_EQ(test, s->quota.esz, ULONG_MAX / 10000);
+
+	damon_destroy_ctx(ctx);
+}
+
 static struct kunit_case damon_test_cases[] = {
 	KUNIT_CASE(damon_test_target),
 	KUNIT_CASE(damon_test_regions),
@@ -1965,6 +2012,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 v2 2/3] mm/damon/tests/core-kunit: test the temporal tuner's size quota conversion

[SJ Park]

On Sun, 20 Sep 2026 11:31:10 +0900 Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

> damos_goal_tune_esz_bp_temporal() encodes the size quota in basis points,

Please wrap commit message lines with 72 columns limit.

> 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>

Other than the above column limit,

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


Thanks,
SJ

[...]

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

[Donggeun Yoo]

Thank you for the review.

On Sun, 20 Sep 2026 03:40:53 -0700 SJ Park <sj@kernel.org> wrote:
> Please wrap commit message lines with 72 columns limit.

Done in v3.

> Other than the above column limit,
>
> Reviewed-by: SJ Park <sj@kernel.org>

Thanks, added.  The code is unchanged in v3.

Thanks,
Donggeun

[PATCH v2 3/3] Docs/admin-guide/mm/damon/usage: document the temporal tuner's quota limit

[Donggeun Yoo]

The temporal goal tuner converts the size quota into basis points in an
unsigned long, so a quotas/bytes above ULONG_MAX / 10000 does not fit,
and is handled as if no size quota were set.  On 32-bit that is 429496
bytes, well below the "echo $((1024*1024*1024)) > quotas/bytes" this
document instructs.  addr_unit does not raise the limit, though a larger
unit does make it harder to reach.

Note both in the goal_tuner description.

Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
 Documentation/admin-guide/mm/damon/usage.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
index ba47255448564..ea7443b307826 100644
--- a/Documentation/admin-guide/mm/damon/usage.rst
+++ b/Documentation/admin-guide/mm/damon/usage.rst
@@ -455,6 +455,13 @@ the background design of the feature and the name of the selectable algorithms.
 Refer to :ref:`goals directory <sysfs_schemes_quota_goals>` for the goals
 setup.
 
+The ``temporal`` algorithm converts the size quota into basis points, so it
+cannot represent a ``bytes`` value larger than 429496 on 32-bit kernels, or
+1844674407370955 on 64-bit ones.  Larger values are handled as if no size
+quota were set.  :ref:`addr_unit <damon_design_addr_unit>` does not raise the
+limit, but it does make the limit harder to reach: with a larger unit, the
+same amount of memory is written to ``bytes`` as a smaller number.
+
 You can set the action-failed memory quota charging ratio by writing the
 numerator and the denominator for the ratio to ``fail_charge_num`` and
 ``fail_charge_denom`` files, respectively.  Reading those files will return the
-- 
2.53.0

Re: [PATCH v2 3/3] Docs/admin-guide/mm/damon/usage: document the temporal tuner's quota limit

[SJ Park]

On Sun, 20 Sep 2026 11:31:11 +0900 Donggeun Yoo <donggeunyoo.kernel@gmail.com> wrote:

> The temporal goal tuner converts the size quota into basis points in an
> unsigned long, so a quotas/bytes above ULONG_MAX / 10000 does not fit,
> and is handled as if no size quota were set.  On 32-bit that is 429496
> bytes, well below the "echo $((1024*1024*1024)) > quotas/bytes" this
> document instructs.  addr_unit does not raise the limit, though a larger
> unit does make it harder to reach.
> 
> Note both in the goal_tuner description.
> 
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
>  Documentation/admin-guide/mm/damon/usage.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
> index ba47255448564..ea7443b307826 100644
> --- a/Documentation/admin-guide/mm/damon/usage.rst
> +++ b/Documentation/admin-guide/mm/damon/usage.rst
> @@ -455,6 +455,13 @@ the background design of the feature and the name of the selectable algorithms.
>  Refer to :ref:`goals directory <sysfs_schemes_quota_goals>` for the goals
>  setup.
>  
> +The ``temporal`` algorithm converts the size quota into basis points, so it
> +cannot represent a ``bytes`` value larger than 429496 on 32-bit kernels, or
> +1844674407370955 on 64-bit ones.

This feels like too much implementation details.

> Larger values are handled as if no size
> +quota were set.

I don't get what this means.

> :ref:`addr_unit <damon_design_addr_unit>` does not raise the
> +limit, but it does make the limit harder to reach: with a larger unit, the
> +same amount of memory is written to ``bytes`` as a smaller number.

I suggested it might make sense to document this problem, but now I doubt if
this is really helpful for users.  Given the fact that the issue can happen on
only untested setups, I now think this documentation change is better not to be
made.  Correct me if I'm wrong.  If I'm not incorrect, could we drop this
patch?


Thanks,
SJ

[...]

Re: [PATCH v2 3/3] Docs/admin-guide/mm/damon/usage: document the temporal tuner's quota limit

[Donggeun Yoo]

Thank you for the review.

On Sun, 20 Sep 2026 03:49:20 -0700 SJ Park <sj@kernel.org> wrote:
> I now think this documentation change is better not to be made.
> Correct me if I'm wrong.  If I'm not incorrect, could we drop this
> patch?

You are not wrong, and I agree.  Dropped in v3.

Thanks,
Donggeun