* [PATCH v3] mm/damon/core: allow esz to be set to zero
@ 2026-09-05 12:44 Liew Rui Yan
2026-09-05 12:58 ` sashiko-bot
2026-09-05 16:18 ` SJ Park
0 siblings, 2 replies; 5+ messages in thread
From: Liew Rui Yan @ 2026-09-05 12:44 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, damon, linux-mm, linux-kernel, Liew Rui Yan,
stable
When the temporal quota goal tuner determines that the goal has been
achieved (score >= 10000), it sets esz_bp to zero so that the esz
becomes zero. However, damos_set_effective_quota() clamps the esz to
min_region_sz when quota->ms is set.
This is a minor issue, the main problem is that it doesn't match the
description in the documentation, which state that if the goal has
already been [over-]achieved, the quota will be set to zero.
Fix this by set quota (esz) as minimum as possible.
Fixes: 8bbde987c2b8 ("mm/damon/core: disallow time-quota setting zero esz")
Cc: <stable@vger.kernel.org> # v7.1.x
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
Changes from v2:
- v2: https://lore.kernel.org/damon/20260902082050.19566-1-aethernet65535@gmail.com
- Changed implementation, more general implementation provided by SJ. [1]
[1] https://lore.kernel.org/damon/20260902150301.88535-1-sj@kernel.org
Changes from v1:
- v1: https://lore.kernel.org/damon/20260901105951.106246-1-aethernet65535@gmail.com
- Changed implementation. The initial solution skipped the
min_region_sz clamping when esz was already zero. However, this would
revert the fix from commit 8bbde987c2b8 and cause a regression. This
revision fixes the issue by adding a specific check for a zero esz_bp.
- Changed patch title, original title: fix quota could not be set to
zero
---
mm/damon/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..2b294fb46648 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3091,6 +3091,7 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
struct damos_quota *quota = &s->quota;
unsigned long throughput;
unsigned long esz = ULONG_MAX;
+ unsigned long esz_time;
if (!quota->ms && list_empty("a->goals)) {
quota->esz = quota->sz;
@@ -3111,8 +3112,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
1000000, quota->total_charged_ns);
else
throughput = PAGE_SIZE * 1024;
- esz = min(throughput * quota->ms, esz);
- esz = max(ctx->min_region_sz, esz);
+ esz_time = max(throughput * quota->ms, ctx->min_region_sz);
+ esz = min(esz_time, esz);
}
if (quota->sz && quota->sz < esz)
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] mm/damon/core: allow esz to be set to zero
2026-09-05 12:44 [PATCH v3] mm/damon/core: allow esz to be set to zero Liew Rui Yan
@ 2026-09-05 12:58 ` sashiko-bot
2026-09-05 14:14 ` Liew Rui Yan
2026-09-05 16:18 ` SJ Park
1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-09-05 12:58 UTC (permalink / raw)
To: Liew Rui Yan; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures.
--
commit dc1c35769ce920ee1796367097f9876b2f24a643
Author: Liew Rui Yan <aethernet65535@gmail.com>
mm/damon/core: allow esz to be set to zero
This commit modifies damos_set_effective_quota() to allow the effective
size to be set to zero when the goal has been achieved. It changes the
order of max and min calculations when quota->ms is set to ensure
that a zero esz_bp results in a minimal effective size instead of being
clamped to min_region_sz.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ce8c6f99106ed..3e9c9b6385d4e 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> 1000000, quota->total_charged_ns);
[Severity: High]
This is a pre-existing issue, but does mult_frac() silently overflow on
32-bit architectures here in damos_set_effective_quota()?
Since quota->total_charged_ns is represented as an unsigned long, it natively
overflows after 4.29 seconds on 32-bit systems. Furthermore, when
calculating throughput via mult_frac(), the remainder calculation
(sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which
causes throughput estimation to be completely incorrect and often near zero.
> else
> throughput = PAGE_SIZE * 1024;
> - esz = min(throughput * quota->ms, esz);
> - esz = max(ctx->min_region_sz, esz);
> + esz_time = max(throughput * quota->ms, ctx->min_region_sz);
[Severity: High]
This is also a pre-existing issue, but can throughput * quota->ms overflow 32
bits for quotas over a few seconds here in damos_set_effective_quota()?
If this wraps around on 32-bit architectures, it improperly clamps the
effective size to ctx->min_region_sz, nullifying the time quota feature.
> + esz = min(esz_time, esz);
> }
>
> if (quota->sz && quota->sz < esz)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260905124706.3398-1-aethernet65535@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm/damon/core: allow esz to be set to zero
2026-09-05 12:58 ` sashiko-bot
@ 2026-09-05 14:14 ` Liew Rui Yan
2026-09-05 16:20 ` SJ Park
0 siblings, 1 reply; 5+ messages in thread
From: Liew Rui Yan @ 2026-09-05 14:14 UTC (permalink / raw)
To: sashiko-bot; +Cc: aethernet65535, damon, sashiko-reviews
On Sat, 05 Sep 2026 12:58:25 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures.
> --
>
> commit dc1c35769ce920ee1796367097f9876b2f24a643
> Author: Liew Rui Yan <aethernet65535@gmail.com>
>
> mm/damon/core: allow esz to be set to zero
>
> This commit modifies damos_set_effective_quota() to allow the effective
> size to be set to zero when the goal has been achieved. It changes the
> order of max and min calculations when quota->ms is set to ensure
> that a zero esz_bp results in a minimal effective size instead of being
> clamped to min_region_sz.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ce8c6f99106ed..3e9c9b6385d4e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
>
> [ ... ]
>
> > @@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> > 1000000, quota->total_charged_ns);
>
> [Severity: High]
> This is a pre-existing issue, but does mult_frac() silently overflow on
> 32-bit architectures here in damos_set_effective_quota()?
>
> Since quota->total_charged_ns is represented as an unsigned long, it natively
> overflows after 4.29 seconds on 32-bit systems. Furthermore, when
> calculating throughput via mult_frac(), the remainder calculation
> (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which
> causes throughput estimation to be completely incorrect and often near zero.
>
> > else
> > throughput = PAGE_SIZE * 1024;
> > - esz = min(throughput * quota->ms, esz);
> > - esz = max(ctx->min_region_sz, esz);
> > + esz_time = max(throughput * quota->ms, ctx->min_region_sz);
>
> [Severity: High]
> This is also a pre-existing issue, but can throughput * quota->ms overflow 32
> bits for quotas over a few seconds here in damos_set_effective_quota()?
>
> If this wraps around on 32-bit architectures, it improperly clamps the
> effective size to ctx->min_region_sz, nullifying the time quota feature.
>
> > + esz = min(esz_time, esz);
> > }
> >
> > if (quota->sz && quota->sz < esz)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260905124706.3398-1-aethernet65535@gmail.com?part=1
Thank you for pointing those out! I will fix those separately.
Best regards,
Rui Yan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm/damon/core: allow esz to be set to zero
2026-09-05 14:14 ` Liew Rui Yan
@ 2026-09-05 16:20 ` SJ Park
0 siblings, 0 replies; 5+ messages in thread
From: SJ Park @ 2026-09-05 16:20 UTC (permalink / raw)
To: Liew Rui Yan; +Cc: SJ Park, sashiko-bot, damon, sashiko-reviews
On Sat, 5 Sep 2026 22:14:09 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> On Sat, 05 Sep 2026 12:58:25 +0000 sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >
> > Pre-existing issues:
> > - [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures.
> > --
> >
> > commit dc1c35769ce920ee1796367097f9876b2f24a643
> > Author: Liew Rui Yan <aethernet65535@gmail.com>
> >
> > mm/damon/core: allow esz to be set to zero
> >
> > This commit modifies damos_set_effective_quota() to allow the effective
> > size to be set to zero when the goal has been achieved. It changes the
> > order of max and min calculations when quota->ms is set to ensure
> > that a zero esz_bp results in a minimal effective size instead of being
> > clamped to min_region_sz.
> >
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index ce8c6f99106ed..3e9c9b6385d4e 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> >
> > [ ... ]
> >
> > > @@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> > > 1000000, quota->total_charged_ns);
> >
> > [Severity: High]
> > This is a pre-existing issue, but does mult_frac() silently overflow on
> > 32-bit architectures here in damos_set_effective_quota()?
> >
> > Since quota->total_charged_ns is represented as an unsigned long, it natively
> > overflows after 4.29 seconds on 32-bit systems. Furthermore, when
> > calculating throughput via mult_frac(), the remainder calculation
> > (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which
> > causes throughput estimation to be completely incorrect and often near zero.
> >
> > > else
> > > throughput = PAGE_SIZE * 1024;
> > > - esz = min(throughput * quota->ms, esz);
> > > - esz = max(ctx->min_region_sz, esz);
> > > + esz_time = max(throughput * quota->ms, ctx->min_region_sz);
> >
> > [Severity: High]
> > This is also a pre-existing issue, but can throughput * quota->ms overflow 32
> > bits for quotas over a few seconds here in damos_set_effective_quota()?
> >
> > If this wraps around on 32-bit architectures, it improperly clamps the
> > effective size to ctx->min_region_sz, nullifying the time quota feature.
> >
> > > + esz = min(esz_time, esz);
> > > }
> > >
> > > if (quota->sz && quota->sz < esz)
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260905124706.3398-1-aethernet65535@gmail.com?part=1
>
> Thank you for pointing those out! I will fix those separately.
These are known low priority issue that I'm working on. Let's not duplicate
works.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm/damon/core: allow esz to be set to zero
2026-09-05 12:44 [PATCH v3] mm/damon/core: allow esz to be set to zero Liew Rui Yan
2026-09-05 12:58 ` sashiko-bot
@ 2026-09-05 16:18 ` SJ Park
1 sibling, 0 replies; 5+ messages in thread
From: SJ Park @ 2026-09-05 16:18 UTC (permalink / raw)
To: Liew Rui Yan
Cc: SJ Park, Andrew Morton, damon, linux-mm, linux-kernel, stable
Hello Liew,
On Sat, 5 Sep 2026 20:44:58 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> When the temporal quota goal tuner determines that the goal has been
> achieved (score >= 10000), it sets esz_bp to zero so that the esz
> becomes zero. However, damos_set_effective_quota() clamps the esz to
> min_region_sz when quota->ms is set.
>
> This is a minor issue, the main problem is that it doesn't match the
> description in the documentation, which state that if the goal has
> already been [over-]achieved, the quota will be set to zero.
>
> Fix this by set quota (esz) as minimum as possible.
Thank you for finding and fixing this.
>
> Fixes: 8bbde987c2b8 ("mm/damon/core: disallow time-quota setting zero esz")
> Cc: <stable@vger.kernel.org> # v7.1.x
> Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
This patch is applied to damon/next [1] tree. If this patch is not added to
mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton)
to pick this. So, no action from your side is needed for now. If it seems I
also forgot doing that or you cannot wait for my action, please feel free to
directly ask that to Andrew.
[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-05 16:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-05 12:44 [PATCH v3] mm/damon/core: allow esz to be set to zero Liew Rui Yan
2026-09-05 12:58 ` sashiko-bot
2026-09-05 14:14 ` Liew Rui Yan
2026-09-05 16:20 ` SJ Park
2026-09-05 16:18 ` SJ Park
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.