DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
@ 2026-11-09  1:13 SJ Park
  2026-11-09  1:28 ` sashiko-bot
  2026-11-09 15:52 ` Sasha Levin
  0 siblings, 2 replies; 4+ messages in thread
From: SJ Park @ 2026-11-09  1:13 UTC (permalink / raw)
  To: stable
  Cc: SJ Park, Andrew Morton, Brendan Higgins, David Gow, Quanmin Yan,
	damon, linux-kernel, linux-mm, Lian Wang, Kunwu Chan

When DAMOS_QUOTA_SOME_MEM_PSI_US metric damos quota goal is set, the PSI
delta for the feedback loop is calculated using
damos_quota_goal->last_psi_total.  However, it is initialized only after
the first feedback loop.  The first iteration of the loop uses the
uninitialized value.  As a result, the feedback loop can change the
effective quota in an unexpected way at the first iteration.

The user impact of the issue is not big, because the issue impacts only
the first iteration of the feedback loop.  The feedback loop also has an
internal cap of the quota adjustment.  The wrong adjustment will soon be
corrected over a few iterations.  For this reason, doing no initialization
at commit time was intentional.  It is also explicitly commented.  That
said, nobody likes behaviors that are unexpected or difficult to be
expected.

Check last_psi_total initialization and skip the tuning round when it is
not initialized.  For this, initialize last_psi_total with U64_MAX in the
goal creation and the goal commit time.  U64_MAX means the field is not
initialized.  The tuning round shows the value and adjusts it to guarantee
the current quota is maintained for the round, and last_psi_total is
correctly initialized on the next round.

Before this change, committing a new PSI goal on an existing PSI goal with
goal-only DAMON sysfs command (commit_schemes_quota_goals) just worked.
After this change, the tuning round right after the commit will be
unnecessarily skipped, because last_psi_total is unconditionally marked as
not initialized in the damos_commit_quota_goal_union().  This is an
intended tradeoff for simplicity.  Skipping just one round of tuning is no
problem.  Meanwhile it makes both the code and the behavior simple to
understand.

Also update the quota goal commit unit test for changed last_psi_total
setup behavior.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/20260902002725.108635-3-sj@kernel.org
Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org [1]
Fixes: 2dbb60f789cb ("mm/damon/core: implement PSI metric DAMOS quota goal")
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Lian Wang <lianux.mm@gmail.com>
Tested-by: Lian Wang <lianux.mm@gmail.com>
Reviewed-by: Kunwu Chan <kunwu.chan@gmail.com>
Cc: Brendan Higgins <brendan.higgins@linux.dev>
Cc: David Gow <davidgow@davidgow.net>
Cc: Quanmin Yan <yanquanmin1@huawei.com>
Cc: <stable@vger.kernel.org> # 6.9.x
(cherry picked from commit 29c6084658af41b5b1fefd873d26d441830ced34)
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v1
- v1: https://lore.kernel.org/20261108195145.115792-4-sj@kernel.org
- Split out from the series.
- Rebase to lastest stable-rc/linux-6.12.y (commit a100533c838ba).
Changes from RFC
- RFC: https://lore.kernel.org/20261107214525.304638-4-sj@kernel.org
- None.

 mm/damon/core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3142776e2e0ee..420ef0adaa7e3 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -324,6 +324,8 @@ struct damos_quota_goal *damos_new_quota_goal(
 		return NULL;
 	goal->metric = metric;
 	goal->target_value = target_value;
+	if (metric == DAMOS_QUOTA_SOME_MEM_PSI_US)
+		goal->last_psi_total = U64_MAX;
 	INIT_LIST_HEAD(&goal->list);
 	return goal;
 }
@@ -707,7 +709,8 @@ static int damos_commit_quota_goal(
 	dst->target_value = src->target_value;
 	if (dst->metric == DAMOS_QUOTA_USER_INPUT)
 		dst->current_value = src->current_value;
-	/* keep last_psi_total as is, since it will be updated in next cycle */
+	else if (dst->metric == DAMOS_QUOTA_SOME_MEM_PSI_US)
+		dst->last_psi_total = U64_MAX;
 	return 0;
 }
 
@@ -1616,7 +1619,12 @@ static void damos_set_quota_goal_current_value(struct damos_quota_goal *goal)
 		break;
 	case DAMOS_QUOTA_SOME_MEM_PSI_US:
 		now_psi_total = damos_get_some_mem_psi_total();
-		goal->current_value = now_psi_total - goal->last_psi_total;
+		/* uninitialized last_psi_total; make no effect this round */
+		if (goal->last_psi_total == U64_MAX)
+			goal->current_value = goal->target_value;
+		else
+			goal->current_value = now_psi_total -
+				goal->last_psi_total;
 		goal->last_psi_total = now_psi_total;
 		break;
 	default:

base-commit: a100533c838bab5a555d92e082e38e092f2b4ea6
-- 
2.47.3

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

* Re: [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
  2026-11-09  1:13 [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total SJ Park
@ 2026-11-09  1:28 ` sashiko-bot
  2026-11-09  1:34   ` SJ Park
  2026-11-09 15:52 ` Sasha Levin
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-11-09  1:28 UTC (permalink / raw)
  To: SJ Park
  Cc: Andrew Morton, Brendan Higgins, David Gow, Quanmin Yan, damon,
	linux-mm, Lian Wang, Kunwu Chan

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm/damon: integer overflow in damos_quota_score() on first
  PSI round
--

commit 5e9b1f636c184ecf41e05d717ad5e0116b0fca20
Author: SJ Park <sj@kernel.org>

mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total

This commit initializes damos_quota_goal->last_psi_total to U64_MAX when
creating or committing a DAMOS_QUOTA_SOME_MEM_PSI_US goal, and sets
goal->current_value to goal->target_value on the first tuning round so the
effective quota remains unchanged until a baseline PSI reading is recorded.

Link: https://lore.kernel.org/20260902002725.108635-3-sj@kernel.org
Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org [1]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3142776e2e0ee..420ef0adaa7e3 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1616,7 +1619,12 @@ static void damos_set_quota_goal_current_value(struct damos_quota_goal *goal)
>  		break;
>  	case DAMOS_QUOTA_SOME_MEM_PSI_US:
>  		now_psi_total = damos_get_some_mem_psi_total();
> -		goal->current_value = now_psi_total - goal->last_psi_total;
> +		/* uninitialized last_psi_total; make no effect this round */
> +		if (goal->last_psi_total == U64_MAX)
> +			goal->current_value = goal->target_value;
> +		else
> +			goal->current_value = now_psi_total -
> +				goal->last_psi_total;
>  		goal->last_psi_total = now_psi_total;
>  		break;
>  	default:

[Severity: Medium]
Can setting goal->current_value = goal->target_value here overflow in
damos_quota_score() when goal->target_value exceeds ULONG_MAX / 10000?

Unlike mainline where damos_quota_score() uses mult_frac(), this branch
computes the score with direct unsigned long multiplication:

mm/damon/core.c:damos_quota_score() {
	damos_for_each_quota_goal(goal, quota) {
		damos_set_quota_goal_current_value(goal);
		highest_score = max(highest_score,
				goal->current_value * 10000 /
				goal->target_value);
	}
}

When goal->target_value > ULONG_MAX / 10000 (for example, above 429,496 us
on 32-bit architectures), goal->current_value * 10000 wraps around and
produces a score below 10000, causing damon_feed_loop_next_input() to
increase quota->esz_bp on the first round instead of keeping the current
quota unchanged.

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

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

* Re: [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
  2026-11-09  1:28 ` sashiko-bot
@ 2026-11-09  1:34   ` SJ Park
  0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-11-09  1:34 UTC (permalink / raw)
  To: sashiko-bot
  Cc: SJ Park, Andrew Morton, Brendan Higgins, David Gow, Quanmin Yan,
	damon, linux-mm, Lian Wang, Kunwu Chan

On Mon, 09 Nov 2026 01:28:24 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm/damon: integer overflow in damos_quota_score() on first
>   PSI round
> --
> 
> commit 5e9b1f636c184ecf41e05d717ad5e0116b0fca20
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
> 
> This commit initializes damos_quota_goal->last_psi_total to U64_MAX when
> creating or committing a DAMOS_QUOTA_SOME_MEM_PSI_US goal, and sets
> goal->current_value to goal->target_value on the first tuning round so the
> effective quota remains unchanged until a baseline PSI reading is recorded.
> 
> Link: https://lore.kernel.org/20260902002725.108635-3-sj@kernel.org
> Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3142776e2e0ee..420ef0adaa7e3 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1616,7 +1619,12 @@ static void damos_set_quota_goal_current_value(struct damos_quota_goal *goal)
> >  		break;
> >  	case DAMOS_QUOTA_SOME_MEM_PSI_US:
> >  		now_psi_total = damos_get_some_mem_psi_total();
> > -		goal->current_value = now_psi_total - goal->last_psi_total;
> > +		/* uninitialized last_psi_total; make no effect this round */
> > +		if (goal->last_psi_total == U64_MAX)
> > +			goal->current_value = goal->target_value;
> > +		else
> > +			goal->current_value = now_psi_total -
> > +				goal->last_psi_total;
> >  		goal->last_psi_total = now_psi_total;
> >  		break;
> >  	default:
> 
> [Severity: Medium]
> Can setting goal->current_value = goal->target_value here overflow in
> damos_quota_score() when goal->target_value exceeds ULONG_MAX / 10000?
> 
> Unlike mainline where damos_quota_score() uses mult_frac(), this branch
> computes the score with direct unsigned long multiplication:
> 
> mm/damon/core.c:damos_quota_score() {
> 	damos_for_each_quota_goal(goal, quota) {
> 		damos_set_quota_goal_current_value(goal);
> 		highest_score = max(highest_score,
> 				goal->current_value * 10000 /
> 				goal->target_value);
> 	}
> }
> 
> When goal->target_value > ULONG_MAX / 10000 (for example, above 429,496 us
> on 32-bit architectures), goal->current_value * 10000 wraps around and
> produces a score below 10000, causing damon_feed_loop_next_input() to
> increase quota->esz_bp on the first round instead of keeping the current
> quota unchanged.

The issue impacts only DAMOS quota auto-tuning quality under certain setups.
No blocker for this patch.

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


Thanks,
SJ

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

* Re: [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
  2026-11-09  1:13 [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total SJ Park
  2026-11-09  1:28 ` sashiko-bot
@ 2026-11-09 15:52 ` Sasha Levin
  1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-11-09 15:52 UTC (permalink / raw)
  To: stable
  Cc: Sasha Levin, SJ Park, Andrew Morton, Brendan Higgins, David Gow,
	Quanmin Yan, damon, linux-kernel, linux-mm, Lian Wang, Kunwu Chan

> mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total

Queued for 6.12, thanks.

-- 
Thanks,
Sasha

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

end of thread, other threads:[~2026-11-09 15:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09  1:13 [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total SJ Park
2026-11-09  1:28 ` sashiko-bot
2026-11-09  1:34   ` SJ Park
2026-11-09 15:52 ` Sasha Levin

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