[RFC PATCH v1.1 0/3] mm/damon: fix wrong behaviors in DAMOS quota goals and sysfs refresh_ms

[RFC PATCH v1.1 0/3] mm/damon: fix wrong behaviors in DAMOS quota goals and sysfs refresh_ms

[SJ Park]

DAMOS quota goals and DAMON sysfs refresh_ms file have bugs that can
produce non critical but still unexpected behaviors.  Fix the two bugs.

Patches 1 and 2 fix the bugs in DAMOS PSI goal initialization and
eligible_mem_bp online commit, respectively.  Patch 3 fixes the bug in
DAMON sysfs refresh_ms file handling.

Changes from RFC
- RFC: https://lore.kernel.org/20260827042920.93580-1-sj@kernel.org
  (Mistakenly missed RFC tag)
- Fix build error.
- Add eligible_mem_bp nid commit fix.

SJ Park (3):
  mm/damon/core: initialize damos_quota_goal->last_psi_total
  mm/damon/core: copy nid for eligible_mem_bp damos quota goal commit
  mm/damon/sysfs: set next refresh jiffies per sysfs context

 mm/damon/core.c  | 21 ++++++++++++++++++++-
 mm/damon/sysfs.c | 11 +++++------
 2 files changed, 25 insertions(+), 7 deletions(-)


base-commit: 9e11dd3caaf0ac70acdc0f568265a7fa5085ef42
-- 
2.47.3

[RFC PATCH v1.1 1/3] mm/damon/core: initialize damos_quota_goal->last_psi_total

[SJ Park]

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.  Fixing it is also simple and not expensive.

Initialize the field at the beginning of kdamond and each commit of
SOME_MEM_PSI_US type quota goals.

The issue was discovered [1] by Sashiko.

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

Fixes: 2dbb60f789cb ("mm/damon/core: implement PSI metric DAMOS quota goal")
Cc: <stable@vger.kernel.org> # 6.9.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index f8dddbff74a77..e5b6b1c6bb7b2 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1118,10 +1118,15 @@ static struct damos_quota_goal *damos_nth_quota_goal(
 	return NULL;
 }
 
+static u64 damos_get_some_mem_psi_total(void);
+
 static void damos_commit_quota_goal_union(
 		struct damos_quota_goal *dst, struct damos_quota_goal *src)
 {
 	switch (dst->metric) {
+	case DAMOS_QUOTA_SOME_MEM_PSI_US:
+		dst->last_psi_total = damos_get_some_mem_psi_total();
+		break;
 	case DAMOS_QUOTA_NODE_MEM_USED_BP:
 	case DAMOS_QUOTA_NODE_MEM_FREE_BP:
 		dst->nid = src->nid;
@@ -1143,7 +1148,6 @@ static void 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 */
 	damos_commit_quota_goal_union(dst, src);
 }
 
@@ -3741,6 +3745,17 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
 	return -EBUSY;
 }
 
+static void damos_init_quota_goal_last_psi(struct damos *s)
+{
+	struct damos_quota_goal *goal;
+
+	damos_for_each_quota_goal(goal, &s->quota) {
+		if (goal->metric != DAMOS_QUOTA_SOME_MEM_PSI_US)
+			continue;
+		goal->last_psi_total = damos_get_some_mem_psi_total();
+	}
+}
+
 static void kdamond_init_ctx(struct damon_ctx *ctx)
 {
 	unsigned long sample_interval = ctx->attrs.sample_interval ?
@@ -3757,6 +3772,7 @@ static void kdamond_init_ctx(struct damon_ctx *ctx)
 	damon_for_each_scheme(scheme, ctx) {
 		damos_set_next_apply_sis(scheme, ctx);
 		damos_set_filters_default_reject(scheme);
+		damos_init_quota_goal_last_psi(scheme);
 	}
 }
 
-- 
2.47.3

Re: [RFC PATCH v1.1 1/3] mm/damon/core: initialize damos_quota_goal->last_psi_total

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditionally resetting last_psi_total in damos_commit_quota_goal_union() breaks the feedback loop measurement interval for existing goals during sysfs updates.
- [Medium] The patch changes the semantics of damos_commit_quota_goal() to explicitly overwrite last_psi_total instead of preserving it, but leaves behind a KUnit test assertion that expects preservation.
--

