From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [PATCH 6.18.y v2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
Date: Sun, 04 Oct 2026 15:28:56 +0000 [thread overview]
Message-ID: <20261004152857.6454F1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20261004151730.4700-1-sj@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: double application of schemes due to non-safe region walk
--
commit 5f2353c7bc7cba2be5c7364e80d79941a4bb1709
Author: SeongJae Park <sj@kernel.org>
Date: Sun Oct 4 15:17:30 2026 +0000
mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
This commit replaces damon_for_each_region_safe() with damon_for_each_region()
in kdamond_apply_schemes() so that regions dynamically split by scheme address
filters are not skipped. It also removes the workaround in
damos_skip_charged_region() that adjusted the region pointer, as the new
iteration logic will naturally visit the newly split regions.
Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c0dcb190b06e3..02cc99aa6d3eb 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1770,9 +1772,7 @@ static bool damos_skip_charged_region(struct damon_target *t,
> sz_to_skip = min_sz_region;
> }
> damon_split_region_at(t, r, sz_to_skip);
> - r = damon_next_region(r);
> - *rp = r;
> - skip = false;
> + skip = true;
[Severity: High]
When damon_split_region_at() splits this region, the remainder is inserted
directly after 'r'. Will this cause earlier schemes to process the remainder
a second time?
Because kdamond_apply_schemes() now uses damon_for_each_region(), the loop
will step into the newly split remainder region next. However,
damon_do_apply_schemes() iterates all schemes from the beginning for each
region. If Scheme 0 already processed the full region before Scheme 1 split
it here, Scheme 0 will now evaluate and charge the remainder portion a
second time.
[ ... ]
> @@ -2329,7 +2329,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>
> mutex_lock(&c->walk_control_lock);
> damon_for_each_target(t, c) {
> - damon_for_each_region_safe(r, next_r, t)
> + damon_for_each_region(r, t)
> damon_do_apply_schemes(c, t, r);
[Severity: High]
Does this non-safe iteration lead to double-charging quotas across schemes?
If a later scheme in damon_do_apply_schemes() splits the region (either due
to skip boundaries in damos_skip_charged_region() or quota limits in
damos_apply_scheme()), the new right-hand region is immediately visited on
the next iteration of this outer loop.
Since damon_do_apply_schemes() always begins its scheme evaluation from the
first scheme, any schemes evaluated before the split will process the newly
split right-hand region again, incorrectly double-applying physical actions
and double-charging their quotas.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261004151730.4700-1-sj@kernel.org?part=1
next prev parent reply other threads:[~2026-10-04 15:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-04 15:17 [PATCH 6.18.y v2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
2026-10-04 15:28 ` sashiko-bot [this message]
2026-10-05 13:50 ` 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=20261004152857.6454F1F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=damon@lists.linux.dev \
--cc=sashiko-reviews@lists.linux.dev \
--cc=sj@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.