DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz
       [not found] <2026050306-june-passerby-c0a0@gregkh>
@ 2026-05-04 13:01 ` SeongJae Park
  2026-05-04 17:06   ` sashiko-bot
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-05-04 13:01 UTC (permalink / raw)
  To: stable; +Cc: damon, SeongJae Park, Andrew Morton

When the throughput of a DAMOS scheme is very slow, DAMOS time quota can
make the effective size quota smaller than damon_ctx->min_region_sz.  In
the case, damos_apply_scheme() will skip applying the action, because the
action is tried at region level, which requires >=min_region_sz size.
That is, the quota is effectively exceeded for the quota charge window.

Because no action will be applied, the total_charged_sz and
total_charged_ns are also not updated.  damos_set_effective_quota() will
try to update the effective size quota before starting the next charge
window.  However, because the total_charged_sz and total_charged_ns have
not updated, the throughput and effective size quota are also not changed.
Since effective size quota can only be decreased, other effective size
quota update factors including DAMOS quota goals and size quota cannot
make any change, either.

As a result, the scheme is unexpectedly deactivated until the user notices
and mitigates the situation.  The users can mitigate this situation by
changing the time quota online or re-install the scheme.  While the
mitigation is somewhat straightforward, finding the situation would be
challenging, because DAMON is not providing good observabilities for that.
Even if such observability is provided, doing the additional monitoring
and the mitigation is somewhat cumbersome and not aligned to the intention
of the time quota.  The time quota was intended to help reduce the user's
administration overhead.

Fix the problem by setting time quota-modified effective size quota be at
least min_region_sz always.

The issue was discovered [1] by sashiko.

Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]
Fixes: 1cd243030059 ("mm/damon/schemes: implement time quota")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 8bbde987c2b84f80da0853f739f0a920386f8b99)
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 87b6c9c2d647..952cc770edf7 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2124,7 +2124,8 @@ static unsigned long damos_quota_score(struct damos_quota *quota)
 /*
  * Called only if quota->ms, or quota->sz are set, or quota->goals is not empty
  */
-static void damos_set_effective_quota(struct damos_quota *quota)
+static void damos_set_effective_quota(struct damos_quota *quota,
+		struct damon_ctx *ctx)
 {
 	unsigned long throughput;
 	unsigned long esz = ULONG_MAX;
@@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
 		else
 			throughput = PAGE_SIZE * 1024;
 		esz = min(throughput * quota->ms, esz);
+		esz = max(DAMON_MIN_REGION, esz);
 	}
 
 	if (quota->sz && quota->sz < esz)
@@ -2186,7 +2188,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	/* First charge window */
 	if (!quota->total_charged_sz && !quota->charged_from) {
 		quota->charged_from = jiffies;
-		damos_set_effective_quota(quota);
+		damos_set_effective_quota(quota, c);
 	}
 
 	/* New charge window starts */
@@ -2199,7 +2201,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 		quota->charged_sz = 0;
 		if (trace_damos_esz_enabled())
 			cached_esz = quota->esz;
-		damos_set_effective_quota(quota);
+		damos_set_effective_quota(quota, c);
 		if (trace_damos_esz_enabled() && quota->esz != cached_esz)
 			damos_trace_esz(c, s, quota);
 	}
-- 
2.47.3


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