commit 8007d7351717974f3d1ef45ed438346542307389
Author: SJ Park <sj@kernel.org>

mm/damon/core: initialize damos_quota_goal->last_psi_total

This patch initializes the last_psi_total for DAMOS_QUOTA_SOME_MEM_PSI_US
quota goals to prevent the first iteration of the feedback loop from using
an uninitialized value. It introduces initialization at the start of kdamond
and during each goal commit.

Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..723ab998b63c7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1118,10 +1118,15 @@ static struct damos_quota_goal *damos_nth_quota_goal(
>  	return NULL;
>  }
>  
> +static u64 damos_get_some_mem_psi_total(void);
> +
>  static void damos_commit_quota_goal_union(
>  		struct damos_quota_goal *dst, struct damos_quota_goal *src)
>  {
>  	switch (dst->metric) {
> +	case DAMOS_QUOTA_SOME_MEM_PSI_US:
> +		dst->last_psi_total = damos_get_some_mem_psi_total();
> +		break;

[Severity: High]
Does this unconditional reset of last_psi_total truncate the measurement
window for running goals that are updated via sysfs?

When a user updates a running DAMON context via sysfs that includes an
existing DAMOS_QUOTA_SOME_MEM_PSI_US goal, this overwrites the running
goal's last_psi_total with the current time's PSI value, without resetting
the overall charged_from interval timer. 

Could this produce an artificially small current_value, causing the feedback
loop tuner to falsely assume memory pressure is low and erroneously increase
the scheme's quota?

>  	case DAMOS_QUOTA_NODE_MEM_USED_BP:
>  	case DAMOS_QUOTA_NODE_MEM_FREE_BP:
>  		dst->nid = src->nid;

[ ... ]

> @@ -1143,7 +1148,6 @@ static void 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 */
>  	damos_commit_quota_goal_union(dst, src);
>  }

[Severity: Medium]
Since the semantics of damos_commit_quota_goal() have changed to explicitly
overwrite last_psi_total rather than preserve it, should the corresponding
KUnit test be updated as well?

The existing KUnit test in mm/damon/tests/core-kunit.h
(damos_test_commit_quota_goal_for()) expects the old preservation behavior:

if (dst_last_psi_total && src->metric == DAMOS_QUOTA_SOME_MEM_PSI_US)
	KUNIT_EXPECT_EQ(test, dst->last_psi_total, dst_last_psi_total);

Without updating this assertion, there appears to be a mismatch between the
new code behavior and what the test expects.

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

Re: [RFC PATCH v1.1 1/3] mm/damon/core: initialize damos_quota_goal->last_psi_total

[SJ Park]

On Thu, 27 Aug 2026 13:59:58 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unconditionally resetting last_psi_total in damos_commit_quota_goal_union() breaks the feedback loop measurement interval for existing goals during sysfs updates.
> - [Medium] The patch changes the semantics of damos_commit_quota_goal() to explicitly overwrite last_psi_total instead of preserving it, but leaves behind a KUnit test assertion that expects preservation.
> --
> 
> commit 8007d7351717974f3d1ef45ed438346542307389
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: initialize damos_quota_goal->last_psi_total
> 
> This patch initializes the last_psi_total for DAMOS_QUOTA_SOME_MEM_PSI_US
> quota goals to prevent the first iteration of the feedback loop from using
> an uninitialized value. It introduces initialization at the start of kdamond
> and during each goal commit.
> 
> Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 644daf5a16560..723ab998b63c7 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1118,10 +1118,15 @@ static struct damos_quota_goal *damos_nth_quota_goal(
> >  	return NULL;
> >  }
> >  
> > +static u64 damos_get_some_mem_psi_total(void);
> > +
> >  static void damos_commit_quota_goal_union(
> >  		struct damos_quota_goal *dst, struct damos_quota_goal *src)
> >  {
> >  	switch (dst->metric) {
> > +	case DAMOS_QUOTA_SOME_MEM_PSI_US:
> > +		dst->last_psi_total = damos_get_some_mem_psi_total();
> > +		break;
> 
> [Severity: High]
> Does this unconditional reset of last_psi_total truncate the measurement
> window for running goals that are updated via sysfs?
> 
> When a user updates a running DAMON context via sysfs that includes an
> existing DAMOS_QUOTA_SOME_MEM_PSI_US goal, this overwrites the running
> goal's last_psi_total with the current time's PSI value, without resetting
> the overall charged_from interval timer. 
> 
> Could this produce an artificially small current_value, causing the feedback
> loop tuner to falsely assume memory pressure is low and erroneously increase
> the scheme's quota?

When the entire context is committed, damos->next_apply_sis is also updated, so
this problem doesn't happen.  However, we allow commit_schemes_quota_goals
sysfs command to directly commit goals.  In the case, this issue can happen.

We could handle the corner case.  But I start feeling this is adding too much
complexity for the small bug.  As also mentioned on the original commit
message, the impact is transient and minor.  Users can also set additional
quotas or quota goals as guardrails.  I will keep the original behavior as is,
and just add comments about it.  We can revisit if a real user gets real
problem from the documented behavior.

> 
> >  	case DAMOS_QUOTA_NODE_MEM_USED_BP:
> >  	case DAMOS_QUOTA_NODE_MEM_FREE_BP:
> >  		dst->nid = src->nid;
> 
> [ ... ]
> 
> > @@ -1143,7 +1148,6 @@ static void 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 */
> >  	damos_commit_quota_goal_union(dst, src);
> >  }
> 
> [Severity: Medium]
> Since the semantics of damos_commit_quota_goal() have changed to explicitly
> overwrite last_psi_total rather than preserve it, should the corresponding
> KUnit test be updated as well?
> 
> The existing KUnit test in mm/damon/tests/core-kunit.h
> (damos_test_commit_quota_goal_for()) expects the old preservation behavior:
> 
> if (dst_last_psi_total && src->metric = DAMOS_QUOTA_SOME_MEM_PSI_US)
> 	KUNIT_EXPECT_EQ(test, dst->last_psi_total, dst_last_psi_total);
> 
> Without updating this assertion, there appears to be a mismatch between the
> new code behavior and what the test expects.

Good catch.  I usually run kunit without CONFIG_PSI, so didn't encounter the
issue.  However, as I'm changing my mind to keep the behavior but just update
the documents, this is irrelevant.

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


Thanks,
SJ

Re: [RFC PATCH v1.1 1/3] mm/damon/core: initialize damos_quota_goal->last_psi_total

[SJ Park]

On Thu, 27 Aug 2026 07:17:37 -0700 SJ Park <sj@kernel.org> wrote:

> On Thu, 27 Aug 2026 13:59:58 +0000 sashiko-bot@kernel.org wrote:
[...]
> > [Severity: High]
> > Does this unconditional reset of last_psi_total truncate the measurement
> > window for running goals that are updated via sysfs?
> > 
> > When a user updates a running DAMON context via sysfs that includes an
> > existing DAMOS_QUOTA_SOME_MEM_PSI_US goal, this overwrites the running
> > goal's last_psi_total with the current time's PSI value, without resetting
> > the overall charged_from interval timer. 
> > 
> > Could this produce an artificially small current_value, causing the feedback
> > loop tuner to falsely assume memory pressure is low and erroneously increase
> > the scheme's quota?
> 
> When the entire context is committed, damos->next_apply_sis is also updated, so
> this problem doesn't happen.  However, we allow commit_schemes_quota_goals
> sysfs command to directly commit goals.  In the case, this issue can happen.
> 
> We could handle the corner case.  But I start feeling this is adding too much
> complexity for the small bug.  As also mentioned on the original commit
> message, the impact is transient and minor.  Users can also set additional
> quotas or quota goals as guardrails.  I will keep the original behavior as is,
> and just add comments about it.  We can revisit if a real user gets real
> problem from the documented behavior.

While writing the documentation I found myself don't really like this random
behavior.  Another solution is initializing the field as UINT64_MAX at new
quota goal creation (damos_new_quota_goal()).  When the ->current_value for the
goal is calculated in damos_set_quota_goal_current_value(), it can check the
UINT64_MAX case and set the current value same to target value, so that this
tuning round make no change to the tuned quota.  The last_psi_total will be
updated to real value and make effect from the next round.

damos_commit_quota_goal_union() will also be updated to set the destination
goal's last_psi_total to the source goal's value.  Let's assume the source was
made with damos_new_quota_goal() and the last_psi_total field is not modified.
It should be a safe assumption since normal API callers are supposed to use
damos_new_quota_goal(), and don't reuse a goal that were updated by kdamond.
So the next round tuning will be skipped, but works without unexpected effect
from the next round.  If the destination goal was the PSI goal, this makes the
goal to skip next round auto-tuning for no good reason.  But that should be
fine.  DAMOS auto-tuning is basically for long term best-effort, not short-term
fully deterministic control.

> 
> > 
> > >  	case DAMOS_QUOTA_NODE_MEM_USED_BP:
> > >  	case DAMOS_QUOTA_NODE_MEM_FREE_BP:
> > >  		dst->nid = src->nid;
> > 
> > [ ... ]
> > 
> > > @@ -1143,7 +1148,6 @@ static void 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 */
> > >  	damos_commit_quota_goal_union(dst, src);
> > >  }
> > 
> > [Severity: Medium]
> > Since the semantics of damos_commit_quota_goal() have changed to explicitly
> > overwrite last_psi_total rather than preserve it, should the corresponding
> > KUnit test be updated as well?
> > 
> > The existing KUnit test in mm/damon/tests/core-kunit.h
> > (damos_test_commit_quota_goal_for()) expects the old preservation behavior:
> > 
> > if (dst_last_psi_total && src->metric = DAMOS_QUOTA_SOME_MEM_PSI_US)
> > 	KUNIT_EXPECT_EQ(test, dst->last_psi_total, dst_last_psi_total);
> > 
> > Without updating this assertion, there appears to be a mismatch between the
> > new code behavior and what the test expects.
> 
> Good catch.  I usually run kunit without CONFIG_PSI, so didn't encounter the
> issue.  However, as I'm changing my mind to keep the behavior but just update
> the documents, this is irrelevant.

So relevant kunit test case may also be updated.


Thanks,
SJ

[...]

Re: [RFC PATCH v1.1 1/3] mm/damon/core: initialize damos_quota_goal->last_psi_total

[SJ Park]

On Thu, 27 Aug 2026 07:50:33 -0700 SJ Park <sj@kernel.org> wrote:

> On Thu, 27 Aug 2026 07:17:37 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > On Thu, 27 Aug 2026 13:59:58 +0000 sashiko-bot@kernel.org wrote:
> [...]
> > > [Severity: High]
> > > Does this unconditional reset of last_psi_total truncate the measurement
> > > window for running goals that are updated via sysfs?
> > > 
> > > When a user updates a running DAMON context via sysfs that includes an
> > > existing DAMOS_QUOTA_SOME_MEM_PSI_US goal, this overwrites the running
> > > goal's last_psi_total with the current time's PSI value, without resetting
> > > the overall charged_from interval timer. 
> > > 
> > > Could this produce an artificially small current_value, causing the feedback
> > > loop tuner to falsely assume memory pressure is low and erroneously increase
> > > the scheme's quota?
> > 
> > When the entire context is committed, damos->next_apply_sis is also updated, so
> > this problem doesn't happen.  However, we allow commit_schemes_quota_goals
> > sysfs command to directly commit goals.  In the case, this issue can happen.
> > 
> > We could handle the corner case.  But I start feeling this is adding too much
> > complexity for the small bug.  As also mentioned on the original commit
> > message, the impact is transient and minor.  Users can also set additional
> > quotas or quota goals as guardrails.  I will keep the original behavior as is,
> > and just add comments about it.  We can revisit if a real user gets real
> > problem from the documented behavior.
> 
> While writing the documentation I found myself don't really like this random
> behavior.  Another solution is initializing the field as UINT64_MAX at new
> quota goal creation (damos_new_quota_goal()).  When the ->current_value for the
> goal is calculated in damos_set_quota_goal_current_value(), it can check the
> UINT64_MAX case and set the current value same to target value, so that this
> tuning round make no change to the tuned quota.  The last_psi_total will be
> updated to real value and make effect from the next round.
> 
> damos_commit_quota_goal_union() will also be updated to set the destination
> goal's last_psi_total to the source goal's value.  Let's assume the source was
> made with damos_new_quota_goal() and the last_psi_total field is not modified.
> It should be a safe assumption since normal API callers are supposed to use
> damos_new_quota_goal(), and don't reuse a goal that were updated by kdamond.

Maybe a better approach is drop the assumption and always set it to U64_MAX.
It may unnecessarily skip one tuning round if the destination was PSI goal.
The impact is modest, and make the behavior more deterministic.

> So the next round tuning will be skipped, but works without unexpected effect
> from the next round.  If the destination goal was the PSI goal, this makes the
> goal to skip next round auto-tuning for no good reason.  But that should be
> fine.  DAMOS auto-tuning is basically for long term best-effort, not short-term
> fully deterministic control.
> 
> > 
> > > 
> > > >  	case DAMOS_QUOTA_NODE_MEM_USED_BP:
> > > >  	case DAMOS_QUOTA_NODE_MEM_FREE_BP:
> > > >  		dst->nid = src->nid;
> > > 
> > > [ ... ]
> > > 
> > > > @@ -1143,7 +1148,6 @@ static void 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 */
> > > >  	damos_commit_quota_goal_union(dst, src);
> > > >  }
> > > 
> > > [Severity: Medium]
> > > Since the semantics of damos_commit_quota_goal() have changed to explicitly
> > > overwrite last_psi_total rather than preserve it, should the corresponding
> > > KUnit test be updated as well?
> > > 
> > > The existing KUnit test in mm/damon/tests/core-kunit.h
> > > (damos_test_commit_quota_goal_for()) expects the old preservation behavior:
> > > 
> > > if (dst_last_psi_total && src->metric = DAMOS_QUOTA_SOME_MEM_PSI_US)
> > > 	KUNIT_EXPECT_EQ(test, dst->last_psi_total, dst_last_psi_total);
> > > 
> > > Without updating this assertion, there appears to be a mismatch between the
> > > new code behavior and what the test expects.
> > 
> > Good catch.  I usually run kunit without CONFIG_PSI, so didn't encounter the
> > issue.  However, as I'm changing my mind to keep the behavior but just update
> > the documents, this is irrelevant.
> 
> So relevant kunit test case may also be updated.

