DAMON development mailing list
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [RFC PATCH 10/11] samples/damon/wsse: do not stop ctx for damon_call() failure
Date: Sat, 04 Jul 2026 18:31:59 +0000	[thread overview]
Message-ID: <20260704183159.E0D791F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260704181135.132956-11-sj@kernel.org>

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

commit 2f9c1f0eae64d458e95f8baff7b56262e634bb2f
Author: SJ Park <sj@kernel.org>

samples/damon/wsse: do not stop ctx for damon_call() failure

This commit modifies the error handling path in damon_sample_wsse_start()
when damon_call() fails. It removes the explicit call to damon_stop()
because damon_call() is updated to stop the context on its own.

> diff --git a/samples/damon/wsse.c b/samples/damon/wsse.c
> index ff5e8a890f448..37fd5da201588 100644
> --- a/samples/damon/wsse.c
> +++ b/samples/damon/wsse.c
> @@ -93,10 +93,8 @@ static int damon_sample_wsse_start(void)
>  	}
>  	repeat_call_control.data = ctx;
>  	err = damon_call(ctx, &repeat_call_control);
> -	if (err) {
> -		damon_stop(&ctx, 1);
> +	if (err)
>  		damon_destroy_ctx(ctx);
> -	}
>  	return err;
>  }

[Severity: High]
Does this code expose a self-deadlock in damon_start() if a context fails to
start?

When damon_sample_wsse_start() calls damon_start() earlier in the execution,
it iterates over the contexts and calls __damon_start() to spawn a kdamond
thread for each, all while holding damon_lock. If __damon_start() fails, the
error handling path calls damon_stop() to tear down the already-started
contexts:

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

damon_stop() internally calls __damon_stop(), which invokes kthread_stop_put()
to synchronously wait for the running kdamond thread to exit.

However, when the exiting kdamond thread finishes its main loop and cleans up,
it attempts to acquire damon_lock to decrement nr_running_ctxs:

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

Because the original thread executing damon_start() is still holding
damon_lock while waiting for the kdamond thread to exit, can this result
in a permanent ABBA deadlock?

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

  reply	other threads:[~2026-07-04 18:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-04 18:11 [RFC PATCH 00/11] mm/damon: refactor damon_{start,stop,commit}() for simple error handling SJ Park
2026-07-04 18:11 ` [RFC PATCH 01/11] mm/damon/core: stop ctxs in damon_start() before returning an error SJ Park
2026-07-04 18:20   ` sashiko-bot
2026-07-04 18:40     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 02/11] samples/damon/mtier: do not stop first context for damon_start() failure SJ Park
2026-07-04 18:24   ` sashiko-bot
2026-07-04 18:42     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 03/11] mm/damon/core: make damon_stop() never fails SJ Park
2026-07-04 18:30   ` sashiko-bot
2026-07-04 18:43     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 04/11] mm/damon/sysfs: ignore damon_stop() return value SJ Park
2026-07-04 18:11 ` [RFC PATCH 05/11] mm/damon/reclaaim: " SJ Park
2026-07-04 18:11 ` [RFC PATCH 06/11] mm/damon/lru_sort: " SJ Park
2026-07-04 18:11 ` [RFC PATCH 07/11] mm/damon/core: change damon_stop() return type to void SJ Park
2026-07-04 18:23   ` sashiko-bot
2026-07-04 19:36     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 08/11] samples/damon/mtier: stop all contexts with single damon_stop() call SJ Park
2026-07-04 18:20   ` sashiko-bot
2026-07-04 19:37     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 09/11] mm/damon/core: stop ctx in damon_call() before reruning an error SJ Park
2026-07-04 18:25   ` sashiko-bot
2026-07-04 19:34     ` SJ Park
2026-07-04 19:47       ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 10/11] samples/damon/wsse: do not stop ctx for damon_call() failure SJ Park
2026-07-04 18:31   ` sashiko-bot [this message]
2026-07-04 19:38     ` SJ Park
2026-07-04 18:11 ` [RFC PATCH 11/11] samples/damon/prcl: do not stop DAMON " SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260704183159.E0D791F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox