DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH] mm/damon/core: always return false when esz is zero
@ 2026-09-01 10:59 Liew Rui Yan
  2026-09-01 11:12 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Liew Rui Yan @ 2026-09-01 10:59 UTC (permalink / raw)
  To: SJ Park; +Cc: Andrew Morton, damon, linux-mm, linux-kernel, Liew Rui Yan,
	stable

After setting goal_tuner to 'temporal', if the goal is achieved,
quota->esz will be set to zero.  In this case, damos_quota_is_full()
will always return true, even if no regions have been tried at all.
This causes qt_exceeds to increase unexpectedly.

Fix it by always returning false when quota->esz is zero.

Fixes: c7ec7d5f6b3d ("mm/damon/core: handle <min_region_sz remaining quota as empty")
Cc: <stable@vger.kernel.org> # v7.2.x
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 mm/damon/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..53cf32f1d515 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->esz)
+		return false;
 	if (quota->charged_sz >= quota->esz)
 		return true;
 	/*
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/damon/core: always return false when esz is zero
  2026-09-01 10:59 [PATCH] mm/damon/core: always return false when esz is zero Liew Rui Yan
@ 2026-09-01 11:12 ` sashiko-bot
  2026-09-01 13:15   ` Liew Rui Yan
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-09-01 11:12 UTC (permalink / raw)
  To: Liew Rui Yan; +Cc: damon

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?

>  	if (quota->charged_sz >= quota->esz)
>  		return true;

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/damon/core: always return false when esz is zero
  2026-09-01 11:12 ` sashiko-bot
@ 2026-09-01 13:15   ` Liew Rui Yan
  0 siblings, 0 replies; 3+ messages in thread
From: Liew Rui Yan @ 2026-09-01 13:15 UTC (permalink / raw)
  To: sashiko-bot, sj; +Cc: aethernet65535, damon, sashiko-reviews

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-01 13:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 10:59 [PATCH] mm/damon/core: always return false when esz is zero Liew Rui Yan
2026-09-01 11:12 ` sashiko-bot
2026-09-01 13:15   ` Liew Rui Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox