Re: [PATCH] mm/damon/core: fix quota could not be set to zero

Re: [PATCH] mm/damon/core: fix quota could not be set to zero

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch places the `if (esz)` check after the throughput calculation, inadvertently reverting a previous fix (commit 8bbde987c2b8) and causing schemes with low throughput to permanently deactivate.
--

commit a1511f591a76665e55c325a03d3d47bf4ba87dc6
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix quota could not be set to zero

This commit modifies the damos quota setting logic to skip clamping to
min_region_sz when the effective size quota is already zero. This is
intended to allow the temporal tuner's zero quota goal to be respected.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3f89cfdf5f022..fdec7c0fe913a 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3122,7 +3122,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
>  		else
>  			throughput = PAGE_SIZE * 1024;
>  		esz = min(throughput * quota->ms, esz);
> -		esz = max(ctx->min_region_sz, esz);
> +		if (esz)
> +			esz = max(ctx->min_region_sz, esz);

[Severity: High]
Does placing the if (esz) check after the throughput calculation inadvertently
revert the fix from commit 8bbde987c2b8 and cause a regression where slow
schemes permanently deactivate?

If a damos scheme with a time quota processes regions slowly, integer division
can cause throughput to truncate to 0. When this happens, the calculation
esz = min(throughput * quota->ms, esz) evaluates to 0.

Because the new if (esz) check is placed after this min() calculation, the 0
value causes the check to fail, skipping the max(ctx->min_region_sz, esz)
clamp.

With esz == 0, damos_quota_is_full() always returns true, bypassing all
scheme operations. Since no regions are processed, total_charged_ns and
total_charged_sz are never updated. This leaves throughput permanently at 0,
resulting in an unrecoverable starvation loop.

>  	}
>  
>  	if (quota->sz && quota->sz < esz)

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

Re: [PATCH] mm/damon/core: fix quota could not be set to zero

[Liew Rui Yan]

On Tue, 01 Sep 2026 11:14:40 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The patch places the `if (esz)` check after the throughput calculation, inadvertently reverting a previous fix (commit 8bbde987c2b8) and causing schemes with low throughput to permanently deactivate.
> --
> 
> commit a1511f591a76665e55c325a03d3d47bf4ba87dc6
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix quota could not be set to zero
> 
> This commit modifies the damos quota setting logic to skip clamping to
> min_region_sz when the effective size quota is already zero. This is
> intended to allow the temporal tuner's zero quota goal to be respected.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3f89cfdf5f022..fdec7c0fe913a 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3122,7 +3122,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> >  		else
> >  			throughput = PAGE_SIZE * 1024;
> >  		esz = min(throughput * quota->ms, esz);
> > -		esz = max(ctx->min_region_sz, esz);
> > +		if (esz)
> > +			esz = max(ctx->min_region_sz, esz);
> 
> [Severity: High]
> Does placing the if (esz) check after the throughput calculation inadvertently
> revert the fix from commit 8bbde987c2b8 and cause a regression where slow
> schemes permanently deactivate?
> 
> If a damos scheme with a time quota processes regions slowly, integer division
> can cause throughput to truncate to 0. When this happens, the calculation
> esz = min(throughput * quota->ms, esz) evaluates to 0.
> 
> Because the new if (esz) check is placed after this min() calculation, the 0
> value causes the check to fail, skipping the max(ctx->min_region_sz, esz)
> clamp.
> 
> With esz == 0, damos_quota_is_full() always returns true, bypassing all
> scheme operations. Since no regions are processed, total_charged_ns and
> total_charged_sz are never updated. This leaves throughput permanently at 0,
> resulting in an unrecoverable starvation loop.

Thank you for pointing out this!  Yes, this can happen, and this is what
8bbde987c2b8 was fixing.

damos_goal_tune_esz_bp_temporal() will set quota->esz_bp to zero when
goal is archived, so this need fix by adding a check for quota->esz_bp.

'''
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..c07ae66caa49 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3118,6 +3118,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 = quota->esz_bp / 10000;
+
        quota->esz = esz;
 }
'''

I will change this in next revision.

> 
> >  	}
> >  
> >  	if (quota->sz && quota->sz < esz)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901105951.106246-1-aethernet65535@gmail.com?part=1

Best regards,
Rui Yan

Re: [PATCH] mm/damon/core: fix quota could not be set to zero

[SJ Park]

Hi Liew,


Seems Sashiko found something and you are planning to revision this patch that.
I will hold review until the next version is posted.  Same for your other patch
that sent in the same time.


Thanks,
SJ

[...]