* Re: [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz
  2026-05-04 13:01 ` [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz SeongJae Park
@ 2026-05-04 17:06   ` sashiko-bot
  2026-05-06 16:49     ` SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-05-04 17:06 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `ctx` parameter in `damos_set_effective_quota()` is unused, and the function incorrectly uses the `DAMON_MIN_REGION` macro instead of `ctx->min_sz_region`.
--

commit 8afd735e4c663dcbcca090e71f37ad10c37c705a
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: disallow time-quota setting zero esz

When DAMOS scheme throughput is very slow, the time quota can make the
effective size quota smaller than the context's minimum region size, causing
actions to be skipped. This commit attempts to fix the problem by ensuring
the effective size quota is at least the minimum region size.

Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2124,7 +2124,8 @@ static unsigned long damos_quota_score(struct damos_quota *quota)
>  /*
>   * Called only if quota->ms, or quota->sz are set, or quota->goals is not empty
>   */
> -static void damos_set_effective_quota(struct damos_quota *quota)
> +static void damos_set_effective_quota(struct damos_quota *quota,
> +		struct damon_ctx *ctx)
>  {
>  	unsigned long throughput;
>  	unsigned long esz = ULONG_MAX;

[ ... ]

> @@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
>  		else
>  			throughput = PAGE_SIZE * 1024;
>  		esz = min(throughput * quota->ms, esz);
> +		esz = max(DAMON_MIN_REGION, esz);

Should this be using ctx->min_sz_region instead of DAMON_MIN_REGION?

The commit message mentions bounding the effective size quota to
damon_ctx->min_region_sz, and the ctx parameter was successfully added
to the function signature. 

However, the ctx parameter is left unused here and the macro is used as a
hardcoded bound instead. Since ctx->min_sz_region can be dynamically
configured, can the effective size quota still fall below it with this
implementation?

>  	}
>  
>  	if (quota->sz && quota->sz < esz)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504130127.25364-1-sj@kernel.org?part=1

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

* Re: [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz
  2026-05-04 17:06   ` sashiko-bot
@ 2026-05-06 16:49     ` SeongJae Park
  2026-05-06 16:52       ` [PATCH 6.18.y v2] " SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-05-06 16:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, sashiko-bot; +Cc: SeongJae Park, stable, damon

TL; DR: This patch is broken, please drop.  I will send v2 soon, as a reply to
this.  Please read below for details.

On Mon, 04 May 2026 17:06:06 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `ctx` parameter in `damos_set_effective_quota()` is unused, and the function incorrectly uses the `DAMON_MIN_REGION` macro instead of `ctx->min_sz_region`.
> --
> 
> commit 8afd735e4c663dcbcca090e71f37ad10c37c705a
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: disallow time-quota setting zero esz
> 
> When DAMOS scheme throughput is very slow, the time quota can make the
> effective size quota smaller than the context's minimum region size, causing
> actions to be skipped. This commit attempts to fix the problem by ensuring
> the effective size quota is at least the minimum region size.
> 
> Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
> Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2124,7 +2124,8 @@ static unsigned long damos_quota_score(struct damos_quota *quota)
> >  /*
> >   * Called only if quota->ms, or quota->sz are set, or quota->goals is not empty
> >   */
> > -static void damos_set_effective_quota(struct damos_quota *quota)
> > +static void damos_set_effective_quota(struct damos_quota *quota,
> > +		struct damon_ctx *ctx)
> >  {
> >  	unsigned long throughput;
> >  	unsigned long esz = ULONG_MAX;
> 
> [ ... ]
> 
> > @@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
> >  		else
> >  			throughput = PAGE_SIZE * 1024;
> >  		esz = min(throughput * quota->ms, esz);
> > +		esz = max(DAMON_MIN_REGION, esz);
> 
> Should this be using ctx->min_sz_region instead of DAMON_MIN_REGION?
> 
> The commit message mentions bounding the effective size quota to
> damon_ctx->min_region_sz, and the ctx parameter was successfully added
> to the function signature. 
> 
> However, the ctx parameter is left unused here and the macro is used as a
> hardcoded bound instead. Since ctx->min_sz_region can be dynamically
> configured, can the effective size quota still fall below it with this
> implementation?

Correct.  Cherry-picking the original commit was causing build issue since the
upstream commit is renamed field (min_region_sz).  When fixing the build issue
I mistakenly assumed the field itslef is not yet introduced to 6.18 and using
DAMON_MIN_REGION.  It should use the field that not yet renamed
(min_sz_region), as Sashiko pointed out.

Greg, please drop this.  I will send a v2 soon, as a reply to this mail.

> 
> >  	}
> >  
> >  	if (quota->sz && quota->sz < esz)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260504130127.25364-1-sj@kernel.org?part=1
> 


Thanks,
SJ

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

* [PATCH 6.18.y v2] mm/damon/core: disallow time-quota setting zero esz
  2026-05-06 16:49     ` SeongJae Park
@ 2026-05-06 16:52       ` SeongJae Park
  2026-05-06 20:40         ` sashiko-bot
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-05-06 16:52 UTC (permalink / raw)
  To: stable; +Cc: damon, Greg Kroah-Hartman, SeongJae Park, Andrew Morton

When the throughput of a DAMOS scheme is very slow, DAMOS time quota can
make the effective size quota smaller than damon_ctx->min_region_sz.  In
the case, damos_apply_scheme() will skip applying the action, because the
action is tried at region level, which requires >=min_region_sz size.
That is, the quota is effectively exceeded for the quota charge window.

Because no action will be applied, the total_charged_sz and
total_charged_ns are also not updated.  damos_set_effective_quota() will
try to update the effective size quota before starting the next charge
window.  However, because the total_charged_sz and total_charged_ns have
not updated, the throughput and effective size quota are also not changed.
Since effective size quota can only be decreased, other effective size
quota update factors including DAMOS quota goals and size quota cannot
make any change, either.

As a result, the scheme is unexpectedly deactivated until the user notices
and mitigates the situation.  The users can mitigate this situation by
changing the time quota online or re-install the scheme.  While the
mitigation is somewhat straightforward, finding the situation would be
challenging, because DAMON is not providing good observabilities for that.
Even if such observability is provided, doing the additional monitoring
and the mitigation is somewhat cumbersome and not aligned to the intention
of the time quota.  The time quota was intended to help reduce the user's
administration overhead.

Fix the problem by setting time quota-modified effective size quota be at
least min_region_sz always.

The issue was discovered [1] by sashiko.

Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]
Fixes: 1cd243030059 ("mm/damon/schemes: implement time quota")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 8bbde987c2b84f80da0853f739f0a920386f8b99)
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 87b6c9c2d647..feec1b0c2bab 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2124,7 +2124,8 @@ static unsigned long damos_quota_score(struct damos_quota *quota)
 /*
  * Called only if quota->ms, or quota->sz are set, or quota->goals is not empty
  */
-static void damos_set_effective_quota(struct damos_quota *quota)
+static void damos_set_effective_quota(struct damos_quota *quota,
+		struct damon_ctx *ctx)
 {
 	unsigned long throughput;
 	unsigned long esz = ULONG_MAX;
@@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
 		else
 			throughput = PAGE_SIZE * 1024;
 		esz = min(throughput * quota->ms, esz);
+		esz = max(ctx->min_sz_region, esz);
 	}
 
 	if (quota->sz && quota->sz < esz)
@@ -2186,7 +2188,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	/* First charge window */
 	if (!quota->total_charged_sz && !quota->charged_from) {
 		quota->charged_from = jiffies;
-		damos_set_effective_quota(quota);
+		damos_set_effective_quota(quota, c);
 	}
 
 	/* New charge window starts */
@@ -2199,7 +2201,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 		quota->charged_sz = 0;
 		if (trace_damos_esz_enabled())
 			cached_esz = quota->esz;
-		damos_set_effective_quota(quota);
+		damos_set_effective_quota(quota, c);
 		if (trace_damos_esz_enabled() && quota->esz != cached_esz)
 			damos_trace_esz(c, s, quota);
 	}
-- 
2.47.3


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

* Re: [PATCH 6.18.y v2] mm/damon/core: disallow time-quota setting zero esz
  2026-05-06 16:52       ` [PATCH 6.18.y v2] " SeongJae Park
@ 2026-05-06 20:40         ` sashiko-bot
  2026-05-07  8:12           ` SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-05-06 20:40 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete fix: DAMOS scheme can still unexpectedly deactivate if using quota goals without a time quota.
--

commit ce7d104c8087fff14fb782b89848659312c204fa
Author: SeongJae Park <sj@kernel.org>
Subject: mm/damon/core: disallow time-quota setting zero esz

This commit addresses an issue where DAMOS time quota can reduce the
effective size quota below the minimum region size. This causes no actions
to be applied, unexpectedly deactivating the scheme. The fix enforces a
lower bound of min_sz_region on the effective size quota when a time quota
is used.

Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 87b6c9c2d6471..feec1b0c2babe 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
>  		else
>  			throughput = PAGE_SIZE * 1024;
>  		esz = min(throughput * quota->ms, esz);
> +		esz = max(ctx->min_sz_region, esz);
>  	}
>  
>  	if (quota->sz && quota->sz < esz)

