mbox series

[0/3] mm/damon: export DAMON symbols and add sample loadable modules

Message ID 20231205022858.1540-1-sj@kernel.org
Headers
Series mm/damon: export DAMON symbols and add sample loadable modules |

Message

SeongJae Park Dec. 5, 2023, 2:28 a.m. UTC 
  Changes from RFC
(https://lore.kernel.org/damon/20231121053604.60798-1-sj@kernel.org/)
- Wordsmith commit message
- Use GPL v2 for Makefile
- Separate patch for each sample module

DAMON cannot be used from loadable modules since it is not exporting its
symbols.  This makes use of DAMON unnecessarily tedious.  For example,
basic usage of DAMON was presented at kernel summit 2021[1] by writing
simple loadable DAMON application modules in live.  Because the time was
limited (10 minutes) and the coding was to be done step by step, it had
to make a downstream commit that exports DAMON symbols.  There were
users asking why the source code is now not working.  It was mainly due
to the absence of the symbols export patch on their tree, but also due
to the updated DAMON programming interfaces.  There were also multiple
users requesting support of loadable modules for easier deployment.

There's no reason to avoid exporting DAMON symbols.  However, it would
be better to have concrete use cases of the symbols in the tree
together, for better maintenance of the interface and the use cases.
The kernels summit 2021's live-coded modules could be used for the
purpose.

Expose DAMON API modules for supporting the minimum modules and add the
kernel summit 2021's modules that updated for latest DAMON API under the
samples directory.  The sample modules will be keep updated.  For
exporting more symbols, requesters would be required to merge their
modules using the symbols in the tree together, or updsate the sample
modules to use the newly-exporting symbols in still simple but
reasonable ways.

[1] https://linuxplumbersconf.org/event/11/contributions/984/

Signed-off-by: SeongJae Park <sj@kernel.org>

SeongJae Park (3):
  mm/damon/core: export symbols for supporting loadable modules
  samples: add working set size estimation DAMON sample module
  samples: add proactive reclamation DAMON sample module

 MAINTAINERS                       |   1 +
 mm/damon/core.c                   |  10 +++
 samples/Kconfig                   |   2 +
 samples/Makefile                  |   2 +
 samples/damon/Kconfig             |  30 +++++++++
 samples/damon/Makefile            |   3 +
 samples/damon/damon_sample_prcl.c | 102 ++++++++++++++++++++++++++++++
 samples/damon/damon_sample_wsse.c |  85 +++++++++++++++++++++++++
 8 files changed, 235 insertions(+)
 create mode 100644 samples/damon/Kconfig
 create mode 100644 samples/damon/Makefile
 create mode 100644 samples/damon/damon_sample_prcl.c
 create mode 100644 samples/damon/damon_sample_wsse.c


base-commit: 7fcebba4a540e4508b923f00a3e9c5e4710f147f

Comments

Christoph Hellwig Dec. 5, 2023, 5:12 a.m. UTC | #1 
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.   What you posted here is basically just adding
hooks without even real in-kernel users.
SeongJae Park Dec. 5, 2023, 5:23 p.m. UTC | #2 
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