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>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"David Gow" <davidgow@davidgow.net>,
	"Quanmin Yan" <yanquanmin1@huawei.com>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	"Lian Wang" <lianux.mm@gmail.com>,
	"Kunwu Chan" <kunwu.chan@gmail.com>
Subject: Re: [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
Date: Sun,  8 Nov 2026 17:34:08 -0800	[thread overview]
Message-ID: <20261109013410.93127-1-sj@kernel.org> (raw)
In-Reply-To: <sashiko-outbox-204294@kernel.org>

On Mon, 09 Nov 2026 01:28:24 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm/damon: integer overflow in damos_quota_score() on first
>   PSI round
> --
> 
> commit 5e9b1f636c184ecf41e05d717ad5e0116b0fca20
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total
> 
> This commit initializes damos_quota_goal->last_psi_total to U64_MAX when
> creating or committing a DAMOS_QUOTA_SOME_MEM_PSI_US goal, and sets
> goal->current_value to goal->target_value on the first tuning round so the
> effective quota remains unchanged until a baseline PSI reading is recorded.
> 
> Link: https://lore.kernel.org/20260902002725.108635-3-sj@kernel.org
> Link: https://lore.kernel.org/20260718005316.89585-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3142776e2e0ee..420ef0adaa7e3 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1616,7 +1619,12 @@ static void damos_set_quota_goal_current_value(struct damos_quota_goal *goal)
> >  		break;
> >  	case DAMOS_QUOTA_SOME_MEM_PSI_US:
> >  		now_psi_total = damos_get_some_mem_psi_total();
> > -		goal->current_value = now_psi_total - goal->last_psi_total;
> > +		/* uninitialized last_psi_total; make no effect this round */
> > +		if (goal->last_psi_total == U64_MAX)
> > +			goal->current_value = goal->target_value;
> > +		else
> > +			goal->current_value = now_psi_total -
> > +				goal->last_psi_total;
> >  		goal->last_psi_total = now_psi_total;
> >  		break;
> >  	default:
> 
> [Severity: Medium]
> Can setting goal->current_value = goal->target_value here overflow in
> damos_quota_score() when goal->target_value exceeds ULONG_MAX / 10000?
> 
> Unlike mainline where damos_quota_score() uses mult_frac(), this branch
> computes the score with direct unsigned long multiplication:
> 
> mm/damon/core.c:damos_quota_score() {
> 	damos_for_each_quota_goal(goal, quota) {
> 		damos_set_quota_goal_current_value(goal);
> 		highest_score = max(highest_score,
> 				goal->current_value * 10000 /
> 				goal->target_value);
> 	}
> }
> 
> When goal->target_value > ULONG_MAX / 10000 (for example, above 429,496 us
> on 32-bit architectures), goal->current_value * 10000 wraps around and
> produces a score below 10000, causing damon_feed_loop_next_input() to
> increase quota->esz_bp on the first round instead of keeping the current
> quota unchanged.

The issue impacts only DAMOS quota auto-tuning quality under certain setups.
No blocker for this patch.

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


Thanks,
SJ


  reply	other threads:[~2026-11-09  1:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  1:13 [PATCH 6.12.y v2] mm/damon/core: handle uninitialized damos_quota_goal->last_psi_total SJ Park
2026-11-09  1:28 ` sashiko-bot
2026-11-09  1:34   ` SJ Park [this message]
2026-11-09 15:52 ` 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=20261109013410.93127-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=brendan.higgins@linux.dev \
    --cc=damon@lists.linux.dev \
    --cc=davidgow@davidgow.net \
    --cc=kunwu.chan@gmail.com \
    --cc=lianux.mm@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=sashiko-bot@kernel.org \
    --cc=yanquanmin1@huawei.com \
    /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.