Re: [RFC PATCH 01/11] mm/damon/core: stop ctxs in damon_start() before returning an error

[SJ Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Calling damon_stop() from within damon_start() while holding damon_lock causes an AB-BA deadlock with kdamond_fn().
> --
> 
> commit 1cec89c3cfde2c1ab34bcccb4767280aac126514
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: stop ctxs in damon_start() before returning an error
> 
> When multiple contexts are passed to damon_start(), it starts them one
> by one and returns an error immediately on failure. This commit adds logic
> to stop any already-started contexts directly within damon_start() if a
> subsequent context fails to start.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c6fc35be633b3..45aa9acef4e30 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1836,8 +1836,10 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
> >  
> >  	for (i = 0; i < nr_ctxs; i++) {
> >  		err = __damon_start(ctxs[i]);
> > -		if (err)
> > +		if (err) {
> > +			damon_stop(ctxs, i);
> 
> [Severity: High]
> Will this cause a deadlock if a context fails to start?
> 
> damon_start() holds damon_lock while iterating through the contexts. When it
> encounters an error, it calls damon_stop() which synchronously waits for the
> previously started kthreads to stop.
> 
> However, the kdamond_fn thread attempts to acquire the same damon_lock
> during its cleanup phase before it can exit:
> 
> kdamond_fn() {
>         ...
>         mutex_lock(&damon_lock);
>         nr_running_ctxs--;
>         ...
>         mutex_unlock(&damon_lock);
> }
> 
> Since damon_start() is blocking on the thread exit while holding damon_lock,
> and the thread is blocking on damon_lock to exit, doesn't this result in a
> circular wait?

Good finding.  I will fix this in the next revision by calling damon_stop()
after releasing the mutex, like below.

"""
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2182,16 +2182,17 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)

        for (i = 0; i < nr_ctxs; i++) {
                err = __damon_start(ctxs[i]);
-               if (err) {
-                       damon_stop(ctxs, i);
+               if (err)
                        break;
-               }
                nr_running_ctxs++;
        }
        if (exclusive && nr_running_ctxs)
                running_exclusive_ctxs = true;
        mutex_unlock(&damon_lock);

+       if (i != nr_ctxs)
+               damon_stop(ctxs, i);
+
        return err;
 }
"""


Thanks,
SJ

[...]