mm/damon: refactor damon_{start,stop,commit}() for simple error handling

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 <[email protected]>

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.

mm/damon/core.c int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive) · line 1840

Quoted 11 lines

mm/damon/core.c +2 -1

index c6fc35be633b3..45aa9acef4e30 100644
@@ -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?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ABBA deadlock in damon_start() during error recovery when rolling back successfully started contexts. --

commit c411cd45182c74c483bd6bd5d58aea28123f720d
Author: SJ Park <[email protected]>

samples/damon/mtier: do not stop first context for damon_start() failure

damon_start() was able to fail while keeping the first context running. mtier hence stops the first context in the case. damon_start() is refactored to avoid that. The error handling is no longer necessary. Remove it.

samples/damon/mtier.c static int damon_sample_mtier_start(void) · line 197

Quoted 10 lines

samples/damon/mtier.c +0 -2

index e567f4edd80ea..90881e8bd441f 100644
@@ -193,8 +193,6 @@ static int damon_sample_mtier_start(void)
 	if (!err)
 		return 0;
 
-	if (damon_is_running(ctxs[0]))
-		damon_stop(ctxs, 1);

[Severity: High]
Does the refactored error handling in damon_start() introduce a deadlock?

Since damon_start() is called with nr_ctxs = 2 here, if the first context starts successfully but the second one fails, the error recovery path in damon_start() could trigger an ABBA deadlock.

If damon_start() calls damon_stop() on the error path while holding damon_lock:

mm/damon/core.c:damon_start() {
    ...
		err = __damon_start(ctxs[i]);
		if (err) {
			damon_stop(ctxs, i);
			break;
		}
    ...
}

damon_stop() blocks until the kdamond_fn() thread exits. However, kdamond_fn() attempts to acquire damon_lock at its exit path:

mm/damon/core.c:kdamond_fn() {
    ...
	mutex_lock(&damon_lock);
	nr_running_ctxs--;
	if (!nr_running_ctxs && running_exclusive_ctxs)
		running_exclusive_ctxs = false;
	mutex_unlock(&damon_lock);
    ...
}

Could this cause kdamond_fn() to hang waiting for damon_lock, which is still held by damon_start(), resulting in a deadlock?

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 <[email protected]>
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.

Quoted 4 lines

mm/damon/core.c +0 -0

index 45aa9acef4e30..0845d6b151f81 100644

[ ... ]

Quoted 1 line

mm/damon/core.c +0 -0

@@ -1865,31 +1863,24 @@ static int __damon_stop(struct damon_ctx *ctx)

[ ... ]

Quoted 11 lines

/**
* 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)
{

mm/damon/core.c +1 -1

-	int i, err = 0;
+	int i;

mm/damon/core.c static int __damon_stop(struct damon_ctx *ctx) · line 1955

Quoted 10 lines

mm/damon/core.c +2 -6

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ABBA deadlock in the error handling path of damon_start() when a context fails to start.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] AB-BA deadlock in damon_start() error path with kdamond_fn over damon_lock.

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 <[email protected]>

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.

Quoted 4 lines

mm/damon/core.c +0 -0

index aa48bf44c2a0b..9c15d44084af4 100644

[ ... ]

mm/damon/core.c int damon_call(struct damon_ctx *ctx, struct damon_call_control *control) · line 1965

Quoted 10 lines

mm/damon/core.c +4 -1

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

Quoted 6 lines

mm/damon/core.c +2 -0

+	return -ECANCELED;
+
 }
 
 /**

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Quoted 53 lines · On Sat, 04 Jul 2026 18:25:12 +0000 [email protected] 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 <[email protected]>

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.

Quoted 4 lines

mm/damon/core.c +0 -0

index aa48bf44c2a0b..9c15d44084af4 100644

[ ... ]

mm/damon/core.c int damon_call(struct damon_ctx *ctx, struct damon_call_control *control) · line 1965

Quoted 10 lines

mm/damon/core.c +4 -1

@@ -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);
}

Quoted 6 lines

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.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A self-deadlock occurs in damon_start() if a subsequent context fails to start.