* [PATCH v4] mm/damon/core: allow esz to be set to zero
@ 2026-09-08 13:54 SJ Park
2026-09-08 14:04 ` sashiko-bot
2026-09-08 14:50 ` SJ Park
0 siblings, 2 replies; 4+ messages in thread
From: SJ Park @ 2026-09-08 13:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Liew Rui Yan, stable, SJ Park, damon, linux-kernel, linux-mm
From: Liew Rui Yan <aethernet65535@gmail.com>
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
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v3:
- v3: https://lore.kernel.org/20260905124706.3398-1-aethernet65535@gmail.com
- Collect R-b: from SJ.
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 7e6cd405161d4..06cf2ab7e97d7 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3242,6 +3242,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;
@@ -3262,8 +3263,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)
base-commit: 6f8c7324e336e626bd993bc36f177dcf493a871f
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v4] mm/damon/core: allow esz to be set to zero
2026-09-08 13:54 [PATCH v4] mm/damon/core: allow esz to be set to zero SJ Park
@ 2026-09-08 14:04 ` sashiko-bot
2026-09-08 14:30 ` SJ Park
2026-09-08 14:50 ` SJ Park
1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-09-08 14:04 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Integer overflow in temporal goal tuner calculating `esz_bp` and `ULONG_MAX` capping severely limits DAMOS effective size quota on 32-bit systems.
- [High] Integer overflow when calculating time quota constraints (`throughput * quota->ms`) on 32-bit systems limits time-based quota.
--
commit 05338618157ff96b3145359ae173701a5ed4f699
Author: Liew Rui Yan <aethernet65535@gmail.com>
mm/damon/core: allow esz to be set to zero
When the temporal quota goal tuner achieves its goal, it sets esz_bp to
zero, but damos_set_effective_quota() previously clamped this to
min_region_sz. This patch fixes this by keeping the effective size quota
(esz) as minimum as possible when the goal is achieved.
> 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
> @@ -3239,6 +3239,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;
[ ... ]
> @@ -3259,8 +3260,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);
[Severity: High]
This isn't a bug introduced by this patch, but does this inline multiplication
wrap around on 32-bit systems?
Both throughput (calculated as bytes per millisecond) and quota->ms are
unsigned long. On a 32-bit architecture, a realistic throughput like 2 MB/ms
combined with a quota limit of 3000 ms yields a product of 6,000,000,000.
Since this exceeds the 32-bit unsigned long limit of 4.29 billion, it silently
wraps around to a significantly smaller size limit, which unintentionally bounds
the effective quota and breaks the intended time-based logic.
[Severity: High]
This is a pre-existing issue, but I also noticed a similar overflow problem
regarding the quota->esz_bp calculation in this same path.
When a user configures quota->sz via sysfs, it is scaled here:
mm/damon/core.c:damos_goal_tune_esz_bp_temporal() {
...
quota->esz_bp = quota->sz * 10000;
else
quota->esz_bp = ULONG_MAX;
...
}
If quota->sz is unset (0), esz_bp defaults to ULONG_MAX (4,294,967,295 on
32-bit systems). When this is later scaled down:
mm/damon/core.c:damos_set_effective_quota() {
...
esz = quota->esz_bp / 10000;
...
}
The resulting esz becomes 429,496 bytes (~419 KB). Does this artificially
throttle DAMOS region actions on 32-bit platforms?
Additionally, if quota->sz is set larger than ~429 KB, quota->sz * 10000
silently overflows the 32-bit unsigned long, creating a much smaller wrapped
quota. Could we address this ULONG_MAX limitation and multiplication overflow
on 32-bit architectures?
> + esz = min(esz_time, esz);
> }
>
> if (quota->sz && quota->sz < esz)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260908135413.97570-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v4] mm/damon/core: allow esz to be set to zero
2026-09-08 14:04 ` sashiko-bot
@ 2026-09-08 14:30 ` SJ Park
0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-09-08 14:30 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Tue, 08 Sep 2026 14:04:45 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Integer overflow in temporal goal tuner calculating `esz_bp` and `ULONG_MAX` capping severely limits DAMOS effective size quota on 32-bit systems.
> - [High] Integer overflow when calculating time quota constraints (`throughput * quota->ms`) on 32-bit systems limits time-based quota.
> --
>
> commit 05338618157ff96b3145359ae173701a5ed4f699
> Author: Liew Rui Yan <aethernet65535@gmail.com>
>
> mm/damon/core: allow esz to be set to zero
>
> When the temporal quota goal tuner achieves its goal, it sets esz_bp to
> zero, but damos_set_effective_quota() previously clamped this to
> min_region_sz. This patch fixes this by keeping the effective size quota
> (esz) as minimum as possible when the goal is achieved.
>
> > 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
> > @@ -3239,6 +3239,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;
>
> [ ... ]
>
> > @@ -3259,8 +3260,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);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this inline multiplication
> wrap around on 32-bit systems?
>
> Both throughput (calculated as bytes per millisecond) and quota->ms are
> unsigned long. On a 32-bit architecture, a realistic throughput like 2 MB/ms
> combined with a quota limit of 3000 ms yields a product of 6,000,000,000.
>
> Since this exceeds the 32-bit unsigned long limit of 4.29 billion, it silently
> wraps around to a significantly smaller size limit, which unintentionally bounds
> the effective quota and breaks the intended time-based logic.
We provide addr_unit for this kind of issues on 32 bit systems. I will
separately work on this with low priority.
>
> [Severity: High]
> This is a pre-existing issue, but I also noticed a similar overflow problem
> regarding the quota->esz_bp calculation in this same path.
>
> When a user configures quota->sz via sysfs, it is scaled here:
>
> mm/damon/core.c:damos_goal_tune_esz_bp_temporal() {
> ...
> quota->esz_bp = quota->sz * 10000;
> else
> quota->esz_bp = ULONG_MAX;
> ...
> }
>
> If quota->sz is unset (0), esz_bp defaults to ULONG_MAX (4,294,967,295 on
> 32-bit systems). When this is later scaled down:
>
> mm/damon/core.c:damos_set_effective_quota() {
> ...
> esz = quota->esz_bp / 10000;
> ...
> }
>
> The resulting esz becomes 429,496 bytes (~419 KB). Does this artificially
> throttle DAMOS region actions on 32-bit platforms?
>
> Additionally, if quota->sz is set larger than ~429 KB, quota->sz * 10000
> silently overflows the 32-bit unsigned long, creating a much smaller wrapped
> quota. Could we address this ULONG_MAX limitation and multiplication overflow
> on 32-bit architectures?
We provide addr_unit for this kind of issues on 32 bit systems. I will
separately work on this with low priority.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] mm/damon/core: allow esz to be set to zero
2026-09-08 13:54 [PATCH v4] mm/damon/core: allow esz to be set to zero SJ Park
2026-09-08 14:04 ` sashiko-bot
@ 2026-09-08 14:50 ` SJ Park
1 sibling, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-09-08 14:50 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, Liew Rui Yan, stable, damon, linux-kernel,
linux-mm
On Tue, 8 Sep 2026 06:54:11 -0700 SJ Park <sj@kernel.org> wrote:
> From: Liew Rui Yan <aethernet65535@gmail.com>
>
> 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.
Sashiko found a few pre-existing issues but no blocker for this patch.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-08 14:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 13:54 [PATCH v4] mm/damon/core: allow esz to be set to zero SJ Park
2026-09-08 14:04 ` sashiko-bot
2026-09-08 14:30 ` SJ Park
2026-09-08 14:50 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox