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

[SJ Park <sj@kernel.org>]

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

[...]