linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: JaeJoon Jung <rgbi3307@gmail.com>
To: SeongJae Park <sj@kernel.org>
Cc: Enze Li <lienze@kylinos.cn>,
	akpm@linux-foundation.org, damon@lists.linux.dev,
	 linux-mm@kvack.org, enze.li@gmx.com
Subject: Re: [PATCH 0/2] mm/damon: export symbols and introduce prdm module
Date: Tue, 23 Dec 2025 05:57:55 +0900	[thread overview]
Message-ID: <CAHOvCC6wQ5h5j42iTaepRnSkA6mqeY-ExpHA4Qc6K+-xb6T0eQ@mail.gmail.com> (raw)
In-Reply-To: <20251221192751.42586-1-sj@kernel.org>

On Mon, 22 Dec 2025 at 04:27, SeongJae Park <sj@kernel.org> wrote:
>
> On Sun, 21 Dec 2025 20:04:23 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
>
> > On Sun, 21 Dec 2025 at 17:12, SeongJae Park <sj@kernel.org> wrote:
> > >
> > > On Sun, 21 Dec 2025 11:42:43 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
> > >
> > > > On Fri, 19 Dec 2025 at 21:52, SeongJae Park <sj@kernel.org> wrote:
> > > > >
> > > > > On Thu, 18 Dec 2025 21:46:37 +0800 Enze Li <lienze@kylinos.cn> wrote:
> > > > >
> > > > > > On 2025/12/16 07:16, SeongJae Park wrote:
> > > > > > > Hello Enze,
> > > > > > >
> > > > > > > On Mon, 15 Dec 2025 22:20:55 +0800 Enze Li <lienze@kylinos.cn> wrote:
> > > [...]
> > > > > If runtime controls are essential, I think using DAMON sysfs
> > > > > interface or DAMON user-space tool is a better choice.
> > > >
> > > > I also think the current DAMON design structure is suitable for DAMON sysfs
> > > > and user-space tools.  DAMON sysfs can run multiple nr_ctxs, so I think it
> > > > would be a good idea to consolidate it into this.  As Enze mentioned, DAMON
> > > > was originally designed as a kernel loadable module,
> > >
> > > I don't recall where Enze mentioned so.  Could you please give me a pointer?
> >
> > Although Enze did not directly state this, His patch is a loadable module,
> > which implies that it was "designed as a kernel loadable module."
>
> Thanks for clarifying.  I understand you are saying Enze's prdm module is
> designed as a loadable module, while you are not saying about other parts of
> DAMON.
>
> >
> > >
> > > And I'm not sure based on what you are saying it was originally designed as a
> > > loadable module.  Maybe you are saying something before DAMON's mainline
> > > landing, but that's quite long ago and my memory management is not good.  Could
> > > you please add more contexts?
> > >
> > > > but I was curious as to
> > > > why it wasn't run that way.
> > >
> > > Maybe I could give you an answer if you could give me the context that I asked
> > > above.
> > >
> > > > When executing the damon_start(**ctxs,
> > > > nr_ctxs, exclusive)
> > > > function, nr_ctxs=1, exclusive=true is passed so that modules are currently
> > > > executed only one at a time.  Therefore, there is no point in running
> > > > DAMON
> > > > as a kernel module, since you are not running multiple modules simultaneously.
> > >
> > > I'm not following your point well.  Could you please elaborate your point?
> >
> > So far in my view, I have analyzed DAMON as follows:
> >
> > DAMON core  <---------> kernel module   <------> damo(user-space)
> > mm/damon/core.c         mm/damon/lru_sort.c
> > mm/damon/vaddr.c        mm/damon/reclaim.c
> > mm/damon/paddr.c        samples/damon/*.c
> > mm/damon/sysfs.c
> >
> > Above, lru_sort and reclaim are DAMON core characteristics, but they have
> > a kernel module structure because they are executed with module_init().
> > I personally think this is inappropriate.
>
> Could you please further clarify why you think it is inappropriate?
>
> > Additionally, currently, DAMON is
> > executed only once at a time according to the exclusive=true condition
> > when damon_start() is called.  In this structure, I believe that the meaning
> > of "kernel loadable module"  is non-existent.
>
> I have to say I still don't get your points.  More elaboration would be nice.

To summarize the Damon I have analyzed so far,

DAMON core  <---------------> DAMON kernel module
| mm/damon/core.c               + mm/damon/lru_sort.c (schemes setting)
| mm/damon/vaddr.c              + mm/damon/reclaim.c (quota, filter)
| mm/damon/paddr.c              + samples/damon/*.c (using samples)
| mm/damon/sysfs.c
            |
        /sys/kernel/mm/damon/admin
            |
            +--------------------------> open/read(get)/write(set)/close
                                               (user-space system call)

The DAMON kernel module above is a sample that sets some parameters of
DAMON.  I think it's better to use damon/sysfs since all of this can be
done in damon/sysfs.  I am continuing to analyze this.  If any
improvements are found, I'll send you a patch.

Thanks,
JaeJoon

>
> >
> > >
> > > > I'm doing some more research into why we run modules with exclusive=true.
> > >
> > > Hoplefully commit 8b9b0d335a34 ("mm/damon/core: allow non-exclusive DAMON
> > > start/stop") will give you more contexts.
> > >
> > > To summarize again, the intention is to avoid kdamonds interrupting each
> > > other's access checks.  For example, suppose the DAMON context for
> > > DAMON_RECLAIM and another one (could be created by DAMON sysfs interface users
> > > or other DAMON modules, say, DAMON_STAT) are running concurrently.  And if
> > > those pick a same page as access check sampling target, their finding of the
> > > access on the page depend on a race condition.  To avoid such a case, DAMON
> > > modules call damon_start() with exclusive argument set.
> > >
> > > Maybe this is unclearly documented.  I will recheck the documentation and
> > > improve it to make this more clear.  If you find the rooms to improve and have
> > > ideas for the improvements, please feel free to send patches.
> > >
> > > > One more thing I would like to add is that I think it would be less
> > > > confusing
> > > > to move lru_sort and reclaim to the samples/damon/ path.
> > >
> > > No, I don't think so.  Those are for real world products, not for samples.
> > >
> > > > And it would
> > > > be
> > > > a good idea to organize the core sources toward
> > > > mm/damon/modules-common.c source.
> > >
> > > I don't find exactly what kind of reorganization you mean.  But I agree there
> > > could be rooms to improve in terms of the files and code organization.  Any
> > > suggestion is welcome.
> > >
> > > [...]
> > > > > > Finally, I would like to add that designing prdm with an extremely
> > > > > > simple workflow -- load the module, set the target PIDs, and enable the
> > > > > > switch -- will contribute to the promotion of DAMON, allowing a broader
> > > > > > user base to benefit from the DAMON system.
> > > > >
> > > > > That workflow is definitely much easier than directly using DAMON sysfs
> > > > > interface.  But, for the reason we have DAMON user-space tool.  Using it, the
> > > > > workflow can also be as simple as prdm, like below.
> > > > >
> > > > >    # damo start $TARGET_PID --damos_action pageout --damos_access_rate 0% 0% --damos_age 2m max
> > > >
> > > > Where can I find the damo source or documentation?
> > >
> > > You could use https://github.com/damonitor/damo and its README.md file.
> >
> > Thank you so much for sharing.  I am trying to analyze the related contents
> > in more detail to improve your DAMON.  I'll try to organize it a bit better
> > and send you a patch.
>
> Looking forward.
>
>
> Thanks,
> SJ
>
> [...]


  reply	other threads:[~2025-12-22 20:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-15 14:20 Enze Li
2025-12-15 14:20 ` [PATCH 1/2] mm/damon/core: export necessary symbols Enze Li
2025-12-15 14:20 ` [PATCH 2/2] mm/damon/modules: introduce prdm module for DAMON Enze Li
2025-12-15 23:16 ` [PATCH 0/2] mm/damon: export symbols and introduce prdm module SeongJae Park
2025-12-18 13:46   ` Enze Li
2025-12-19 11:46     ` SeongJae Park
2025-12-21  2:42       ` JaeJoon Jung
2025-12-21  8:12         ` SeongJae Park
2025-12-21 11:04           ` JaeJoon Jung
2025-12-21 19:27             ` SeongJae Park
2025-12-22 20:57               ` JaeJoon Jung [this message]
2025-12-22 21:33                 ` SeongJae Park
2025-12-22  7:08       ` Enze Li
2025-12-22 15:21         ` 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=CAHOvCC6wQ5h5j42iTaepRnSkA6mqeY-ExpHA4Qc6K+-xb6T0eQ@mail.gmail.com \
    --to=rgbi3307@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=enze.li@gmx.com \
    --cc=lienze@kylinos.cn \
    --cc=linux-mm@kvack.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox