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

PATCH Superseded Liew Rui Yan 3 participants 7 messages Sep 2

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

Liew Rui Yan

author

2026-09-02 08:20 UTC #0 lore raw

PATCH v2 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 effective quota (esz) becomes zero. However, damos_set_effective_quota() clamps the quota to min_region_sz, preventing the quota from ever reaching zero.

Fix this by adding a check for zero esz_bp.

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 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 | 3 +++
 1 file changed, 3 insertions(+)

mm/damon/core.c +3 -0

index e13a154aa4b9..8e3ded1a2cf6 100644
@@ -3133,6 +3133,9 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 	if (quota->sz && quota->sz < esz)
 		esz = quota->sz;
 
+	if (!quota->esz_bp)
+		esz = 0;
+
 	quota->esz = esz;
 }

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-02 08:30 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The patch breaks purely time-based quotas by unconditionally zeroing the effective size quota…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks purely time-based quotas by unconditionally zeroing the effective size quota (esz) for DAMOS schemes without goals. --

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

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

This commit updates damos_set_effective_quota() to allow the effective quota to be set to zero when the temporal quota goal tuner achieves its goal. Previously, the quota was clamped to min_region_sz, preventing it from reaching zero.

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

Quoted 11 lines

mm/damon/core.c +3 -0

index df22621765911..73dc9a70fb12b 100644
@@ -3251,6 +3251,9 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 	if (quota->sz && quota->sz < esz)
 		esz = quota->sz;
 
+	if (!quota->esz_bp)
+		esz = 0;
+

[Severity: High]
Could this cause a regression for schemes that use purely time-based quotas?

If a scheme is configured with a time-based quota (quota->ms > 0) but has no quota goals (list_empty(&quota->goals)), quota->esz_bp remains at its default initialization value of 0.

While damos_set_effective_quota() correctly computes esz from the throughput and quota->ms, this unconditional check will then force esz to 0 because quota->esz_bp is 0. An esz of 0 causes damos_quota_is_full() to always return true, effectively breaking schemes relying only on time-based quotas.

Quoted 3 lines

mm/damon/core.c +0 -0
 	quota->esz = esz;
 }
 

Signature · 1 line

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

Liew Rui Yan

author

2026-09-02 09:51 UTC #2 lore raw

Quoted 29 lines · On Wed, 02 Sep 2026 08:30:46 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks purely time-based quotas by unconditionally zeroing the effective size quota (esz) for DAMOS schemes without goals. --
commit 34629844ff11daf28855cce5457468ad8bc08c73
Author: Liew Rui Yan <[email protected]>
mm/damon/core: allow esz to be set to zero
This commit updates damos_set_effective_quota() to allow the effective quota to be set to zero when the temporal quota goal tuner achieves its goal. Previously, the quota was clamped to min_region_sz, preventing it from reaching zero.
mm/damon/core.c static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s) · line 3256
Quoted 11 lines
mm/damon/core.c +3 -0
index df22621765911..73dc9a70fb12b 100644
@@ -3251,6 +3251,9 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
 	if (quota->sz && quota->sz < esz)
 		esz = quota->sz;
 
+	if (!quota->esz_bp)
+		esz = 0;
+
[Severity: High]
Could this cause a regression for schemes that use purely time-based quotas?

Quoted 8 lines

If a scheme is configured with a time-based quota (quota->ms > 0) but has no quota goals (list_empty(&quota->goals)), quota->esz_bp remains at its default initialization value of 0.
While damos_set_effective_quota() correctly computes esz from the throughput and quota->ms, this unconditional check will then force esz to 0 because quota->esz_bp is 0. An esz of 0 causes damos_quota_is_full() to always return true, effectively breaking schemes relying only on time-based quotas.

Yes, thank you for pointing this out!

esz_bp is a meaningful field only when (!list_empty(&quota->goals)), so the correct check should be:

'''
if (!list_empty(&quota->goals) && !quota->esz_bp)

    esz = 0;

'''

Quoted 7 lines

Quoted 4 lines
mm/damon/core.c +0 -0
 	quota->esz = esz;
 }
 
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Best regards,
Rui Yan

SJ Park

maintainer

2026-09-02 14:13 UTC #3 lore raw

In reply to Liew Rui Yan

Quoted 5 lines · On Wed, 2 Sep 2026 16:20:50 +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 effective quota (esz) becomes zero. However, damos_set_effective_quota() clamps the quota to min_region_sz, preventing the quota from ever reaching zero.

Where in the code it is clamped to min_region_sz, when? And what user issue this can cause?

Thanks,
SJ

[...]

Liew Rui Yan

author

2026-09-02 14:35 UTC #4 lore raw

Quoted 10 lines · On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <[email protected]> wrote:

Quoted 5 lines · On Wed, 2 Sep 2026 16:20:50 +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 effective quota (esz) becomes zero. However, damos_set_effective_quota() clamps the quota to min_region_sz, preventing the quota from ever reaching zero.
Where in the code it is clamped to min_region_sz, when? And what user issue this can cause?

In damos_set_effective_quota(), when quota->ms is set.

'''

	if (quota->ms) {
		if (quota->total_charged_ns)
			throughput = mult_frac(quota->total_charged_sz,
					1000000, quota->total_charged_ns);
		else
			throughput = PAGE_SIZE * 1024;
		esz = min(throughput * quota->ms, esz);
		esz = max(ctx->min_region_sz, esz); /* <- HERE */
	}

'''

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

Original documentation:

- ``temporal``: More straightforward algorithm. Tries to achieve the goal as fast as possible, using maximum allowed quota, but only for a temporal short time. When the quota is under-achieved, this algorithm keeps tuning quota to a maximum allowed one. Once the quota is [over]-achieved, this sets the quota zero. Useful for deterministic control required environments.

Best regards,
Rui Yan

SJ Park

maintainer

2026-09-02 15:03 UTC #5 lore raw

Quoted 10 lines · On Wed, 2 Sep 2026 22:35:20 +0800 Liew Rui Yan <[email protected]>…

Quoted 8 lines · On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <[email protected]> wrote:
Quoted 5 lines · On Wed, 2 Sep 2026 16:20:50 +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 effective quota (esz) becomes zero. However, damos_set_effective_quota() clamps the quota to min_region_sz, preventing the quota from ever reaching zero.

Quoted 4 lines

Quoted 2 lines
Where in the code it is clamped to min_region_sz, when? And what user issue this can cause?
In damos_set_effective_quota(), when quota->ms is set.

Please clarify this kind of thing (when quota->ms is set) from the next time.

Quoted 17 lines

'''
	if (quota->ms) {
		if (quota->total_charged_ns)
			throughput = mult_frac(quota->total_charged_sz,
					1000000, quota->total_charged_ns);
		else
			throughput = PAGE_SIZE * 1024;
		esz = min(throughput * quota->ms, esz);
		esz = max(ctx->min_region_sz, esz); /* <- HERE */
	}
'''
This is a minor issue, the main problem is that it doesn't match the description in the documentation, which states that if the goal has already been [over-]achieved, the quota will be set to 0.

Quoted 7 lines

Original documentation:
- ``temporal``: More straightforward algorithm. Tries to achieve the goal as fast as possible, using maximum allowed quota, but only for a temporal short time. When the quota is under-achieved, this algorithm keeps tuning quota to a maximum allowed one. Once the quota is [over]-achieved, this sets the quota zero. Useful for deterministic control required environments.

Thank you for clarifying. Please clarify what is the problem like this from the next time. Without it, reviewing spend unnecessary time.

I feel like your recent patches tend to lack such clarifications and spend unnecessary time for reviewing. If you unsure, please ask questions first or use RFC tag at least.

I agree this behavior is not matching with the documented one. The point of quota is making DAMOS not unnecessarily aggressive. Hence it is designed to be set as minimum as possible. How about below?

'''

mm/damon/core.c +3 -2

@@ -3485,6 +3485,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;

@@ -3505,8 +3506,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)

'''

Thanks,
SJ

[...]

Liew Rui Yan

author

2026-09-02 23:13 UTC #6 lore raw

Quoted 38 lines · On Wed, 02 Sep 2026 08:03:00 -0700 SJ Park <[email protected]> wrote:

Quoted 10 lines · On Wed, 2 Sep 2026 22:35:20 +0800 Liew Rui Yan <[email protected]>…
Quoted 8 lines · On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <[email protected]> wrote:
Quoted 5 lines · On Wed, 2 Sep 2026 16:20:50 +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 effective quota (esz) becomes zero. However, damos_set_effective_quota() clamps the quota to min_region_sz, preventing the quota from ever reaching zero.
Quoted 4 lines
Quoted 2 lines
Where in the code it is clamped to min_region_sz, when? And what user issue this can cause?
In damos_set_effective_quota(), when quota->ms is set.
Please clarify this kind of thing (when quota->ms is set) from the next time.
Quoted 19 lines
'''
	if (quota->ms) {
		if (quota->total_charged_ns)
			throughput = mult_frac(quota->total_charged_sz,
					1000000, quota->total_charged_ns);
		else
			throughput = PAGE_SIZE * 1024;
		esz = min(throughput * quota->ms, esz);
		esz = max(ctx->min_region_sz, esz); /* <- HERE */
	}
'''
This is a minor issue, the main problem is that it doesn't match the description in the documentation, which states that if the goal has already been [over-]achieved, the quota will be set to 0.
Original documentation:

Quoted 8 lines

Quoted 5 lines
- ``temporal``: More straightforward algorithm. Tries to achieve the goal as fast as possible, using maximum allowed quota, but only for a temporal short time. When the quota is under-achieved, this algorithm keeps tuning quota to a maximum allowed one. Once the quota is [over]-achieved, this sets the quota zero. Useful for deterministic control required environments.
Thank you for clarifying. Please clarify what is the problem like this from the next time. Without it, reviewing spend unnecessary time.

Noted. I will ensure future commit messages and descriptions clearly state the triggering conditions and potential user impact to make the review process more efficient.

Quoted 4 lines

I feel like your recent patches tend to lack such clarifications and spend unnecessary time for reviewing. If you unsure, please ask questions first or use RFC tag at least.

Understood. I will use the RFC tag or ask questions first when the nature of the issue is ambiguous.

Quoted 27 lines

I agree this behavior is not matching with the documented one. The point of quota is making DAMOS not unnecessarily aggressive. Hence it is designed to be set as minimum as possible. How about below?

'''

mm/damon/core.c +3 -2

@@ -3485,6 +3485,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;

@@ -3505,8 +3506,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)

Quoted 1 line

'''

This solution is much better than my initial approach.

I will incorporate this change into the next version. Thank you for the review and the improved fix!

Best regards,
Rui Yan