Re: [RFC PATCH 4/9] mm/damon/core: use damos_quota_is_set() in damos_adjust_quota()

[KunWu Chan]

On Mon, Sep 14, 2026 at 11:16 PM SJ Park <sj@xxxxxxxxxx> wrote:
>
> On Mon, 14 Sep 2026 23:01:48 +0800 Kunwu Chan <kunwu.chan@xxxxxxxxx> wrote:
>
> > On Sat, 12 Sep 2026 13:08:05 -0700 SJ Park <sj@xxxxxxxxxx> wrote:
> >
> > > damos_adjust_quota() is manually checking if the user set the DAMOS
> > > quota.  There is a dedicated helper, damos_quota_is_set(), for the
> > > purpose.  Use the helper.
> > >
> > > Signed-off-by: SJ Park <sj@xxxxxxxxxx>
> > > ---
> > >  mm/damon/core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index cf10f6d725c19..dd27068cb83e3 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -3344,7 +3344,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
> > >     unsigned long cumulated_sz, cached_esz;
> > >     unsigned int score, max_score = 0;
> > >
> > > -   if (!quota->ms && !quota->sz && list_empty(&quota->goals))
> > > +   if (!damos_quota_is_set(quota))
> >
> > Hi SJ,
> > This is more than a cleanup, because damos_quota_is_set() also checks
> > quota->esz, which the original condition did not.
> >
> > The path that makes this matter: damos_commit_quota() copies ms and sz
> > from sysfs but not esz.  So if a scheme runs with a non-zero ms, then
> > the user sets ms=0 and commits, the running scheme gets ms=0 while esz
> > keeps its old value.
> >
> > The old check would then return early, leaving esz stale.  That stale
> > esz is visible to damos_valid_target() and damos_quota_is_full(),
> > which also use damos_quota_is_set().
> >
> > With the helper, damos_adjust_quota() proceeds into
> > damos_set_effective_quota(), where the first branch resets esz from sz
> > when ms and goals are unset.
> >
> > The code change is correct, but worth noting the behavioral change in
> > the commit message.
>
> So, if user sets only time quota, and then commits the quota to zero while the
> esz is non-zero, esz is never reset to zero.  As a result, DAMOS keeps applying
> the action with the esz forever.  Correct?

Yes, the stale esz path is confirmed.

One clarification, though: it does not keep applying the action
forever. While charged_sz < stale_esz, damos_adjust_quota() still
returns early, but damos_apply_target() continues to apply the
scheme and damos_charge_quota() keeps increasing charged_sz.

Once charged_sz >= stale_esz, damos_quota_is_full() returns true,
so damon_do_apply_schemes() skips the scheme. Since the early return
also prevents the charge-window handling from running,
charged_sz is never reset, and the scheme remains permanently
inactive.

So clearing the quota can result in the scheme applying up to the
stale esz, and then stopping permanently.

>
> If so, this may deserve Cc-ing stable@.  Kunwu, what do you think?

I think this is a functional bug in the existing code, and Cc'ing
stable makes sense.

Thanks,
Kunwu

>
>
> Thanks,
> SJ
>
> [...]