Re: [PATCH] mm/damon/core: always return false when esz is zero

[Liew Rui Yan <aethernet65535@gmail.com>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch breaks the O(1) early bailout for schemes with a zero quota, causing O(N) evaluation of all regions, significant CPU overhead, and corruption of the 'nr_tried' statistic.
> --
> 
> commit dd8f74f4c9d96719d46a1466acabcc4d814aadc9
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: always return false when esz is zero
> 
> This patch modifies damos_quota_is_full() to return false when the effective
> quota size (esz) is zero. This prevents qt_exceeds from increasing
> unexpectedly when a temporal goal tuner achieves its target and sets the
> quota to zero.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3f89cfdf5f022..012e8970f154f 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2551,6 +2551,8 @@ static bool damos_quota_is_full(struct damos_quota *quota,
> >  {
> >  	if (!damos_quota_is_set(quota))
> >  		return false;
> > +	if (!quota->esz)
> > +		return false;
> 
> [Severity: Medium]
> When a scheme's goal tuner achieves its target and sets the quota to zero,
> does returning false here unintentionally break the early bailout in
> damon_do_apply_schemes()?
> 
> When this function returned true, damon_do_apply_schemes() used it to skip
> the scheme entirely:
> 
> mm/damon/core.c:damon_do_apply_schemes() {
>     ...
> 		/* Check the quota */
> 		if (damos_quota_is_full(quota, c->min_region_sz))
> 			continue;
>     ...
> }
> 
> If it now returns false when esz is 0, doesn't that cause the loop to
> evaluate every region via damos_valid_target() and damos_apply_scheme()
> instead of skipping the scheme?
> 
> If the scheme is not skipped, it appears damos_apply_scheme() will calculate
> a zero sz and jump to update_stat:
> 
> mm/damon/core.c:damos_apply_scheme() {
>     ...
> 		if (damos_quota_is_set(quota) &&
> 				quota->charged_sz + sz > quota->esz) {
> 			sz = ALIGN_DOWN(quota->esz - quota->charged_sz,
> 					c->min_region_sz);
> 			if (!sz)
> 				goto update_stat;
>     ...
> update_stat:
> 	damos_update_stat(s, sz, sz_applied, sz_ops_filter_passed);
> }
> 
> Will this cause s->stat.nr_tried to be incorrectly incremented in
> damos_update_stat() for every bypassed region?

Thank you for pointing this out!

This can fixed by adding a check before damos_quota_is_set().

'''
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..795f802657ff 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2560,6 +2560,8 @@ static bool damos_quota_is_full(struct damos_quota *quota,
 {
        if (!damos_quota_is_set(quota))
                return false;
+       if (!quota->charged_sz)
+               return false;
        if (quota->charged_sz >= quota->esz)
                return true;
        /*
@@ -2654,6 +2656,9 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
                if (!s->wmarks.activated)
                        continue;
 
+               if (!quota->esz)
+                       continue;
+
                /* Check the quota */
                if (damos_quota_is_full(quota, c->min_region_sz))
                        continue;
'''

Note that this also changes the implementation of this patch, because
quota->esz is an upper-bound, while quota->charged_sz represents the
actual quota usage.

> 
> >  	if (quota->charged_sz >= quota->esz)
> >  		return true;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901105925.106186-1-aethernet65535@gmail.com?part=1

Best regards,
Rui Yan