Re: [PATCH 1/2] mm/damon/core: preserve the caller's quota in damon_new_scheme()

[SJ Park]

On Mon, 21 Sep 2026 10:11:54 -0700 SJ Park <sj@xxxxxxxxxx> wrote:

> On Mon, 21 Sep 2026 02:30:46 +0200 Karl Mehltretter <kmehltretter@xxxxxxxxx> wrote:
> 
> > damon_new_scheme() calls damos_quota_init() on the caller's quota before
> > copying it to the new scheme. This clears the caller's effective quota,
> > feedback input and charging state as a side effect.
> 
> Apparently the above paragraph assumes it is called under damon_commit_ctx().
> Lack of the context makes this quite confusing.  Could you please rewrite?

To calrify my opinion more, "caller's quota" feels unclear to me.  I hope it to
be more clear that it means "the quota that is passed as a parameter to the
function".

> 
> > 
> > damon_commit_ctx() first copies the running context into a temporary
> > context for validating the proposed parameters.

I'd prefer using the term, 'commit' instead of 'copies' for clarity.

> > When
> > damon_commit_schemes() creates the temporary schemes, it passes the quota
> > of each running scheme to damon_new_scheme(). The quota pointer therefore
> > refers to the running scheme, and damos_quota_init() clears that scheme's
> > state before it is copied to the temporary scheme. Even an update
> > rejected with -EINVAL loses the running quota state.
> > 
> > For a size quota, this discards the bytes already charged and allows the
> > scheme to use a fresh quota before the reset interval has elapsed. For a
> > goal-driven quota, the consist tuner loses its accumulated input and
> > restarts from its minimum input. A time quota loses its throughput
> > estimate and falls back to the initial estimate.
> > 
> > The constructor side effect was introduced by commit 70e0c1d1bf94
> > ("mm/damon/core: factor out 'damos_quota' private fileds initialization").
> > Commit 60bd24f272d0 ("mm/damon/sysfs: test commit input against realistic
> > destination"), merged in v6.19, exposed it when
> > validating sysfs updates against a copy of the running context. Commit
> > b90408ef1163 ("mm/damon/core: safely validate src on damon_commit_ctx()")
> > later moved that validation into the core API.

I overlooked this part in the previous reply, sorry.  And thank you for adding
this detailed context.

> > 
> > Sashiko reported the same side effect [1] on the RFC of the core API
> > change.

Nice catch, I misunderstood Sashiko's point.  Thank you for catching this,
Karl.

> > 
> > Copy the quota to the new scheme first, then initialize that copy. Make
> > damos_quota_init() return void, since its return value is no longer needed.
> > 
> > Fixes: 70e0c1d1bf94 ("mm/damon/core: factor out 'damos_quota' private fileds initialization")
> > Cc: <stable@xxxxxxxxxxxxxxx> # 6.19.x
> 
> The Fixes commit was introduced in 6.1.  So the comment on Cc: stable@ line
> should be fixed.  Also, at the time of the commit, validation purpose running
> ctx committing didn't exist.  So, the issue you are explaining cannot happen on
> the commit.  Or, am I missing something?  If I'm not incorrect, could you
> please find the proper Fixes: commit and fix it?
> 
> Also, are you using LLM for Fixes...?  If so, the LLM seems not good at that.
> Your previous patch also made a similar mistake.  Please manually work on
> Fixes: tag or double check LLM's output.

Now I understand you added the comment for commit 60bd24f272d0.  I think Fixes:
should also be 60bd24f272d0.


Thanks,
SJ

[...]