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