Re: [RFC PATCH] mm/damon: reset thread status parameters upon kdamond termination

[Liew Rui Yan <aethernet65535@gmail.com>]

Hi SeongJae,

On Mon, 30 Mar 2026 22:02:28 -0700 SeongJae Park <sj@kernel.org> wrote:

> Nice catch!
> 
> I guess this can easily reproducible?  Sharing detailed reproduction steps
> would be nice.

I will include the reproduction steps in the next version.

> > Solution
> > ========
> > Introduce a 'thread_status' structure to link the internal kdamond
> > state with module parameters ('enabled' and 'kdamond_pid').
> > 
> > Specifically:
> > 1. Extend 'struct damon_ctx' to include pointers to the module's
> >    parameters.
> > 2. Initialize these pointers in damon_lru_sort_apply_parameters()
> >    and damon_reclaim_apply_parameters() to point the respective
> >    module variables.
> > 3. Implement damon_update_thread_status() to reset 'enabled' to
> >    false and 'kdamond_pid' to -1 when the kdamond thread finishes.
> 
> This feels too much extension of core API for a problem that can more simply be
> fixed.  Can't we detect the unexpected termination of kdamond from the modules
> and update the paramter values accordingly?
> 
> If we cannot due to a limitation of the DAMON core API, I'd like to extend the
> API for letting the caller detects the unexpected termination.
> 
> Because this is an RFC and I already have question for the high level
> direction, I will skip code level review.
> I agree that the current approach was too heavy as it forced the DAMON
> Core to directly manipulate module-specific parameters, leading to tight
> coupling between layers.

To address this, I've brainstormed two alternative directions and would
love to hear your thoughts:

Option 1: Introduce a generic termination callback
==================================================
Add 'void *after_terminate_fn(void*)' and 'void *after_terminate_data'
to 'struct damon_ctx' or 'struct damon_operations'. While this extends
the Core API, it provides a clean notification mechanism. When kdamond
exits for any reason, the module can perform its own cleanup (e.g.,
resetting 'enabled' and 'kdamond_pid') within its own callback. This
keeps the core logic decoupled from module parameters.

Option 2: On-demand state correction in the module
==================================================
In damon_{lru_sort, reclaim}_enabled_store(), if damon_stop() fails, we
check is_kdamond_running(). If the kdamond is found to be terminated, we
forcibly reset 'enabled' and 'kdamond_pid'.

My perspective
--------------
I personally prefer OPTION-1 because it ensures the sysfs state in
synchronized actively.

OPTION-2 is simpler and avoids API changes, but it's a "passive" fix
that only triggers when user atttempts a write operation. User might
still see inconsistent value until they try to interact with the module.

I value your judgment on which path better aligns with DAMON's long-term
design philosophy, as your experience with real-world kernel maintenance
is far beyond mine. :>

Best regards,
Rui Yan

[...]