[v1] mm/damon: define and use DAMON initialization check function

[PATCH 0/7] mm/damon: define and use DAMON initialization check function

Posted by SeongJae Park 2 weeks, 2 days ago

If DAMON is tried to be used by its API callers when it is not yet
successfully initialized, the callers could be crashed.  Such issues
actually happened and were fixed [1].  DAMON API callers are therefore
having their own hacks for seeing if it is safe to use DAMON or not.
Those built on an untreliable assumption that DAMON should be ready to
be used on module init time.  DAMON initialization could fail if
KMEM_CACHE() fails, though.  Also those are basically duplications that
make their maintenance difficult.

Make it reliable and easy to maintain, by implementing a new DAMON core
layer API function for seeing if DAMON is ready to be used or not, and
replacing the hacks of DAMON API callers with the new core layer
function.

Changes from RFC
(https://lore.kernel.org/20250912023946.62337-1-sj@kernel.org)
- Rebase on latest mm-new

[1] https://lore.kernel.org/20250909022238.2989-1-sj@kernel.org

SeongJae Park (7):
  mm/damon/core: implement damon_initialized() function
  mm/damon/stat: use damon_initialized()
  mm/damon/reclaim: use damon_initialized()
  mm/damon/lru_sort: use damon_initialized()
  samples/damon/wsse: use damon_initialized()
  samples/damon/prcl: use damon_initialized()
  samples/damon/mtier: use damon_initialized()

 include/linux/damon.h |  1 +
 mm/damon/core.c       | 10 ++++++++++
 mm/damon/lru_sort.c   |  9 +++++++--
 mm/damon/reclaim.c    |  9 +++++++--
 mm/damon/stat.c       | 10 ++++++----
 samples/damon/mtier.c | 11 +++++++----
 samples/damon/prcl.c  | 11 +++++++----
 samples/damon/wsse.c  | 15 +++++++++------
 8 files changed, 54 insertions(+), 22 deletions(-)


base-commit: b6a9d79765ceb52c8889fd24e1ff3169cc12c80b
-- 
2.39.5

Re: [PATCH 0/7] mm/damon: define and use DAMON initialization check function

Posted by Andrew Morton 2 weeks, 2 days ago

On Mon, 15 Sep 2025 20:35:04 -0700 SeongJae Park <sj@kernel.org> wrote:

> If DAMON is tried to be used by its API callers when it is not yet
> successfully initialized, the callers could be crashed.  Such issues
> actually happened and were fixed [1].  DAMON API callers are therefore
> having their own hacks for seeing if it is safe to use DAMON or not.
> Those built on an untreliable assumption that DAMON should be ready to
> be used on module init time.  DAMON initialization could fail if
> KMEM_CACHE() fails, though.

Wait.  Is there any realistic expectation that KMEM_CACHE() will fail
when DAMON uses it?  We do have the convention of assuming that
__init-time allocations do not fail.  If they do, an oops or panic is
an acceptable response.

Are these problems actually real-world demonstrable things, or has
someone been playing with fault injection or, ...?

> Also those are basically duplications that
> make their maintenance difficult.

Unclear.  This means that the client hacks are no longer necessary after
these changes?

Re: [PATCH 0/7] mm/damon: define and use DAMON initialization check function

Posted by SeongJae Park 2 weeks, 2 days ago

On Mon, 15 Sep 2025 21:33:12 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 15 Sep 2025 20:35:04 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > If DAMON is tried to be used by its API callers when it is not yet
> > successfully initialized, the callers could be crashed.  Such issues
> > actually happened and were fixed [1].  DAMON API callers are therefore
> > having their own hacks for seeing if it is safe to use DAMON or not.
> > Those built on an untreliable assumption that DAMON should be ready to
> > be used on module init time.  DAMON initialization could fail if
> > KMEM_CACHE() fails, though.
> 
> Wait.  Is there any realistic expectation that KMEM_CACHE() will fail
> when DAMON uses it?

Not really.  The commit message is saying just a theoretical and unlikely
possibility.

> We do have the convention of assuming that
> __init-time allocations do not fail.  If they do, an oops or panic is
> an acceptable response.
> 
> Are these problems actually real-world demonstrable things, or has
> someone been playing with fault injection or, ...?

I don't have a way to demonstrate it.  The commit message was only for
discussing a theoretical and unlikely case.  Maybe it was better to just not
mention.

> 
> > Also those are basically duplications that
> > make their maintenance difficult.
> 
> Unclear.  This means that the client hacks are no longer necessary after
> these changes?

You're correct.

Sorry for the poor cover letter message.  If I have to post a new version of
this patch series, I would update the cover letter message as below.  Does it
look better?  If so, could you please update the cover letter part of the
commit on the mm tree?

"""
DAMON is initialized in subsystem initialization time, by damon_init().  If
DAMON API functions are called before the initialization, the system could
crash.  Actually such issues happened and were fixed [1] in the past.  For the
fix, DAMON API callers have updated to check if DAMON is initialized or not,
using their own hacks.  The hacks are unnecessarily duplicated on every DAMON
API callers and therefore it would be difficult to reliably maintain in the
long term.

Make it reliable and easy to maintain.  For this, implement a new DAMON core
layer API function that returns if DAMON is successfully initialized.  If it
returns true, it means DAMON API functions are safe to be used.  After the
introduction of the new API, update DAMON API callers to use the new function
instead of their own hacks.

[1] https://lore.kernel.org/20250909022238.2989-1-sj@kernel.org
"""

As always, please let me know if there is anything I can help.

Thanks,
SJ