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

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

Sashiko found some issues, so I will send my comment for it soon.

Forwarding sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260413185249.5921-3-aethernet65535@gmail.com
# start of sashiko.dev inline review
commit cefd2ed657fa3baedb834d79191ef3af7c5363ac
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/reclaim: reset thread status parameters upon kdamond termination

This commit aims to fix an issue where an unexpected kdamond termination
leaves the enabled parameter as 'Y' and a stale kdamond_pid, preventing
the user from restarting DAMON without a reboot. It adds fallback logic
to reset these parameters when damon_commit_ctx() fails or when
damon_stop() fails but the thread is no longer running.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 86da147786583..e3e148fd80f97 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
[ ... ]
> @@ -250,6 +250,10 @@ static int damon_reclaim_apply_parameters(void)
>  	if (err)
>  		goto out;
>  	err = damon_commit_ctx(ctx, param_ctx);
> +	if (err) {
> +		enabled = false;
> +		kdamond_pid = -1;
> +	}
>  out:
>  	damon_destroy_ctx(param_ctx);
>  	return err;

Can prematurely resetting enabled to false here introduce a race condition
leading to a use-after-free of the DAMON context structures?

If damon_reclaim_apply_parameters() is invoked from the kdamond worker thread
(for example, when applying commit_inputs) and damon_commit_ctx() fails,
ctx->maybe_corrupted is set to true. This signals the kdamond thread to
break its main loop and begin its teardown phase, such as executing
damon_destroy_targets() to free lists.

Because enabled is set to false asynchronously here, a concurrent sysfs write
of 'Y' to enabled will succeed and immediately trigger
damon_reclaim_turn(true). This unconditionally calls
damon_reclaim_apply_parameters() and executes damon_commit_ctx() from the
sysfs thread.

Since damon_commit_ctx() locklessly mutates and frees items in
ctx->adaptive_targets and ctx->schemes, would this race directly with the
still-exiting kdamond thread traversing and freeing those exact same lists,
resulting in list corruption and a use-after-free?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260413185249.5921-3-aethernet65535@gmail.com

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260413185249.5921-3-aethernet65535@gmail.com
#
# [1] https://github.com/sjp38/hackermail