Could this still leave a case where the scheme is unexpectedly deactivated
if quota goals are used without a time quota?

If a scheme is configured with quota goals (!list_empty(&quota->goals)) but
no time quota (quota->ms == 0), the bounds check added here inside the
if (quota->ms) block is bypassed.

When the system metric over-achieves, the feedback loop in
damon_feed_loop_next_input() can limit quota->esz_bp to its minimum of
10000, which evaluates to an esz of 1:

	if (!list_empty(&quota->goals)) {
		unsigned long score = damos_quota_score(quota);

		quota->esz_bp = damon_feed_loop_next_input(
				max(quota->esz_bp, 10000UL),
				score);
		esz = quota->esz_bp / 10000;
	}

During scheme application, damos_apply_scheme() calculates the size using
ALIGN_DOWN(1 - 0, ctx->min_sz_region). Since min_sz_region is typically
PAGE_SIZE, this evaluates to 0 and the action is skipped entirely.

Because the feedback loop increases esz_bp multiplicatively, it would take
multiple charge windows for esz to grow from 1 back to min_sz_region after
the score drops. During this period, the scheme applies 0 actions despite
the score indicating they are needed.

Should the max(ctx->min_sz_region, esz) bound be applied outside the
if (quota->ms) block to ensure any auto-tuned esz is properly bounded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506165211.10410-1-sj@kernel.org?part=1

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

