damon.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: SeongJae Park <sj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] mm/damon: export DAMON symbols and add sample loadable modules
Date: Tue,  5 Dec 2023 17:23:07 +0000	[thread overview]
Message-ID: <20231205172307.2310-1-sj@kernel.org> (raw)
In-Reply-To: <ZW6xTyyl0qvcqi3O@infradead.org>

Hi Christoph,

On 2023-12-04T21:12:47-08:00 Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Dec 05, 2023 at 02:28:55AM +0000, SeongJae Park wrote:
> > DAMON cannot be used from loadable modules since it is not exporting its
> > symbols.
> 
> And that is a good thing.  We should absolutely not allow random modules
> probing MM internals.

I agree.

> What you posted here is basically just adding hooks without even real
> in-kernel users.

I was thinking someone might be able to think even the sample module as real
usage since there was actually some questions about it from real users.  That
said, those were more like questions than strong requests, so I still think
this change is somewhat better to be made for at least some folks, but I also
agree that this wouldn't be not really essential for everyone, and could be
only long term maintenance threat.

I personally don't have strong opinion, and thank you for raising your concern.
I will hold this patchset unless someone request this change again with good
rationale.

Btw, I know there were many concerns about unnecessarily exporting symbols, but
do we have a formal guideline or documentation about the requirements for
exporting symbols in sepcific subsystem?  I was hoping to have such one so that
I could provide better answer to DAMON's loadable module support questions, but
I was unable to find such one with my poor searching skill.  This reply could
be used for the purpose meanwhile, though, so appreciate again. :)


Thanks,
SJ

      reply	other threads:[~2023-12-05 17:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-05  2:28 [PATCH 0/3] mm/damon: export DAMON symbols and add sample loadable modules SeongJae Park
2023-12-05  2:28 ` [PATCH 1/3] mm/damon/core: export symbols for supporting " SeongJae Park
2023-12-05  2:28 ` [PATCH 2/3] samples: add working set size estimation DAMON sample module SeongJae Park
2023-12-05  2:28 ` [PATCH 3/3] samples: add proactive reclamation " SeongJae Park
2023-12-05  5:12 ` [PATCH 0/3] mm/damon: export DAMON symbols and add sample loadable modules Christoph Hellwig
2023-12-05 17:23   ` SeongJae Park [this message]

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=20231205172307.2310-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).