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

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

On Tue, 31 Mar 2026 03:51:07 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> [...]
> > > +static void damon_update_thread_status(struct damon_ctx *ctx)
> > > +{
> > > +	if (ctx->thread_status.kdamond_pid)
> > > +		*ctx->thread_status.kdamond_pid = -1;
> > > +	if (ctx->thread_status.enabled)
> > > +		*ctx->thread_status.enabled = false;
> > 
> > Can this write race with the user enabling the module? If a user writes 'Y'
> > to 'enabled', the store function spawns kdamond and prepares to set 'enabled'
> > to true. If kdamond exits immediately (e.g., due to invalid targets), could
> > this line asynchronously set 'enabled' to false before the store function
> > overwrites it with true?
> > 
> > If so, the system would be left in a state where the thread is dead but
> > 'enabled' is true. Subsequent attempts to write 'N' to 'enabled' would fail
> > on damon_stop(), leaving the module permanently locked.
> 
> You are right. I now see the potential race between kdamond exiting and
> the enabled_store(). While it seems unlikely, the window exists and
> could lead to an inconsistent state. I'm sitll thinking about the way to
> synchronize this without introducing new issues. I will try to address
> this is next-version once I have a solid plan.

I think I've found a solution for the race.

In damon_lru_sort_enabled_store(), the current code sets 'enabled =
enable' only after damon_lru_sort_turn(enable) returns successfully.
This creates the window you mentioned.

I plan to update 'enabled' before calling damon_lru_sort_turn(), and
rollback the value only if the turn operation fails.

Best regards,
Rui Yan