All of lore.kernel.org
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Akinobu Mita <akinobu.mita@gmail.com>
Cc: SeongJae Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [RFC PATCH 0/4] mm/damon: introduce perf event based access check
Date: Tue, 17 Feb 2026 07:15:44 -0800	[thread overview]
Message-ID: <20260217151545.77290-1-sj@kernel.org> (raw)
In-Reply-To: <CAC5umygPq8+FQWTG73-QPOKHT1P5=N2+qFkrRfZAkL_7G=gQXQ@mail.gmail.com>

On Tue, 17 Feb 2026 22:32:43 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:

> 2026年2月17日(火) 9:13 SeongJae Park <sj@kernel.org>:
> >
> > On Tue, 27 Jan 2026 17:12:23 -0800 SeongJae Park <sj@kernel.org> wrote:
> >
> > > On Tue, 27 Jan 2026 21:56:43 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> > >
> > > > 2026年1月27日(火) 15:43 SeongJae Park <sj@kernel.org>:
> > > > > Actually DAMON is internally setting such maximum region size based on the
> > > > > min_nr_regions parameter, via damon_region_sz_limit().  Nonetheless, the limit
> > > > > is applied only in regions merge time.  That's why it requires the regions
> > > > > split to happen sufficiently until the real fixed granularity monitoring is
> > > > > started.
> > > > >
> > > > > And I think this behavior is just a bug, or suboptimum implementation at least.
> > > > > That is, users set the minimum number of regions but it may not really be kept.
> > > > > That's definitely confusing behavior.  Actually there was a similar case that
> > > > > number of regions can be larger than the max_nr_regions.  We fixed it, with
> > > > > commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when
> > > > > max_nr_regions is unmet").  I think we discussed about similar case for
> > > > > min_nr_regions, but I cannot find the discussion for now.
> > > > >
> > > > > So, I think it is better to fix this rather than introducing a new parameter.
> > > >
> > > > I agree with that.
> > > >
> > > > > Maybe we can split regions based on the min_nr_regions based size limit, before
> > > > > starting the main loop of kdamond_fn().  Similar to the max_nr_regions
> > > > > violation, there could be yet another corner case on online parameters commit
> > > > > situation, so it would better to check the case, too.  You could implement such
> > > > > fix on your own, or let me do that.  In the latter case, if you don't mind, I
> > > > > will add your Reported-by: tag to the fix.  Please let me know your
> > > > > preferrence.
> > > >
> > > > You'll be better able to fix it, so please fix it at your convenience.
> > > > Adding Reported-by: tag is fine.
> > >
> > > Sounds good, I will do so!
> >
> > I just posted an RFC patch series [1] for this.  I will drop RFC tag after the
> > current merge window is finished.  Please let me know if you find something
> > wrong there!
> >
> > [1] https://lore.kernel.org/20260217000400.69056-1-sj@kernel.org
> 
> Thank you for posting the patch series.
> 
> I tried it and found that patch 2/3 made things worse than before in terms of
> monitoring at page size granularity.

Thank you for sharing the test results!  

> This is because, in my evaluation, new
> target processes are added without stopping kdamond, so processes added later
> are not initially monitored at page size granularity.

Nice catch.  But, the min_nr_regions based region split of vaddr was triggered
by damon_va_init(), which is called before the kdamond_fn()'s main loop.  How
the regions were split before the patch?  Probably I'm missing something.
Could you please clarify?

> 
> Would performing a split operation like damon_apply_min_nr_regions() within
> damon_set_regions() solve the problem?

Yes, that's the plan.  The split operation should be done for online updates of
total size of monitoring regions and min_nr_regions.  I'm preparing a followup
patch series for that.  While working on it, however, I found it requires some
refactoring and cleanup that taking time longer than I expected.  Meanwhile I
was thinking your issue is only at the beginning of kdamond, and I didn't want
to make you wait too long.  Hence posted the series first.


Thanks,
SJ

  reply	other threads:[~2026-02-17 15:15 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23  2:10 [RFC PATCH 0/4] mm/damon: introduce perf event based access check Akinobu Mita
2026-01-23  2:10 ` [RFC PATCH 1/4] mm/damon/core: add common code for " Akinobu Mita
2026-01-23  2:10 ` [RFC PATCH 2/4] mm/damon/vaddr: support " Akinobu Mita
2026-01-23  2:10 ` [RFC PATCH 3/4] mm/damon/paddr: " Akinobu Mita
2026-01-23  2:10 ` [RFC PATCH 4/4] mm/damon: allow user to set min and max size of region Akinobu Mita
2026-01-24  2:39 ` [RFC PATCH 0/4] mm/damon: introduce perf event based access check SeongJae Park
2026-01-24  2:48   ` SeongJae Park
2026-02-23  8:08     ` Namhyung Kim
2026-02-25  6:48       ` Akinobu Mita
2026-02-26  1:24         ` Namhyung Kim
2026-01-27  1:29   ` Akinobu Mita
2026-01-27  6:43     ` SeongJae Park
2026-01-27 12:56       ` Akinobu Mita
2026-01-28  1:12         ` SeongJae Park
2026-02-17  0:13           ` SeongJae Park
2026-02-17 13:32             ` Akinobu Mita
2026-02-17 15:15               ` SeongJae Park [this message]
2026-02-18  8:20                 ` Akinobu Mita
2026-02-18 15:40                   ` SeongJae Park
2026-02-19  6:28                     ` Akinobu Mita
2026-02-19  6:49                       ` SeongJae Park
2026-03-03  1:05                         ` SeongJae 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=20260217151545.77290-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akinobu.mita@gmail.com \
    --cc=damon@lists.linux.dev \
    /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.