And I found the current test is broken.  It is not really testing committing
PSI goal to PSI goal case.  I will fix it, too.


Thanks,
SJ

[...]

[RFC PATCH v1.1 2/3] mm/damon/core: copy nid for eligible_mem_bp damos quota goal commit

[SJ Park]

damos_commit_quota_goal_union() is not updating the ->nid union field
when the goal metric is DAMOS_QUITA_NODE_ELIGIBLE_MEM_BP.  Hence, if a
DAMOS quota goal of the type is online committed in a way that it will
reuse other quota goal's memory space, the new goal will work with a
garbage nid value.  As a result, the DAMOS scheme can show unexpected
aggressiveness.  Do the update.

The user impact is not catastrophic.  No leak or crash happens.  Doing
the quota goal online commit that can reproduce the issue is expected to
be not common.  This issue was not found by real users but the AI
review.  That said, the issue can reliably be reproduced.

This issue was discovered [1] by Sashiko.

[1] https://lore.kkernel.org/20260827045035.94611-1-sj@kernel.org

Fixes: 9138e27a3bc3 ("mm/damon: add node_eligible_mem_bp goal metric")
Cc: <stable@vger.kernel.org> # 7.2.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index e5b6b1c6bb7b2..11b20a4a726f9 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1136,6 +1136,9 @@ static void damos_commit_quota_goal_union(
 		dst->nid = src->nid;
 		dst->memcg_id = src->memcg_id;
 		break;
