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: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
Date: Sat,  1 Aug 2026 13:29:38 -0700	[thread overview]
Message-ID: <20260801202939.117292-1-sj@kernel.org> (raw)
In-Reply-To: <20260801193949.4270A1F00AC4@smtp.kernel.org>

On Sat, 01 Aug 2026 19:39:48 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] The commit message incorrectly identifies the function being modified, failing to align its claims with the code changes.
> 
> Pre-existing issues:
> - [High] Multiplication overflow in `mult_frac()` and other basis point scaling calculations on 32-bit architectures leads to incorrect memory ratio calculations and scaling.
> --
> 
> --- Patch [9]: [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() ---
> commit fee9e9f299e48399ca370c1f9725936302eaeb8c
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core: handle extreme memory state in get_in_active_mem_bp()
> 
> This patch prevents a divide-by-zero risk in DAMON's active memory ratio
> calculations. In extreme environments where active and inactive memory
> both amount to zero, the patch clamps the total memory denominator to a
> minimum of 1 to avoid a crash.
> 
> Link: https://lore.kernel.org/20260721034756.147011-1-sj@kernel.org
> 
> > damon_get_intervals_adaptation_bp() uses the sum of the active and
> > inactive memory amount as a denominator.  In an extreme and unlikely
> > environment, active and inactive memory might be zero.
> 
> [Severity: Medium]
> Is the function name referenced in the commit message correct?
> 
> The commit message explicitly states that damon_get_intervals_adaptation_bp()
> is being modified to fix a divide-by-zero bug, but the patch actually
> modifies damos_get_in_active_mem_bp().

Good catch, I will fix.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 76764a2056f7c..4944cf2c5afae 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3010,7 +3010,7 @@ static unsigned int damos_get_in_active_mem_bp(bool active_ratio)
> >  		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
> >  	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
> >  		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
> > -	total = active + inactive;
> > +	total = max(active + inactive, 1);
> >  	if (active_ratio)
> >  		return mult_frac(active, 10000, total);
> 
> [Severity: High]
> This is a pre-existing issue, but does this calculation overflow on 32-bit
> architectures?
> 
> In damos_get_in_active_mem_bp(), if active memory exceeds approximately
> 429,496 pages (~1.75 GB), multiplying the active page count by 10000 inside
> mult_frac() will overflow an unsigned long on 32-bit systems before the
> division by total. This could result in artificially low active/inactive
> memory ratios and break DAMON's adaptive tuning.
> 
> Does this same overflow affect damon_feed_loop_next_input()?
> 
> That function returns last_input * score_cg / goal. When last_input is
> passed as 100,000,000 and score_cg can be up to 10,000, that multiplication
> would similarly wrap around 32-bit boundaries.

Known low priority issue.

> 
> >  	return mult_frac(inactive, 10000, total);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=9


Thanks,
SJ

      reply	other threads:[~2026-08-01 20:29 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-01 17:53   ` sashiko-bot
2026-08-01 20:01     ` SJ Park
2026-08-01 20:31       ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:21   ` sashiko-bot
2026-08-01 20:07     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:50   ` sashiko-bot
2026-08-01 20:12     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
2026-08-01 19:11   ` sashiko-bot
2026-08-01 20:18     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-01 19:31   ` sashiko-bot
2026-08-01 20:26     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
2026-08-01 19:39   ` sashiko-bot
2026-08-01 20:29     ` SJ Park [this message]

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=20260801202939.117292-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.