mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
Subsystems (3)
SeongJae Park
maintainerRFC PATCH v2 1/1 mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
When kdamond_fn() main loop is finished, the function cancels all remaining damon_call() requests and unset the damon_ctx->kdamond so that API callers can show the context is terminated. damon_call() adds the caller's request to the queue first. After that, it shows if the kdamond of the damon_ctx is still running (damon_ctx->kdamond is set). Only if the kdamond is running, damon_call() starts waiting for the kdamond's handling of the newly added request.
The damon_call() requests registration and damon_ctx->kdamond unset are protected by different mutexes, though. Hence, damon_call() could race with damon_ctx->kdamond unset, and result in deadlocks.
For example, let's suppose kdamond successfully finished the damon_call() requests cancelling. Right after that, damon_call() is called for the context. It registers the new request, and shows the context is still running, because damon_ctx->kdamond unset is not yet done. Hence the damon_call() caller starts waiting for the handling of the request. However, the kdamond is already on the termination steps, so it never handles the new request. As a result, the damon_call() caller threads infinitely waits.
Fix this by introducing another damon_ctx field, namely call_controls_obsolete. It is protected by the damon_ctx->call_controls_lock, which protects damon_call() registration. Initialize (unset) it in kdamond_init_ctx() and set it just before the cancelling of remaining damon_call() is executed. damon_call() reads the obsolete field under the lock and avoids adding a new request.
After this change, only requests that are guaranteed to be handled or cancelled are registered. Hence the after-registration DAMON context termination check is no longer needed. Remove it together.
The issue is found by sashiko [1].
[1] https://lore.kernel.org/[email protected]
---
Changes from RFC v1
(https://lore.kernel.org/[email protected])
- Clarify damon_call() call condition.
- Init call_controls_obsolete before kdamond_started completion.
- Wordsmith commit message.
include/linux/damon.h | 1 + mm/damon/core.c | 45 ++++++++++++++----------------------------- 2 files changed, 15 insertions(+), 31 deletions(-)
include/linux/damon.h +1 -0
@@ -818,6 +818,7 @@ struct damon_ctx {/* lists of &struct damon_call_control */ struct list_head call_controls; +bool call_controls_obsolete;struct mutex call_controls_lock;struct damos_walk_control *walk_control;
mm/damon/core.c +14 -31
77 lines not on this page · raw message
base-commit: 871ee9fc2ebebaa1fa26d4a266ebdf36252b53cf
Signature · 1 line
2.47.3
SeongJae Park
maintainerRe: (sashiko review) [RFC PATCH v2] mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/[email protected] # start of sashiko.dev inline review commit 36b78ee39968830aca3d375219a2cfce73d0d1d0 Author: SeongJae Park <[email protected]>
mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
This commit fixes a race condition between damon_call and the kdamond_fn exit path that could result in a deadlock. It introduces a call_controls_obsolete flag to prevent damon_call from waiting indefinitely for a terminating thread.
Quoted 3 lines
mm/damon/core.c +0
-0
[ ... ]
mm/damon/core.c static int kdamond_fn(void *data) · line 3049
Quoted 9 lines
mm/damon/core.c +3
-0@@ -3062,6 +3042,9 @@ static int kdamond_fn(void *data)damon_destroy_targets (ctx);kfree (ctx->regions_score_histogram); +mutex_lock (&ctx->call_controls_lock); + ctx->call_controls_obsolete = true; +mutex_unlock (&ctx->call_controls_lock);kdamond_call (ctx, true);damos_walk_cancel (ctx);
Does damos_walk suffer from the same exit race regression that this patch fixes for damon_call?
In kdamond_fn, the thread termination sequence executes damos_walk_cancel and only later sets ctx->kdamond = NULL under the kdamond_lock.
damos_walk appears to use the same pattern as the old damon_call where it registers ctx->walk_control, checks damon_is_running (which verifies ctx->kdamond != NULL), and then unconditionally blocks on wait_for_completion.
If a user triggers a DAMOS walk in the exact window after damos_walk_cancel has finished but before ctx->kdamond = NULL is executed, wouldn't damos_walk successfully register its control and pass the damon_is_running check?
If so, it would then block indefinitely on wait_for_completion because the cancellation phase has already passed and the thread is exiting.
Because damos_walk is invoked via state_store with the global damon_sysfs_lock held, could this infinite wait block the entire DAMON sysfs interface for all users on the system?
# end of sashiko.dev inline review # review url: https://sashiko.dev/#/patchset/[email protected] # # hkml [1] generated a draft of this mail. It can be regenerated # using below command: # # hkml patch sashiko_dev --for_forwarding \ # [email protected] # # [1] https://github.com/sjp38/hackermail
Sent using hkml (https://github.com/sjp38/hackermail)
SeongJae Park
maintainerRe: (sashiko review) [RFC PATCH v2] mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
Quoted 46 lines · On Thu, 26 Mar 2026 19:15:45 -0700 SeongJae Park <[email protected]> wrote:
Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/[email protected] # start of sashiko.dev inline review commit 36b78ee39968830aca3d375219a2cfce73d0d1d0 Author: SeongJae Park <[email protected]>
mm/damon/core: fixdamon_call()vskdamond_fn()exit race deadlockThis commit fixes a race condition between
damon_calland thekdamond_fnexit path that could result in a deadlock. It introduces acall_controls_obsoleteflag to preventdamon_callfrom waiting indefinitely for a terminating thread.Quoted 3 lines
mm/damon/core.c +0
-0[ ... ]
mm/damon/core.c static int kdamond_fn(void *data) · line 3049
Quoted 9 lines
mm/damon/core.c +3
-0@@ -3062,6 +3042,9 @@ static int kdamond_fn(void *data)damon_destroy_targets (ctx);kfree (ctx->regions_score_histogram); +mutex_lock (&ctx->call_controls_lock); + ctx->call_controls_obsolete = true; +mutex_unlock (&ctx->call_controls_lock);kdamond_call (ctx, true);damos_walk_cancel (ctx);Does
damos_walksuffer from the same exit race regression that this patch fixes fordamon_call?In
kdamond_fn, the thread termination sequence executesdamos_walk_canceland only later setsctx->kdamond= NULL under thekdamond_lock.
damos_walkappears to use the same pattern as the olddamon_callwhere it registersctx->walk_control, checksdamon_is_running(which verifiesctx->kdamond!= NULL), and then unconditionally blocks onwait_for_completion.If a user triggers a DAMOS walk in the exact window after
damos_walk_cancelhas finished but beforectx->kdamond= NULL is executed, wouldn'tdamos_walksuccessfully register its control and pass thedamon_is_runningcheck?
Quoted 6 lines
If so, it would then block indefinitely on
wait_for_completionbecause the cancellation phase has already passed and the thread is exiting.Because
damos_walkis invoked viastate_storewith the globaldamon_sysfs_lockheld, could this infinite wait block the entire DAMON sysfs interface for all users on the system?
Yes, and I'm working on it separately, as I also replied [1] to the previous version.
[1] https://lore.kernel.org/[email protected]
Thanks,
SJ
[...]
SeongJae Park
maintainerRe: (sashiko review) [RFC PATCH v2] mm/damon/core: fix damon_call() vs kdamond_fn() exit race deadlock
Quoted 51 lines · On Thu, 26 Mar 2026 19:20:13 -0700 SeongJae Park <[email protected]> wrote:
Quoted 49 lines · On Thu, 26 Mar 2026 19:15:45 -0700 SeongJae Park <[email protected]> wrote:
Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/[email protected] # start of sashiko.dev inline review commit 36b78ee39968830aca3d375219a2cfce73d0d1d0 Author: SeongJae Park <[email protected]>
mm/damon/core: fixdamon_call()vskdamond_fn()exit race deadlockThis commit fixes a race condition between
damon_calland thekdamond_fnexit path that could result in a deadlock. It introduces acall_controls_obsoleteflag to preventdamon_callfrom waiting indefinitely for a terminating thread.Quoted 3 lines
mm/damon/core.c +0
-0[ ... ]
mm/damon/core.c static int kdamond_fn(void *data) · line 3049
Quoted 9 lines
mm/damon/core.c +3
-0@@ -3062,6 +3042,9 @@ static int kdamond_fn(void *data)damon_destroy_targets (ctx);kfree (ctx->regions_score_histogram); +mutex_lock (&ctx->call_controls_lock); + ctx->call_controls_obsolete = true; +mutex_unlock (&ctx->call_controls_lock);kdamond_call (ctx, true);damos_walk_cancel (ctx);Does
damos_walksuffer from the same exit race regression that this patch fixes fordamon_call?In
kdamond_fn, the thread termination sequence executesdamos_walk_canceland only later setsctx->kdamond= NULL under thekdamond_lock.
damos_walkappears to use the same pattern as the olddamon_callwhere it registersctx->walk_control, checksdamon_is_running(which verifiesctx->kdamond!= NULL), and then unconditionally blocks onwait_for_completion.If a user triggers a DAMOS walk in the exact window after
damos_walk_cancelhas finished but beforectx->kdamond= NULL is executed, wouldn'tdamos_walksuccessfully register its control and pass thedamon_is_runningcheck?If so, it would then block indefinitely on
wait_for_completionbecause the cancellation phase has already passed and the thread is exiting.
Quoted 6 lines
Quoted 3 lines
Because
damos_walkis invoked viastate_storewith the globaldamon_sysfs_lockheld, could this infinite wait block the entire DAMON sysfs interface for all users on the system?Yes, and I'm working on it separately, as I also replied [1] to the previous version.
Maybe I will make the next version of this patch as a series of two patches, one for this one and the other one for the damos_walk() race fix.
Quoted 2 lines
Thanks,
SJ
[...]
SeongJae Park
maintainerQuoted 21 lines · On Thu, 26 Mar 2026 17:49:51 -0700 SeongJae Park <[email protected]> wrote:
When
kdamond_fn()main loop is finished, the function cancels all remainingdamon_call()requests and unset thedamon_ctx->kdamondso that API callers can show the context is terminated.damon_call()adds the caller's request to the queue first. After that, it shows if the kdamond of thedamon_ctxis still running (damon_ctx->kdamondis set). Only if the kdamond is running,damon_call()starts waiting for the kdamond's handling of the newly added request.The
damon_call()requests registration anddamon_ctx->kdamondunset are protected by different mutexes, though. Hence,damon_call()could race withdamon_ctx->kdamondunset, and result in deadlocks.For example, let's suppose kdamond successfully finished the
damon_call()requests cancelling. Right after that,damon_call()is called for the context. It registers the new request, and shows the context is still running, becausedamon_ctx->kdamondunset is not yet done. Hence thedamon_call()caller starts waiting for the handling of the request. However, the kdamond is already on the termination steps, so it never handles the new request. As a result, thedamon_call()caller threads infinitely waits.
Quoted 4 lines
Fix this by introducing another
damon_ctxfield, namelycall_controls_obsolete. It is protected by thedamon_ctx->call_controls_lock, which protectsdamon_call()registration. Initialize (unset) it inkdamond_init_ctx()
In this veersion, I updated the initialization to be done in kdamond_fn() before the damon_started completion. But I forgot updating the above sentence. I will make the update in the next version.
Thanks,
SJ
[...]