+	case DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP:
+		dst->nid = src->nid;
+		break;
 	default:
 		break;
 	}
-- 
2.47.3

Re: [RFC PATCH v1.1 2/3] mm/damon/core: copy nid for eligible_mem_bp damos quota goal commit

[sashiko-bot]

> damos_commit_quota_goal_union() is not updating the ->nid union field
> when the goal metric is DAMOS_QUITA_NODE_ELIGIBLE_MEM_BP.  Hence, if a
> DAMOS quota goal of the type is online committed in a way that it will
> reuse other quota goal's memory space, the new goal will work with a
> garbage nid value.  As a result, the DAMOS scheme can show unexpected
> aggressiveness.  Do the update.
> 
> The user impact is not catastrophic.  No leak or crash happens.  Doing
> the quota goal online commit that can reproduce the issue is expected to
> be not common.  This issue was not found by real users but the AI
> review.  That said, the issue can reliably be reproduced.
> 
> This issue was discovered [1] by Sashiko.
> 
> [1] https://lore.kkernel.org/20260827045035.94611-1-sj@kernel.org
> 
> Fixes: 9138e27a3bc3 ("mm/damon: add node_eligible_mem_bp goal metric")
> Cc: <stable@vger.kernel.org> # 7.2.x
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[RFC PATCH v1.1 3/3] mm/damon/sysfs: set next refresh jiffies per sysfs context

