Skip to contentSkip to conversation

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

PATCH Applied SJ Park 2 participants 4 messages Sep 8

Subsystems (3)
lore
Share & tools
lore

SJ Park

author maintainer
#0 lore raw

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

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

Fixes 8bbde987c2b8 ("mm/damon/core: disallow time-quota setting zero esz")
Cc <[email protected]> # v7.1.x
Cc Andrew Morton <[email protected]>

---
Changes from v3:
- v3: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
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 7e6cd405161d4..06cf2ab7e97d7 100644
@@ -3242,6 +3242,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;
@@ -3262,8 +3263,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)

base-commit: 6f8c7324e336e626bd993bc36f177dcf493a871f

Signature · 1 line
2.47.3
#1 lore raw
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

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

Pre-existing issues:
- [High] Integer overflow in temporal goal tuner calculating esz_bp and ULONG_MAX capping severely limits DAMOS effective size quota on 32-bit systems.
- [High] Integer overflow when calculating time quota constraints (throughput * quota->ms) on 32-bit systems limits time-based quota. --

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

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

When the temporal quota goal tuner achieves its goal, it sets esz_bp to zero, but damos_set_effective_quota() previously clamped this to min_region_sz. This patch fixes this by keeping the effective size quota (esz) as minimum as possible when the goal is achieved.

Quoted 12 lines
mm/damon/core.c +1 -0
index ce8c6f99106ed..3e9c9b6385d4e 100644
@@ -3239,6 +3239,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;

[ ... ]

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

Quoted 7 lines
mm/damon/core.c +1 -2
@@ -3259,8 +3260,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);

[Severity: High]
This isn't a bug introduced by this patch, but does this inline multiplication wrap around on 32-bit systems?

Both throughput (calculated as bytes per millisecond) and quota->ms are unsigned long. On a 32-bit architecture, a realistic throughput like 2 MB/ms combined with a quota limit of 3000 ms yields a product of 6,000,000,000.

Since this exceeds the 32-bit unsigned long limit of 4.29 billion, it silently wraps around to a significantly smaller size limit, which unintentionally bounds the effective quota and breaks the intended time-based logic.

[Severity: High]
This is a pre-existing issue, but I also noticed a similar overflow problem regarding the quota->esz_bp calculation in this same path.

When a user configures quota->sz via sysfs, it is scaled here:

mm/damon/core.c:damos_goal_tune_esz_bp_temporal() {
    ...
		quota->esz_bp = quota->sz * 10000;
	else
		quota->esz_bp = ULONG_MAX;
    ...
}

If quota->sz is unset (0), esz_bp defaults to ULONG_MAX (4,294,967,295 on 32-bit systems). When this is later scaled down:

mm/damon/core.c:damos_set_effective_quota() {
    ...
		esz = quota->esz_bp / 10000;
    ...
}

The resulting esz becomes 429,496 bytes (~419 KB). Does this artificially throttle DAMOS region actions on 32-bit platforms?

Additionally, if quota->sz is set larger than ~429 KB, quota->sz * 10000 silently overflows the 32-bit unsigned long, creating a much smaller wrapped quota. Could we address this ULONG_MAX limitation and multiplication overflow on 32-bit architectures?

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
author maintainer
#2 lore raw
Quoted 44 lines · On Tue, 08 Sep 2026 14:04:45 +0000 [email protected] wrote:

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

Pre-existing issues:
- [High] Integer overflow in temporal goal tuner calculating esz_bp and ULONG_MAX capping severely limits DAMOS effective size quota on 32-bit systems.
- [High] Integer overflow when calculating time quota constraints (throughput * quota->ms) on 32-bit systems limits time-based quota. --

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

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

When the temporal quota goal tuner achieves its goal, it sets esz_bp to zero, but damos_set_effective_quota() previously clamped this to min_region_sz. This patch fixes this by keeping the effective size quota (esz) as minimum as possible when the goal is achieved.

Quoted 12 lines
mm/damon/core.c +1 -0
index ce8c6f99106ed..3e9c9b6385d4e 100644
@@ -3239,6 +3239,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;

[ ... ]

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

Quoted 7 lines
mm/damon/core.c +1 -2
@@ -3259,8 +3260,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);

[Severity: High]
This isn't a bug introduced by this patch, but does this inline multiplication wrap around on 32-bit systems?

Quoted 7 lines

Both throughput (calculated as bytes per millisecond) and quota->ms are unsigned long. On a 32-bit architecture, a realistic throughput like 2 MB/ms combined with a quota limit of 3000 ms yields a product of 6,000,000,000.

Since this exceeds the 32-bit unsigned long limit of 4.29 billion, it silently wraps around to a significantly smaller size limit, which unintentionally bounds the effective quota and breaks the intended time-based logic.

We provide addr_unit for this kind of issues on 32 bit systems. I will separately work on this with low priority.

Quoted 24 lines

[Severity: High]
This is a pre-existing issue, but I also noticed a similar overflow problem regarding the quota->esz_bp calculation in this same path.

When a user configures quota->sz via sysfs, it is scaled here:

mm/damon/core.c:damos_goal_tune_esz_bp_temporal() {
    ...
		quota->esz_bp = quota->sz * 10000;
	else
		quota->esz_bp = ULONG_MAX;
    ...
}

If quota->sz is unset (0), esz_bp defaults to ULONG_MAX (4,294,967,295 on 32-bit systems). When this is later scaled down:

mm/damon/core.c:damos_set_effective_quota() {
    ...
		esz = quota->esz_bp / 10000;
    ...
}
Quoted 7 lines

The resulting esz becomes 429,496 bytes (~419 KB). Does this artificially throttle DAMOS region actions on 32-bit platforms?

Additionally, if quota->sz is set larger than ~429 KB, quota->sz * 10000 silently overflows the 32-bit unsigned long, creating a much smaller wrapped quota. Could we address this ULONG_MAX limitation and multiplication overflow on 32-bit architectures?

We provide addr_unit for this kind of issues on 32 bit systems. I will separately work on this with low priority.

Thanks,
SJ

[...]

author maintainer
#3 lore raw
In reply to SJ Park
Quoted 7 lines · On Tue, 8 Sep 2026 06:54:11 -0700 SJ Park <[email protected]> wrote:

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

Quoted 5 lines

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.

Sashiko found a few pre-existing issues but no blocker for this patch.

Thanks,
SJ

[...]