All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [PATCH 6.6.y] mm/damon/core: don't skip damos_adjust_quota() while esz is not zero
Date: Sat,  3 Oct 2026 11:02:38 -0700	[thread overview]
Message-ID: <20261003180240.34878-1-sj@kernel.org> (raw)
In-Reply-To: <20261003175616.B26E01F0089B@smtp.kernel.org>

On Sat, 03 Oct 2026 17:56:16 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] mm: commit message references nonexistent damos_quota_is_set()
> - [Low] mm: outdated comment for damos_set_effective_quota()
> --
> 
> commit bdd4b48218af232c248d2db96f0af460e859f59e
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: don't skip damos_adjust_quota() while esz is not zero
> 
> DAMOS could unexpectedly stop working when a user disables quota using the
> online parameters commit feature. Fix it by correcting a wrong quota
> unset check in damos_adjust_quota().
> 
> Link: https://lore.kernel.org/20260916135020.86483-1-sj@kernel.org
> 
> > Other logic for catching that uses damos_quota_is_set(), which understands
> > the fact and therefore checks the effective size quota in addition to the
> > user setup.  Fix the issue by using damos_quota_is_set() in
> > damos_adjust_quota() to determine if its works should be skipped.
> 
> [Severity: Medium]
> The commit message states the fix uses damos_quota_is_set(). However, the
> code below checks the fields directly inline. Should the commit message
> be updated to reflect the inline check?

No, since this is a backporting of the mainline commit.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index fe91b296f4c66..bd46a63b611cf 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1087,7 +1087,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
> >  	unsigned long cumulated_sz;
> >  	unsigned int score, max_score = 0;
> >  
> > -	if (!quota->ms && !quota->sz)
> > +	if (!quota->esz && !quota->ms && !quota->sz)
> >  		return;
> 
> [Severity: Low]
> With this change, damos_adjust_quota() can proceed even when quota->ms and
> quota->sz are zero. This allows damos_set_effective_quota() to be called
> later in the function during a new charge window:
> 
> 	if (!time_in_range_open(...)) {
> 		...
> 		damos_set_effective_quota(quota);
> 	}
> 
> Does this make the comment above damos_set_effective_quota() outdated? It
> currently states:
> 
> /* Shouldn't be called if quota->ms and quota->sz are zero */
> static void damos_set_effective_quota(struct damos_quota *quota)

That's out of the scope of this backport in my opinion.

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


Thanks,
SJ

  reply	other threads:[~2026-10-03 18:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <2026100318-preteen-hazelnut-d38b@gregkh>
2026-10-03 15:48 ` [PATCH 6.6.y] mm/damon/core: don't skip damos_adjust_quota() while esz is not zero SJ Park
2026-10-03 17:56   ` sashiko-bot
2026-10-03 18:02     ` SJ Park [this message]
2026-10-04 16:29   ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261003180240.34878-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-bot@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.