* [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