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

[SJ Park <sj@kernel.org>]

On Sat, 04 Jul 2026 18:30:02 +0000 sashiko-bot@kernel.org wrote:

> 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?

Yes, I will fix damon_start() in the next revision.


Thanks,
SJ

[...]