DAMON development mailing list
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox