mm/damon/core: allow esz to be set to zero

PATCH In review Liew Rui Yan 3 participants 5 messages Sep 5

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

Liew Rui Yan

author

2026-09-05 12:47 UTC #0 lore raw

PATCH v3 1/1 mm/damon/core: allow esz to be set to zero

When the temporal quota goal tuner determines that the goal has been achieved (score >= 10000), it sets esz_bp to zero so that the esz becomes zero. However, damos_set_effective_quota() clamps the esz to min_region_sz when quota->ms is set.

This is a minor issue, the main problem is that it doesn't match the description in the documentation, which state that if the goal has already been [over-]achieved, the quota will be set to zero.

Fix this by set quota (esz) as minimum as possible.

Fixes 8bbde987c2b8 ("mm/damon/core: disallow time-quota setting zero esz")

Cc <[email protected]> # v7.1.x

Signed-off-by Liew Rui Yan <[email protected]>

---

Changes from v2:
- v2: https://lore.kernel.org/damon/[email protected]
- Changed implementation, more general implementation provided by SJ. [1]

[1] https://lore.kernel.org/damon/[email protected]

Changes from v1:
- v1: https://lore.kernel.org/damon/[email protected]
- Changed implementation. The initial solution skipped the min_region_sz clamping when esz was already zero. However, this would revert the fix from commit 8bbde987c2b8 and cause a regression. This revision fixes the issue by adding a specific check for a zero esz_bp.
- Changed patch title, original title: fix quota could not be set to zero

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

mm/damon/core.c +3 -2

index 644daf5a1656..2b294fb46648 100644
@@ -3091,6 +3091,7 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 	struct damos_quota *quota = &s->quota;
 	unsigned long throughput;
 	unsigned long esz = ULONG_MAX;
+	unsigned long esz_time;
 
 	if (!quota->ms && list_empty(&quota->goals)) {
 		quota->esz = quota->sz;

@@ -3111,8 +3112,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 					1000000, quota->total_charged_ns);
 		else
 			throughput = PAGE_SIZE * 1024;
-		esz = min(throughput * quota->ms, esz);
-		esz = max(ctx->min_region_sz, esz);
+		esz_time = max(throughput * quota->ms, ctx->min_region_sz);
+		esz = min(esz_time, esz);
 	}
 
 	if (quota->sz && quota->sz < esz)

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-05 12:58 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures. --

commit dc1c35769ce920ee1796367097f9876b2f24a643
Author: Liew Rui Yan <[email protected]>

mm/damon/core: allow esz to be set to zero

This commit modifies damos_set_effective_quota() to allow the effective size to be set to zero when the goal has been achieved. It changes the order of max and min calculations when quota->ms is set to ensure that a zero esz_bp results in a minimal effective size instead of being clamped to min_region_sz.

Quoted 4 lines

mm/damon/core.c +0 -0

index ce8c6f99106ed..3e9c9b6385d4e 100644

[ ... ]

mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3260

Quoted 2 lines

mm/damon/core.c +0 -0

@@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 					1000000, quota->total_charged_ns);

[Severity: High]
This is a pre-existing issue, but does mult_frac() silently overflow on 32-bit architectures here in damos_set_effective_quota()?

Since quota->total_charged_ns is represented as an unsigned long, it natively overflows after 4.29 seconds on 32-bit systems. Furthermore, when calculating throughput via mult_frac(), the remainder calculation (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which causes throughput estimation to be completely incorrect and often near zero.

mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3115

Quoted 5 lines

mm/damon/core.c +1 -2

 		else
 			throughput = PAGE_SIZE * 1024;
-		esz = min(throughput * quota->ms, esz);
-		esz = max(ctx->min_region_sz, esz);
+		esz_time = max(throughput * quota->ms, ctx->min_region_sz);

[Severity: High]
This is also a pre-existing issue, but can throughput * quota->ms overflow 32 bits for quotas over a few seconds here in damos_set_effective_quota()?

If this wraps around on 32-bit architectures, it improperly clamps the effective size to ctx->min_region_sz, nullifying the time quota feature.

Quoted 4 lines

mm/damon/core.c +1 -0

+		esz = min(esz_time, esz);
 	}
 
 	if (quota->sz && quota->sz < esz)

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Liew Rui Yan

author

2026-09-05 14:14 UTC #2 lore raw