* Re: [PATCH 6.18.y v2] mm/damon/core: disallow time-quota setting zero esz
  2026-05-06 20:40         ` sashiko-bot
@ 2026-05-07  8:12           ` SeongJae Park
  0 siblings, 0 replies; 6+ messages in thread
From: SeongJae Park @ 2026-05-07  8:12 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SeongJae Park, damon

On Wed, 06 May 2026 20:40:45 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incomplete fix: DAMOS scheme can still unexpectedly deactivate if using quota goals without a time quota.

No valid issue.  Read below for details, please.

> --
> 
> commit ce7d104c8087fff14fb782b89848659312c204fa
> Author: SeongJae Park <sj@kernel.org>
> Subject: mm/damon/core: disallow time-quota setting zero esz
> 
> This commit addresses an issue where DAMOS time quota can reduce the
> effective size quota below the minimum region size. This causes no actions
> to be applied, unexpectedly deactivating the scheme. The fix enforces a
> lower bound of min_sz_region on the effective size quota when a time quota
> is used.
> 
> Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
> Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 87b6c9c2d6471..feec1b0c2babe 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
> >  		else
> >  			throughput = PAGE_SIZE * 1024;
> >  		esz = min(throughput * quota->ms, esz);
> > +		esz = max(ctx->min_sz_region, esz);
> >  	}
> >  
> >  	if (quota->sz && quota->sz < esz)
> 
> Could this still leave a case where the scheme is unexpectedly deactivated
> if quota goals are used without a time quota?
> 
> If a scheme is configured with quota goals (!list_empty(&quota->goals)) but
> no time quota (quota->ms == 0), the bounds check added here inside the
> if (quota->ms) block is bypassed.
> 
> When the system metric over-achieves, the feedback loop in
> damon_feed_loop_next_input() can limit quota->esz_bp to its minimum of
> 10000, which evaluates to an esz of 1:
> 
> 	if (!list_empty(&quota->goals)) {
> 		unsigned long score = damos_quota_score(quota);
> 
> 		quota->esz_bp = damon_feed_loop_next_input(
> 				max(quota->esz_bp, 10000UL),
> 				score);
> 		esz = quota->esz_bp / 10000;
> 	}
> 
> During scheme application, damos_apply_scheme() calculates the size using
> ALIGN_DOWN(1 - 0, ctx->min_sz_region). Since min_sz_region is typically
> PAGE_SIZE, this evaluates to 0 and the action is skipped entirely.
> 
> Because the feedback loop increases esz_bp multiplicatively, it would take
> multiple charge windows for esz to grow from 1 back to min_sz_region after
> the score drops. During this period, the scheme applies 0 actions despite
> the score indicating they are needed.

Yes, and this is the intended behavior.

> 
> Should the max(ctx->min_sz_region, esz) bound be applied outside the
> if (quota->ms) block to ensure any auto-tuned esz is properly bounded?

Because it is an intended behavior, no.


Thanks,
SJ

[...]

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

end of thread, other threads:[~2026-05-07  8:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2026050306-june-passerby-c0a0@gregkh>
2026-05-04 13:01 ` [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz SeongJae Park
2026-05-04 17:06   ` sashiko-bot
2026-05-06 16:49     ` SeongJae Park
2026-05-06 16:52       ` [PATCH 6.18.y v2] " SeongJae Park
2026-05-06 20:40         ` sashiko-bot
2026-05-07  8:12           ` SeongJae Park

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