Re: [RFC PATCH 09/11] mm/damon/core: stop ctx in damon_call() before reruning an error

[SJ Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Concurrent invocations of `__damon_stop()` triggered by canceled `damon_call()`s lead to a double `kthread_stop()` deadlock.
> --
> 
> commit 5b208204f87a1b80335c35ce1b66a42e8f74a1ba
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: stop ctx in damon_call() before reruning an error
> 
> This commit updates damon_call() to ensure the DAMON context is explicitly
> stopped when a call fails or is canceled.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index aa48bf44c2a0b..9c15d44084af4 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -1956,8 +1958,13 @@ int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
> >  		return 0;
> >  	wait_for_completion(&control->completion);
> >  	if (control->canceled)
> > -		return -ECANCELED;
> > +		goto canceled;
> >  	return 0;
> > +
> > +canceled:
> > +	__damon_stop(ctx);
> 
> [Severity: High]
> Could this lead to a deadlock if multiple threads invoke damon_call()
> concurrently on the same context?
> 
> If multiple threads are waiting in damon_call() and the kthread exits, it
> cancels all pending controls and wakes the waiters. These threads will jump
> to the canceled label and call __damon_stop(ctx) concurrently.
> 
> Looking at __damon_stop(), it reads ctx->kdamond without clearing it:
> 
> static void __damon_stop(struct damon_ctx *ctx)
> {
> 	struct task_struct *tsk;
> 
> 	mutex_lock(&ctx->kdamond_lock);
> 	tsk = ctx->kdamond;
> 	if (tsk) {
> 		get_task_struct(tsk);
> 		mutex_unlock(&ctx->kdamond_lock);
> 		kthread_stop_put(tsk);
> 		return;
> 	}
> 	mutex_unlock(&ctx->kdamond_lock);
> }
> 
> Does this allow multiple threads to fetch the same task struct and call
> kthread_stop_put() concurrently? 
> 
> Since kthread_stop() waits on the kthread's exit completion which is
> signaled via complete() (waking only one waiter), would the subsequent
> threads block forever in kthread_stop(), causing a deadlock?

Good finding.  But there is no such parallel usage of damon_call() to my
understanding.

DAMON_SYSFS calls damon_call() for state file writes.  Multiple threads can
write the file in parallel.  But the damon_call() from store file write
callback (state_store()) is protected by global damon_sysfs_lock.

DAMON modules can call damon_call() for parameters commit and initial start.
But param file writing is protected by the param_lock.

This may be better to be documented or make the code more robust.  But no real
issue.

> 
> > +	return -ECANCELED;
> > +
> >  }
> >  
> >  /**
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260704181135.132956-1-sj@kernel.org?part=9


Thanks,
SJ