Re: [RFC PATCH 03/11] mm/damon/core: make damon_stop() never fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] AB-BA deadlock in damon_start() error path when failing to start multiple contexts.
--

commit c787674782fb673ed8c1babcfb03eca187f10a1d
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: make damon_stop() never fails

This commit modifies damon_stop() and __damon_stop() to return void instead
of an error code. This simplifies the error handling in damon_stop() since
it no longer needs to break out of the loop on failure.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 45aa9acef4e30..0845d6b151f81 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -1865,31 +1863,24 @@ static int __damon_stop(struct damon_ctx *ctx)
[ ... ]
>  /**
>   * damon_stop() - Stops the monitorings for a given group of contexts.
>   * @ctxs:	an array of the pointers for contexts to stop monitoring
>   * @nr_ctxs:	size of @ctxs
> - *
> - * Return: 0 on success, negative error code otherwise.
>   */
>  int damon_stop(struct damon_ctx **ctxs, int nr_ctxs)
>  {
> -	int i, err = 0;
> +	int i;
>  
> -	for (i = 0; i < nr_ctxs; i++) {
> +	for (i = 0; i < nr_ctxs; i++)
>  		/* nr_running_ctxs is decremented in kdamond_fn */
> -		err = __damon_stop(ctxs[i]);
> -		if (err)
> -			break;
> -	}
> -	return err;
> +		__damon_stop(ctxs[i]);

[Severity: High]
Does calling damon_stop() here result in an AB-BA deadlock during a partial
startup failure?

While this issue was introduced by an earlier patch in the series, it remains
present here. If damon_start() successfully starts the first context but fails
to start a subsequent one, it calls damon_stop() while still holding
damon_lock:

mm/damon/core.c:damon_start() {
    ...
    mutex_lock(&damon_lock);
    ...
    for (i = 0; i < nr_ctxs; i++) {
        err = __damon_start(ctxs[i]);
        if (err) {
            damon_stop(ctxs, i);
            break;
        }
        nr_running_ctxs++;
    }
    ...
    mutex_unlock(&damon_lock);
}

Inside damon_stop(), __damon_stop() calls kthread_stop_put(), which blocks
waiting for the running kdamond thread to exit. However, kdamond_fn() must
acquire damon_lock to decrement nr_running_ctxs before it can complete its
exit:

mm/damon/core.c:kdamond_fn() {
    ...
done:
    ...
    mutex_lock(&damon_lock);
    nr_running_ctxs--;
    ...
    mutex_unlock(&damon_lock);
    return 0;
}

Can this cause damon_start() and the kdamond thread to deadlock waiting for
each other?

> +	return 0;
>  }

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