Quoted 47 lines · On Sat, 05 Sep 2026 12:58:25 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures. --
commit dc1c35769ce920ee1796367097f9876b2f24a643
Author: Liew Rui Yan <[email protected]>
mm/damon/core: allow esz to be set to zero
This commit modifies damos_set_effective_quota() to allow the effective size to be set to zero when the goal has been achieved. It changes the order of max and min calculations when quota->ms is set to ensure that a zero esz_bp results in a minimal effective size instead of being clamped to min_region_sz.
Quoted 4 lines
mm/damon/core.c +0 -0
index ce8c6f99106ed..3e9c9b6385d4e 100644
[ ... ]
mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3260
Quoted 2 lines
mm/damon/core.c +0 -0
@@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 					1000000, quota->total_charged_ns);
[Severity: High]
This is a pre-existing issue, but does mult_frac() silently overflow on 32-bit architectures here in damos_set_effective_quota()?
Since quota->total_charged_ns is represented as an unsigned long, it natively overflows after 4.29 seconds on 32-bit systems. Furthermore, when calculating throughput via mult_frac(), the remainder calculation (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which causes throughput estimation to be completely incorrect and often near zero.
mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3115
Quoted 5 lines
mm/damon/core.c +1 -2
 		else
 			throughput = PAGE_SIZE * 1024;
-		esz = min(throughput * quota->ms, esz);
-		esz = max(ctx->min_region_sz, esz);
+		esz_time = max(throughput * quota->ms, ctx->min_region_sz);
[Severity: High]
This is also a pre-existing issue, but can throughput * quota->ms overflow 32 bits for quotas over a few seconds here in damos_set_effective_quota()?

Quoted 10 lines

If this wraps around on 32-bit architectures, it improperly clamps the effective size to ctx->min_region_sz, nullifying the time quota feature.
Quoted 4 lines
mm/damon/core.c +1 -0
+		esz = min(esz_time, esz);
 	}
 
 	if (quota->sz && quota->sz < esz)
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thank you for pointing those out! I will fix those separately.

Best regards,
Rui Yan

SJ Park

maintainer

2026-09-05 16:20 UTC #3 lore raw

Quoted 55 lines · On Sat, 5 Sep 2026 22:14:09 +0800 Liew Rui Yan <[email protected]>…

Quoted 53 lines · On Sat, 05 Sep 2026 12:58:25 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures. --
commit dc1c35769ce920ee1796367097f9876b2f24a643
Author: Liew Rui Yan <[email protected]>
mm/damon/core: allow esz to be set to zero
This commit modifies damos_set_effective_quota() to allow the effective size to be set to zero when the goal has been achieved. It changes the order of max and min calculations when quota->ms is set to ensure that a zero esz_bp results in a minimal effective size instead of being clamped to min_region_sz.
Quoted 4 lines
mm/damon/core.c +0 -0
index ce8c6f99106ed..3e9c9b6385d4e 100644
[ ... ]
mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3260
Quoted 2 lines
mm/damon/core.c +0 -0
@@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 					1000000, quota->total_charged_ns);
[Severity: High]
This is a pre-existing issue, but does mult_frac() silently overflow on 32-bit architectures here in damos_set_effective_quota()?
Since quota->total_charged_ns is represented as an unsigned long, it natively overflows after 4.29 seconds on 32-bit systems. Furthermore, when calculating throughput via mult_frac(), the remainder calculation (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which causes throughput estimation to be completely incorrect and often near zero.
mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3115
Quoted 5 lines
mm/damon/core.c +1 -2
 		else
 			throughput = PAGE_SIZE * 1024;
-		esz = min(throughput * quota->ms, esz);
-		esz = max(ctx->min_region_sz, esz);
+		esz_time = max(throughput * quota->ms, ctx->min_region_sz);
[Severity: High]
This is also a pre-existing issue, but can throughput * quota->ms overflow 32 bits for quotas over a few seconds here in damos_set_effective_quota()?
If this wraps around on 32-bit architectures, it improperly clamps the effective size to ctx->min_region_sz, nullifying the time quota feature.
Quoted 3 lines
mm/damon/core.c +1 -0
+		esz = min(esz_time, esz);
 	}
 

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
mm/damon/core.c +0 -0
 	if (quota->sz && quota->sz < esz)
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Thank you for pointing those out! I will fix those separately.

These are known low priority issue that I'm working on. Let's not duplicate works.

Thanks,
SJ

[...]

SJ Park

maintainer Reviewed-by on v3 1/1

2026-09-05 16:18 UTC #4 lore raw

In reply to Liew Rui Yan

Hello Liew,

Quoted 10 lines · On Sat, 5 Sep 2026 20:44:58 +0800 Liew Rui Yan <[email protected]>…

When the temporal quota goal tuner determines that the goal has been achieved (score >= 10000), it sets esz_bp to zero so that the esz becomes zero. However, damos_set_effective_quota() clamps the esz to min_region_sz when quota->ms is set.
This is a minor issue, the main problem is that it doesn't match the description in the documentation, which state that if the goal has already been [over-]achieved, the quota will be set to zero.
Fix this by set quota (esz) as minimum as possible.

Thank you for finding and fixing this.

Quoted 4 lines

Fixes 8bbde987c2b8 ("mm/damon/core: disallow time-quota setting zero esz")
Cc <[email protected]> # v7.1.x
Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

This patch is applied to damon/next [1] tree. If this patch is not added to mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton) to pick this. So, no action from your side is needed for now. If it seems I also forgot doing that or you cannot wait for my action, please feel free to directly ask that to Andrew.

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

Thanks,
SJ

[...]