1. Patchew
  2. linux
  3. View series

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

Liew Rui Yan posted 1 patch 3 weeks, 3 days ago
There is a newer version of this series
mm/damon/core.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH v2] mm/damon/core: allow esz to be set to zero
Posted by Liew Rui Yan 3 weeks, 3 days ago 
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
Posted by SJ Park 3 weeks, 2 days ago 
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
Posted by Liew Rui Yan 3 weeks, 2 days ago 
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
Posted by SJ Park 3 weeks, 2 days ago 
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
Posted by Liew Rui Yan 3 weeks, 2 days ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.