Linux kernel -stable discussions
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, sashiko-bot@kernel.org
Cc: SeongJae Park <sj@kernel.org>,
	stable@vger.kernel.org, damon@lists.linux.dev
Subject: Re: [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz
Date: Wed,  6 May 2026 09:49:58 -0700	[thread overview]
Message-ID: <20260506164959.10212-1-sj@kernel.org> (raw)
In-Reply-To: <20260504170607.529EBC2BCB8@smtp.kernel.org>

TL; DR: This patch is broken, please drop.  I will send v2 soon, as a reply to
this.  Please read below for details.

On Mon, 04 May 2026 17:06:06 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `ctx` parameter in `damos_set_effective_quota()` is unused, and the function incorrectly uses the `DAMON_MIN_REGION` macro instead of `ctx->min_sz_region`.
> --
> 
> commit 8afd735e4c663dcbcca090e71f37ad10c37c705a
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: disallow time-quota setting zero esz
> 
> When DAMOS scheme throughput is very slow, the time quota can make the
> effective size quota smaller than the context's minimum region size, causing
> actions to be skipped. This commit attempts to fix the problem by ensuring
> the effective size quota is at least the minimum region size.
> 
> Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
> Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2124,7 +2124,8 @@ static unsigned long damos_quota_score(struct damos_quota *quota)
> >  /*
> >   * Called only if quota->ms, or quota->sz are set, or quota->goals is not empty
> >   */
> > -static void damos_set_effective_quota(struct damos_quota *quota)
> > +static void damos_set_effective_quota(struct damos_quota *quota,
> > +		struct damon_ctx *ctx)
> >  {
> >  	unsigned long throughput;
> >  	unsigned long esz = ULONG_MAX;
> 
> [ ... ]
> 
> > @@ -2150,6 +2151,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
> >  		else
> >  			throughput = PAGE_SIZE * 1024;
> >  		esz = min(throughput * quota->ms, esz);
> > +		esz = max(DAMON_MIN_REGION, esz);
> 
> Should this be using ctx->min_sz_region instead of DAMON_MIN_REGION?
> 
> The commit message mentions bounding the effective size quota to
> damon_ctx->min_region_sz, and the ctx parameter was successfully added
> to the function signature. 
> 
> However, the ctx parameter is left unused here and the macro is used as a
> hardcoded bound instead. Since ctx->min_sz_region can be dynamically
> configured, can the effective size quota still fall below it with this
> implementation?

Correct.  Cherry-picking the original commit was causing build issue since the
upstream commit is renamed field (min_region_sz).  When fixing the build issue
I mistakenly assumed the field itslef is not yet introduced to 6.18 and using
DAMON_MIN_REGION.  It should use the field that not yet renamed
(min_sz_region), as Sashiko pointed out.

Greg, please drop this.  I will send a v2 soon, as a reply to this mail.

> 
> >  	}
> >  
> >  	if (quota->sz && quota->sz < esz)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260504130127.25364-1-sj@kernel.org?part=1
> 


Thanks,
SJ

       reply	other threads:[~2026-05-06 16:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260504170607.529EBC2BCB8@smtp.kernel.org>
2026-05-06 16:49 ` SeongJae Park [this message]
2026-05-06 16:52   ` [PATCH 6.18.y v2] mm/damon/core: disallow time-quota setting zero esz SeongJae Park
2026-05-03 12:41 FAILED: patch "[PATCH] mm/damon/core: disallow time-quota setting zero esz" failed to apply to 6.18-stable tree gregkh
2026-05-04 13:01 ` [PATCH 6.18.y] mm/damon/core: disallow time-quota setting zero esz SeongJae Park

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=20260506164959.10212-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox