Re: [PATCH v2 0/2] mm/damon: reset thread status parameters upon kdamond termination

[SeongJae Park <sj@kernel.org>]

On Mon, 13 Apr 2026 17:23:03 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Tue, 14 Apr 2026 02:52:47 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> > Problem
> > ========
> 
> Let's align the underline with the subject.  Also, let's add one blank line
> after the underline.
> 
> > When kdamond terminates unexpectedly, 'enabled' remains 'Y' and
> > 'kdamond_pid' remains stale. This prevents user from restarting DAMON
> > because both writing 'Y' and 'N' to 'enabled' will fail.
> > 
> > "Unexpected termination" here means the kdamond exits without any user
> > request (e.g., not by writing 'N' to 'enabled').
> 
> Could you please further explain when such termination can happen?
> 
> > 
> > User Impact
> > ===========
> > Once kdamond terminates this way, it cannot be restarted via sysfs
> > because:
> > 
> > 1. DAMON_LRU_SORT/DAMON_RECLAIM is built into the kernel, so it cannot
> >    be unloaded and reloaded at runtime.
> 
> I think this is quite obvious, so may better to be dropped.
> 
> > 2. Writing 'N' to 'enabled' fails because kdamond no longer exists;
> >    Writing 'Y' does nothing, as 'enabled' is already Y.
> > 
> > As a result, the only way to restore DAMON functionality is a full
> > system reboot.
> 
> Thank you for clarifying the user impact.  I think this deserves Cc-ing
> stable@.
> 
> I think 'Problem' and 'User Impact' can be unified into one section.
> 
> > 
> > Solution
> > ========
> > damon_commit_ctx() sets 'maybe_corrupted=true' at the beginning and only
> > sets it to false upon successful completion. When 'maybe_corrupted'
> > remains true, kdamond will terminate eventually.
> 
> This seems better to be explained earlier, on the problem section.
> 
> > 
> > Therefore:
> > 1. In damon_{lru_sort, reclaim}_turn(): Add fallback logic to reset
> >    parameters when damon_stop() fails but kdamond is not running.
> > 2. In damon_{lru_sort, reclaim}_apply_parameters(): Reset parameters
> >    when damon_commit_ctx() fails, as kdamond will terminate due to
> >    maybe_corrupted mechanism.
> 
> So the problem is that 'enable' parameter value is not trustworthy, and this
> series is trying to make it trustworthy.  I think it is bit complicated,
> especially for stable@ fix.  What about simply using more trustworthy
> information, e.g.,
> 
> '''
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -390,7 +390,7 @@ MODULE_PARM_DESC(addr_unit,
>  static int damon_reclaim_enabled_store(const char *val,
>                 const struct kernel_param *kp)
>  {
> -       bool is_enabled = enabled;
> +       bool is_enabled = false;
>         bool enable;
>         int err;
> 
> @@ -398,6 +398,9 @@ static int damon_reclaim_enabled_store(const char *val,
>         if (err)
>                 return err;
> 
> +       if (ctx)
> +               is_enabled = damon_is_running(ctx);
> +
>         if (is_enabled == enable)
>                 return 0;
> 
> '''
> 
> > 
> > Changes from RFC-v1
> > (https://lore.kernel.org/20260330164347.12772-1-aethernet65535@gmail.com)
> > - Remove RFC tag.
> 
> When dropping RFC tag, let's start from v1 again, from the next time.

Also, I just found the patches don't have Fixes: and Cc: stable@.  Could you
please add those appripriately?


Thanks,
SJ

> 
> 
> Thanks,
> SJ
> 
> [...]
>