Re: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A TOCTOU race between `damon_start()` and `damon_call()` can cause `damon_call()` to loop indefinitely.
--

--- Patch [2]: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started ---
commit 9a410362ed5a4418640b6bf73c4ee49300c9dc16
Author: SJ Park <sj@kernel.org>

mm/damon/lru_sort: skip damon_call() if ctx has not started

This commit prevents damon_call() from hanging when the DAMON context is
initialized but not yet started. It introduces a boolean flag to track whether
the context has ever successfully started, and skips the call if it has not.

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 7e077084cb03a..7df45f9a0b3ae 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
[ ... ]
> @@ -364,11 +366,8 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
>  	if (!commit_inputs_request)
>  		return 0;
>  
> -	/*
> -	 * Skip damon_call() if ctx is not initialized to avoid
> -	 * NULL pointer dereference.
> -	 */
> -	if (!ctx)
> +	/* Skip damon_call() if ctx has not successfully started. */
> +	if (!damon_lru_sort_damon_has_started)
>  		return -EINVAL;
>  
>  	err = damon_call(ctx, &control);

[Severity: High]
This is a pre-existing issue, but could there be a TOCTOU race here between
damon_start() and damon_call() that causes damon_call() to loop indefinitely?

If a user starts DAMON (setting damon_lru_sort_damon_has_started = true),
stops it (leaving call_controls_obsolete = true), and then concurrently
starts DAMON again while calling damon_call() via commit_inputs, can
damon_call() get stuck?

Looking at mm/damon/core.c, __damon_start() sets ctx->kdamond before the new
thread clears ctx->call_controls_obsolete:

mm/damon/core.c:__damon_start() {
    ...
    ctx->kdamond = kthread_run(kdamond_fn, ctx, "kdamond.%d", nr_running_ctxs);
    ...
}

If damon_call() runs concurrently, it checks call_controls_obsolete without
holding kdamond_lock, sees the stale true value, and jumps to the canceled
label:

mm/damon/core.c:damon_call() {
    ...
    if (ctx->call_controls_obsolete) {
        ...
        goto canceled;
    }
    ...
canceled:
    while (damon_is_running(ctx))
        schedule_timeout_idle(msecs_to_jiffies(100));
    return -ECANCELED;
}

Since damon_is_running() returns true (because ctx->kdamond is set), and the
new healthy thread will run indefinitely, will this task loop forever in
TASK_IDLE?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802185430.9940-1-sj@kernel.org?part=2