From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [PATCH 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()
Date: Mon, 29 Jun 2026 21:34:50 -0700 [thread overview]
Message-ID: <20260630043451.151900-1-sj@kernel.org> (raw)
In-Reply-To: <20260630042540.7F8461F000E9@smtp.kernel.org>
On Tue, 30 Jun 2026 04:25:40 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] 32-bit integer overflow in `damon_nr_accesses_mvsum()` when calculating `left_window_bp` via `mult_frac()`.
> --
>
> commit 73c4d41e08fa85ce70e1cc04b1dee325a553e44e
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()
>
> This commit replaces the direct read of nr_accesses_bp with a call to
> damon_nr_accesses_mvsum() in __damos_valid_target(). This calculates the
> access score dynamically to be more tolerant to errors.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index df79f4d10bdc3..50939c3359bf7 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2126,10 +2126,11 @@ static noinline_for_stack void kdamond_tune_intervals(struct damon_ctx *c)
> > damon_set_attrs(c, &new_attrs);
> > }
> >
> > -static bool __damos_valid_target(struct damon_region *r, struct damos *s)
> > +static bool __damos_valid_target(struct damon_region *r, struct damos *s,
> > + struct damon_ctx *c)
> > {
> > unsigned long sz;
> > - unsigned int nr_accesses = r->nr_accesses_bp / 10000;
> > + unsigned int nr_accesses = damon_nr_accesses_mvsum(r, c);
>
> [Severity: Medium]
> Does wiring up damon_nr_accesses_mvsum() to the active DAMOS region filtering
> path expose a 32-bit integer overflow?
>
> Inside damon_nr_accesses_mvsum(), the calculation of left_window_bp uses
> the mult_frac() macro:
>
> left_window_bp = mult_frac(left_window, 10000, window_len);
>
> If a user or auto-tuning configures an aggr_interval / sample_interval ratio
> greater than ~429,496 (for example, an aggr_interval of 5,000,000 us and a
> sample_interval of 10 us), the intermediate multiplication inside mult_frac()
> can exceed the maximum 32-bit unsigned long value on 32-bit architectures:
>
> (left_window % window_len) * 10000
>
> This truncation would result in a severely deflated left_window_bp, leading
> to incorrect moving sum access scores.
>
> Could this cause DAMOS schemes to evaluate targets improperly, leading to
> sub-optimal memory reclaim or migration decisions?
Known low priority issue. Not a blocker for this patch.
Thanks,
SJ
[...]
next prev parent reply other threads:[~2026-06-30 4:34 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-30 4:07 [PATCH 00/18] mm/damon: optimize out nr_accesses_bp SJ Park
2026-06-30 4:07 ` [PATCH 01/18] mm/damon/core: introduce damon_nr_accesses_mvsum() SJ Park
2026-06-30 4:19 ` sashiko-bot
2026-06-30 4:28 ` SJ Park
2026-06-30 4:07 ` [PATCH 02/18] mm/damon/tests/core-kunit: test damon_mvsum() SJ Park
2026-06-30 4:07 ` [PATCH 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change SJ Park
2026-06-30 4:21 ` sashiko-bot
2026-06-30 4:30 ` SJ Park
2026-06-30 4:07 ` [PATCH 04/18] mm/damon/core: handle unreset nr_accesses in damon_nr_accesses_mvsum() SJ Park
2026-06-30 4:23 ` sashiko-bot
2026-06-30 4:33 ` SJ Park
2026-06-30 4:07 ` [PATCH 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target() SJ Park
2026-06-30 4:25 ` sashiko-bot
2026-06-30 4:34 ` SJ Park [this message]
2026-06-30 4:07 ` [PATCH 06/18] mm/damon/core: use damon_nr_accesses_mvsum() for damos region tracing SJ Park
2026-06-30 4:08 ` [PATCH 07/18] mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions SJ Park
2026-06-30 4:08 ` [PATCH 08/18] mm/damon/core: remove damon_warn_fix_nr_accesses_corruption() SJ Park
2026-06-30 4:08 ` [PATCH 09/18] mm/damon/core: remove damon_verify_reset_aggregated() SJ Park
2026-06-30 4:08 ` [PATCH 10/18] mm/damon/core: remove damon_verify_merge_regions_of() SJ Park
2026-06-30 4:08 ` [PATCH 11/18] mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests SJ Park
2026-06-30 4:08 ` [PATCH 12/18] selftests/damon/drgn_dump_damon_status: do not dump nr_accesses_bp SJ Park
2026-06-30 4:08 ` [PATCH 13/18] mm/damon/core: remove nr_accesses_bp setups and updates SJ Park
2026-06-30 4:08 ` [PATCH 14/18] mm/damon/core: remove attrs param from damon_update_region_access_rate() SJ Park
2026-06-30 4:19 ` sashiko-bot
2026-06-30 4:39 ` SJ Park
2026-06-30 4:48 ` SJ Park
2026-06-30 4:08 ` [PATCH 15/18] mm/damon/paddr: remove attrs param from __damon_pa_check_access() SJ Park
2026-06-30 4:08 ` [PATCH 16/18] mm/damon/vaddr: remove attrs param from __damon_va_check_access() SJ Park
2026-06-30 4:22 ` sashiko-bot
2026-06-30 4:45 ` SJ Park
2026-06-30 4:08 ` [PATCH 17/18] mm/damon/core: remove damon_moving_sum() and its unit test SJ Park
2026-06-30 4:08 ` [PATCH 18/18] mm/damon/core: remove damon_region->nr_accesses_bp SJ Park
2026-06-30 4:52 ` [PATCH 00/18] mm/damon: optimize out nr_accesses_bp SJ 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=20260630043451.151900-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