[SJ Park]

When 'refresh_ms' is set, DAMON sysfs interface periodically updates
auto-tuned parameters and DAMOS stats.  The timestamp for the next
refresh is initialized when a DAMON context starts, and updated in its
damon_call() callback function.  That is, each DAMON context updates it.
However, the timestamp is a global variable that is shared with all the
contexts.  When there are multiple DAMON contexts having different
refresh_ms, the update frequency will be changed, depending on the order
of the contexts.  When there are multiple kdamonds, it will be even more
chaotic.  Fix the problem by having the timestamp per each context.

The user impact is not very critical.  It does not leak, corrupt or
crash.  The update will not be faster or slower than the lowest and
largest refresh_ms values of the contexts, respectively.  The user can
also manually ask the updates on demand using kdamond state commands.
That said, clearly this is a bug and can easily be reproduced.

Fixes: 9fd7bb5083d1 ("mm/damon/sysfs: change next_update_jiffies to a global variable")
Cc: <stable@vger.kernel.org> # 6.18.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index e3858ffab4b22..f05b256c90ee7 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1789,6 +1789,7 @@ struct damon_sysfs_kdamond {
 	struct damon_sysfs_contexts *contexts;
 	struct damon_ctx *damon_ctx;
 	unsigned int refresh_ms;
+	unsigned long next_refresh_jiffies;
 };
 
 static struct damon_sysfs_kdamond *damon_sysfs_kdamond_alloc(void)
@@ -2215,17 +2216,15 @@ static struct damon_ctx *damon_sysfs_build_ctx(
 	return ctx;
 }
 
-static unsigned long damon_sysfs_next_update_jiffies;
-
 static int damon_sysfs_repeat_call_fn(void *data)
 {
 	struct damon_sysfs_kdamond *sysfs_kdamond = data;
 
 	if (!sysfs_kdamond->refresh_ms)
 		return 0;
-	if (time_before(jiffies, damon_sysfs_next_update_jiffies))
+	if (time_before(jiffies, sysfs_kdamond->next_refresh_jiffies))
 		return 0;
-	damon_sysfs_next_update_jiffies = jiffies +
+	sysfs_kdamond->next_refresh_jiffies = jiffies +
 		msecs_to_jiffies(sysfs_kdamond->refresh_ms);
 
 	if (!mutex_trylock(&damon_sysfs_lock))
@@ -2273,8 +2272,8 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
 	}
 	kdamond->damon_ctx = ctx;
 
-	damon_sysfs_next_update_jiffies =
-		jiffies + msecs_to_jiffies(kdamond->refresh_ms);
+	kdamond->next_refresh_jiffies = jiffies +
+		msecs_to_jiffies(kdamond->refresh_ms);
 
 	repeat_call_control->fn = damon_sysfs_repeat_call_fn;
 	repeat_call_control->data = kdamond;
-- 
2.47.3

Re: [RFC PATCH v1.1 3/3] mm/damon/sysfs: set next refresh jiffies per sysfs context

[sashiko-bot]

> When 'refresh_ms' is set, DAMON sysfs interface periodically updates
> auto-tuned parameters and DAMOS stats.  The timestamp for the next
> refresh is initialized when a DAMON context starts, and updated in its
> damon_call() callback function.  That is, each DAMON context updates it.
> However, the timestamp is a global variable that is shared with all the
> contexts.  When there are multiple DAMON contexts having different
> refresh_ms, the update frequency will be changed, depending on the order
> of the contexts.  When there are multiple kdamonds, it will be even more
> chaotic.  Fix the problem by having the timestamp per each context.
> 
> The user impact is not very critical.  It does not leak, corrupt or
> crash.  The update will not be faster or slower than the lowest and
> largest refresh_ms values of the contexts, respectively.  The user can
> also manually ask the updates on demand using kdamond state commands.
> That said, clearly this is a bug and can easily be reproduced.
> 
> Fixes: 9fd7bb5083d1 ("mm/damon/sysfs: change next_update_jiffies to a global variable")
> Cc: <stable@vger.kernel.org> # 6.18.x
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

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