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: [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
Date: Sat, 10 Oct 2026 00:11:22 -0700	[thread overview]
Message-ID: <20261010071123.8894-1-sj@kernel.org> (raw)
In-Reply-To: <sashiko-outbox-166283@kernel.org>

On Sat, 10 Oct 2026 06:41:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: double-application of earlier schemes to newly split regions
> --
> 
> commit 01d768c4d318527360ef5913771d20f22a99984a
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
> 
> kdamond_apply_schemes() is using damon_for_each_region_safe(), which is
> safe for deallocation of the region inside the loop. However, the loop
> internal logic does not deallocate regions. Hence it is only wasting the
> next pointer.
> 
> 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 7afbc20833ea4..b5c85e6488596 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -912,7 +911,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  	}
> >  
> >  	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 code cause earlier schemes to be applied twice to newly split
> regions?
> 
> If multiple schemes are configured, an earlier scheme (e.g. Scheme 0) might
> apply its action to a full, un-split region. If a later scheme (e.g.
> Scheme 1) then decides to split the region, damon_split_region_at() inserts
> the newly split part into the list immediately after the current region.
> 
> Because this loop in kdamond_apply_schemes() now uses
> damon_for_each_region() instead of the safe variant, the outer loop will
> advance to this newly split region and call damon_do_apply_schemes() on it
> again.
> 
> Will this cause damon_do_apply_schemes() to evaluate the new region from
> the beginning of the scheme list, resulting in Scheme 0 executing its
> memory management actions (like madvise) and counting statistics a second
> time on the same memory range?

The same issue exists in the mainline.  The user impact is minor.  No blocker
of this patch in my opinion.  I will separately work on it.

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


Thanks,
SJ

  reply	other threads:[~2026-10-10  7:11 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <2026092949-surpass-thimble-02a8@gregkh>
2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
2026-09-30 10:05   ` sashiko-bot
2026-09-30 10:23     ` SJ Park
2026-10-02 14:19   ` Sasha Levin
2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
2026-10-09 14:48   ` [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
2026-10-09 14:56     ` sashiko-bot
2026-10-09 15:09       ` SJ Park
2026-10-09 14:48   ` [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
2026-10-09 15:04     ` sashiko-bot
2026-10-09 15:11       ` SJ Park
2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
2026-10-10  6:30   ` [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
2026-10-10  6:41     ` sashiko-bot
2026-10-10  7:11       ` SJ Park [this message]
2026-10-10  6:30   ` [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
2026-10-10  6:46     ` sashiko-bot
2026-10-10  7:12       ` SJ Park
2026-10-11 16:48   ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() 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=20261010071123.8894-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.