[PATCH v2] mm/damon/core: allow esz to be set to zero

[PATCH v2] mm/damon/core: allow esz to be set to zero

[Liew Rui Yan]

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: <stable@vger.kernel.org> # v7.1.x
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---

Changes from v1:
- v1: https://lore.kernel.org/damon/20260901105951.106246-1-aethernet65535@gmail.com
- 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(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index e13a154aa4b9..8e3ded1a2cf6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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;
 }
 
-- 
2.55.0

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[sashiko-bot]

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 <aethernet65535@gmail.com>

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index df22621765911..73dc9a70fb12b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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.

>  	quota->esz = esz;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902082050.19566-1-aethernet65535@gmail.com?part=1

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[Liew Rui Yan]

On Wed, 02 Sep 2026 08:30:46 +0000 sashiko-bot@kernel.org 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 <aethernet65535@gmail.com>
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index df22621765911..73dc9a70fb12b 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -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.

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

> 
> >  	quota->esz = esz;
> >  }
> >
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260902082050.19566-1-aethernet65535@gmail.com?part=1

Best regards,
Rui Yan

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[SJ Park]

On Wed,  2 Sep 2026 16:20:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> 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

[...]

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[Liew Rui Yan]

On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <sj@kernel.org> wrote:

> On Wed,  2 Sep 2026 16:20:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> > 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

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[SJ Park]

On Wed,  2 Sep 2026 22:35:20 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > On Wed,  2 Sep 2026 16:20:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> > 
> > > 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.

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

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

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?

'''
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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

[...]

Re: [PATCH v2] mm/damon/core: allow esz to be set to zero

[Liew Rui Yan]

On Wed, 02 Sep 2026 08:03:00 -0700 SJ Park <sj@kernel.org> wrote:

> On Wed,  2 Sep 2026 22:35:20 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> > On Wed, 02 Sep 2026 07:13:48 -0700 SJ Park <sj@kernel.org> wrote:
> > 
> > > On Wed,  2 Sep 2026 16:20:50 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> > > 
> > > > 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.
> 
> Please clarify this kind of thing (when quota->ms is set) from the next time.
> 
> > 
> > '''
> > 	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.
> 
> 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.

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

> 
> 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?
> 
> '''
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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)
> '''

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