mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()
Subsystems (2)
SJ Park
maintainercover mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()
Callers of damon_call() should validate the damon_ctx object parameter. If it is NULL or never damon_start()-ed object, damon_call() could dereference the NULL pointer or indefinitely hang. Ensuring all callers doing the validation correctly has turned out to be difficult. Handle the corner cases inside the core layer and remove callers' validations.
Patches 1 and 2 respectively allow passing NULL and not yet damon_start()-ed ctx parameter to damon_call(). Patches 3 and 4 remove the callers side validations in DMON_RECLAIM and DASMON_LRU_SORT, respectively.
SJ Park (4):
mm/damon/core: handle NULL ctx parameter in damon_call() mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() mm/damon/reclaim: remove unnecessary damon_call() param validation mm/damon/lru_sort: remove unnecessary damon_call() param validation
mm/damon/core.c | 7 +++---- mm/damon/lru_sort.c | 8 -------- mm/damon/reclaim.c | 8 -------- 3 files changed, 3 insertions(+), 20 deletions(-)
base-commit: c8520e181a2aaf85290d0447656de0df112dbbea
Signature · 1 line
2.47.3
SJ Park
maintainerRFC PATCH 1/4 mm/damon/core: handle NULL ctx parameter in damon_call()
When NULL damon_ctx pointer parameter is passed, damon_call() could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are many damon_call() callers. Meanwhile, damon_call() is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check inside damon_call() so that callers can pass the parameter without NULL checks.
--- mm/damon/core.c | 2 ++ 1 file changed, 2 insertions(+)
mm/damon/core.c +2 -0
@@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)*/ int damon_call (struct damon_ctx *ctx,struct damon_call_control *control) { +if (!ctx) +return -EINVAL ;if (!control->repeat)init_completion (&control->completion); control->canceled = false;
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 8 lines
When NULL
damon_ctxpointer parameter is passed,damon_call()could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are manydamon_call()callers. Meanwhile,damon_call()is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check insidedamon_call()so that callers can pass the parameter without NULL checks.Signed-off-by SJ Park <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
In reply to SJ ParkHi SJ,
Quoted 17 lines · SJ Park
When NULL
damon_ctxpointer parameter is passed,damon_call()could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are manydamon_call()callers. Meanwhile,damon_call()is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check insidedamon_call()so that callers can pass the parameter without NULL checks.Signed-off-by SJ Park <[email protected]>--- mm/damon/core.c | 2 ++ 1 file changed, 2 insertions(+)mm/damon/core.c +0
-0
mm/damon/core.c int damon_kdamond_pid(struct damon_ctx *ctx) · line 2195
Quoted 8 lines
mm/damon/core.c +2
-0@@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)*/ int damon_call (struct damon_ctx *ctx,struct damon_call_control *control) { +if (!ctx) +return -EINVAL ;if (!control->repeat)init_completion (&control->completion); control->canceled = false;
I know that this is a minor optimization, but how about moving INIT_LIST_HEAD(&control->list) to right before list_add_tail?
It doesn't make sense to initialize the list before checking call_controls_obsolete.
Maybe this suggestion should go in a different patch.
Signature · 2 lines
Asier Gutierrez Huawei
SJ Park
maintainerRe: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
Hi Asier,
Thank you for your review!
Quoted 32 lines · On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier…
Hi SJ,
Quoted 17 lines · SJ Park
When NULL
damon_ctxpointer parameter is passed,damon_call()could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are manydamon_call()callers. Meanwhile,damon_call()is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check insidedamon_call()so that callers can pass the parameter without NULL checks.Signed-off-by SJ Park <[email protected]>--- mm/damon/core.c | 2 ++ 1 file changed, 2 insertions(+)mm/damon/core.c +0
-0mm/damon/core.c int damon_kdamond_pid(struct damon_ctx *ctx) · line 2195
Quoted 8 lines
mm/damon/core.c +2
-0@@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)*/ int damon_call (struct damon_ctx *ctx,struct damon_call_control *control) { +if (!ctx) +return -EINVAL ;if (!control->repeat)init_completion (&control->completion); control->canceled = false;I know that this is a minor optimization, but how about moving
INIT_LIST_HEAD(&control->list)to right beforelist_add_tail?
Quoted 1 line
It doesn't make sense to initialize the list before checking
call_controls_obsolete.
I agree it might be more efficient.
I think my intention of the code was to to keep the call_controls_lock critical section minimal with access to things that really protected by the mutex. Moving the simple initialization should be fine. As you mentioned, it should be minor change. I'm bit concerned if it will confuse people if the list head is also protected by the mutex, though. What do you think?
Quoted 2 lines
Maybe this suggestion should go in a different patch.
I agree.
Thanks,
SJ
[...]
Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
Quoted 41 lines · SJ Park
Hi Asier,
Thank you for your review!
Quoted 32 lines · On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier…
Hi SJ,
Quoted 17 lines · SJ Park
When NULL
damon_ctxpointer parameter is passed,damon_call()could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are manydamon_call()callers. Meanwhile,damon_call()is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check insidedamon_call()so that callers can pass the parameter without NULL checks.Signed-off-by SJ Park <[email protected]>--- mm/damon/core.c | 2 ++ 1 file changed, 2 insertions(+)mm/damon/core.c +0
-0mm/damon/core.c int damon_kdamond_pid(struct damon_ctx *ctx) · line 2195
Quoted 8 lines
mm/damon/core.c +2
-0@@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)*/ int damon_call (struct damon_ctx *ctx,struct damon_call_control *control) { +if (!ctx) +return -EINVAL ;if (!control->repeat)init_completion (&control->completion); control->canceled = false;I know that this is a minor optimization, but how about moving
INIT_LIST_HEAD(&control->list)to right beforelist_add_tail?Quoted 1 line
It doesn't make sense to initialize the list before checking
call_controls_obsolete.
Quoted 7 lines
I agree it might be more efficient.
I think my intention of the code was to to keep the
call_controls_lockcritical section minimal with access to things that really protected by the mutex. Moving the simple initialization should be fine. As you mentioned, it should be minor change. I'm bit concerned if it will confuse people if the list head is also protected by the mutex, though. What do you think?
Good point, I didn't think about it.
Given that damon_call is not called often, I believe user experience should be more important than efficiency. Anyway, you have the last word.
Quoted 10 lines
Quoted 3 lines
Maybe this suggestion should go in a different patch.
I agree.
Thanks,
SJ[...]
Signature · 2 lines
Asier Gutierrez Huawei
SJ Park
maintainerRe: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
Quoted 51 lines · On Thu, 13 Aug 2026 17:27:51 +0300 Gutierrez Asier…
Quoted 41 lines · SJ Park
Hi Asier,
Thank you for your review!
Quoted 32 lines · On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier…
Hi SJ,
Quoted 17 lines · SJ Park
When NULL
damon_ctxpointer parameter is passed,damon_call()could do NULL dereference. The caller is responsible to avoid that. It is easy to forget, and there are manydamon_call()callers. Meanwhile,damon_call()is never meant to be performance critical. It uses mutex and completion. Add the NULL pointer check insidedamon_call()so that callers can pass the parameter without NULL checks.Signed-off-by SJ Park <[email protected]>--- mm/damon/core.c | 2 ++ 1 file changed, 2 insertions(+)mm/damon/core.c +0
-0mm/damon/core.c int damon_kdamond_pid(struct damon_ctx *ctx) · line 2195
Quoted 8 lines
mm/damon/core.c +2
-0@@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)*/ int damon_call (struct damon_ctx *ctx,struct damon_call_control *control) { +if (!ctx) +return -EINVAL ;if (!control->repeat)init_completion (&control->completion); control->canceled = false;I know that this is a minor optimization, but how about moving
INIT_LIST_HEAD(&control->list)to right beforelist_add_tail?Quoted 1 line
It doesn't make sense to initialize the list before checking
call_controls_obsolete.Quoted 7 lines
I agree it might be more efficient.
I think my intention of the code was to to keep the
call_controls_lockcritical section minimal with access to things that really protected by the mutex. Moving the simple initialization should be fine. As you mentioned, it should be minor change. I'm bit concerned if it will confuse people if the list head is also protected by the mutex, though. What do you think?
Quoted 4 lines
Good point, I didn't think about it.
Given that
damon_callis not called often, I believe user experience should be more important than efficiency. Anyway, you have the last word.
Thank you for the quick answer, Asier. I understand you are find with keeping this as is. Let's keep this as is for now.
Thanks,
SJ
[...]
SJ Park
maintainerRFC PATCH 2/4 mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
In reply to SJ Parkdamon_ctx->call_controls_obsolete is used to disallow damon_call() requests when the request cannot be served. The field is unset and set when the context execution is started and terminated, respectively. The intention is to allow damon_call() requests only while the context is actively being executed.
damon_ctx constructor, damon_new_ctx() unsets the field, though. As a result, passing the damon_ctx parameter that never successfully damon_start()-ed to damon_call() can indefinitely hang. The callers should ensure to avoid the case. Such parameter validation is not always simple. Actually such bugs in DAMON_RECLAIM and DAMON_LRU_SORT have been found and fixed [1].
Set the field in damon_new_ctx(), so that DAMON API callers can pass the context parameter to damon_call() without the additional check.
[1] https://lore.kernel.org/[email protected]
--- mm/damon/core.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
mm/damon/core.c +1 -4
@@ -920,6 +920,7 @@ struct damon_ctx *damon_new_ctx(void)INIT_LIST_HEAD (&ctx->adaptive_targets);INIT_LIST_HEAD (&ctx->schemes); + ctx->call_controls_obsolete = true;prandom_seed_state (&ctx->rnd_state,get_random_u64 ());return ctx;
@@ -2178,10 +2179,6 @@ int damon_kdamond_pid(struct damon_ctx *ctx)* synchronization. The return value of the function will be saved in * &damon_call_control->return_code. * -* Note that this function should be called only after damon_start() with the -* @ctx has succeeded. Otherwise, this function could fall into an indefinite -* wait. -* * When this function is failed, the @ctx is guaranteed to be stopped. * * Return: 0 on success, negative error code otherwise.
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The patch introduces an inconsistent state in the `damon_ctx` API by initializing…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces an inconsistent state in the damon_ctx API by initializing call_controls_obsolete to true while leaving its identically-designed sibling field walk_control_obsolete as false. --
--- Patch [2]: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() --- commit 94c12d6bfcca8f83b408960a36d0c0a8519d80c7
Author: SJ Park <[email protected]>
mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
This commit addresses an issue where passing an unstarted DAMON context to damon_call() can cause an indefinite hang. It fixes this by initializing ctx->call_controls_obsolete to true in damon_new_ctx().
mm/damon/core.c struct damon_ctx *damon_new_ctx(void) · line 867
Quoted 9 lines
mm/damon/core.c +1
-0@@ -864,6 +864,7 @@ struct damon_ctx *damon_new_ctx(void)INIT_LIST_HEAD (&ctx->adaptive_targets);INIT_LIST_HEAD (&ctx->schemes); + ctx->call_controls_obsolete = true;
[Severity: Medium]
Should ctx->walk_control_obsolete also be initialized to true here?
Because walk_control_obsolete is zero-initialized to false during allocation, an unstarted context passed to damos_walk() will bypass the obsolete check:
mm/damon/core.c:damos_walk() {
...
if (ctx->walk_control_obsolete) {
mutex_unlock(&ctx->walk_control_lock);
return -ECANCELED;
}
...
}This appears to leave damos_walk() vulnerable to the exact same indefinite wait_for_completion() hang on unstarted contexts that this patch resolves for damon_call().
Does it make sense to update walk_control_obsolete alongside call_controls_obsolete to ensure the API behaves consistently?
Quoted 3 lines
mm/damon/core.c +0
-0prandom_seed_state (&ctx->rnd_state,get_random_u64 ());return ctx;
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
SJ Park
maintainerRe: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
Quoted 41 lines · On Thu, 13 Aug 2026 04:02:07 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces an inconsistent state in thedamon_ctxAPI by initializingcall_controls_obsoleteto true while leaving its identically-designed sibling fieldwalk_control_obsoleteas false. ----- Patch [2]: [RFC PATCH 2/4]
mm/damon/core: setctx->call_controls_obsoleteindamon_new_ctx()--- commit94c12d6bfcca8f83b408960a36d0c0a8519d80c7
Author: SJ Park <[email protected]>
mm/damon/core: setctx->call_controls_obsoleteindamon_new_ctx()This commit addresses an issue where passing an unstarted DAMON context to
damon_call()can cause an indefinite hang. It fixes this by initializingctx->call_controls_obsoleteto true indamon_new_ctx().mm/damon/core.c struct damon_ctx *damon_new_ctx(void) · line 867
Quoted 9 lines
mm/damon/core.c +1
-0@@ -864,6 +864,7 @@ struct damon_ctx *damon_new_ctx(void)INIT_LIST_HEAD (&ctx->adaptive_targets);INIT_LIST_HEAD (&ctx->schemes); + ctx->call_controls_obsolete = true;[Severity: Medium]
Shouldctx->walk_control_obsoletealso be initialized to true here?Because
walk_control_obsoleteis zero-initialized to false during allocation, an unstarted context passed todamos_walk()will bypass the obsolete check:mm/damon/core.c:damos_walk() { ... if (ctx->walk_control_obsolete) { mutex_unlock(&ctx->walk_control_lock); return -ECANCELED; } ... }
Quoted 6 lines
This appears to leave
damos_walk()vulnerable to the exact same indefinitewait_for_completion()hang on unstarted contexts that this patch resolves fordamon_call().Does it make sense to update
walk_control_obsoletealongsidecall_controls_obsoleteto ensure the API behaves consistently?
The issue could happen in damon_call() because it has repeat mode. damos_walk() doesn't have the repeat mode, so I don't think the initialization is needed right now. I will consider doing so in future, for consistency or a case that it turns out the infinite hang is really possible.
Quoted 9 lines
Quoted 4 lines
mm/damon/core.c +0
-0prandom_seed_state (&ctx->rnd_state,get_random_u64 ());return ctx;[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Thanks,
SJ
SJ Park
maintainerRFC PATCH 3/4 mm/damon/reclaim: remove unnecessary damon_call() param validation
In reply to SJ ParkDAMON_RECLAIM avoids passing NULL or unstarted damon_ctx to damon_call() with its own validation. The validation is no longer needed, because the DAMON core layer now handles the corner cases itself. Remove the unnecessary check.
--- mm/damon/reclaim.c | 8 -------- 1 file changed, 8 deletions(-)
mm/damon/reclaim.c +0 -8
@@ -271,8 +271,6 @@ static int damon_reclaim_commit_inputs_fn(void *arg)return damon_reclaim_apply_parameters (); } -static bool damon_reclaim_damon_has_started; -static int damon_reclaim_commit_inputs_store (const char *val,const struct kernel_param *kp) {
@@ -293,10 +291,6 @@ static int damon_reclaim_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_reclaim_damon_has_started) -return -EINVAL ; - err =damon_call (ctx, &control);return err ? err : control.return_code;
@@ -343,8 +337,6 @@ static int damon_reclaim_turn(bool on) err =damon_start (&ctx,1 , true);if (err)return err; -if (!damon_reclaim_damon_has_started) - damon_reclaim_damon_has_started = true;return damon_call (ctx, &call_control); }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent invocation of damon_call() during kthread startup leads to an infinite kernel loop. --
--- Patch [3]: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation --- commit 07f6c5438f6069b0b8225e26e3726d6aa4e3da09
Author: SJ Park <[email protected]>
mm/damon/reclaim: remove unnecessary damon_call() param validation
DAMON_RECLAIM avoids passing NULL or unstarted damon_ctx to damon_call() with its own validation. The validation is no longer needed, because the DAMON core layer now handles the corner cases itself. Remove the unnecessary check.
Quoted 4 lines
mm/damon/reclaim.c +0
-0
[ ... ]
mm/damon/reclaim.c static int damon_reclaim_commit_inputs_store(const char *val, · line 299
Quoted 8 lines
mm/damon/reclaim.c +0
-4@@ -293,10 +291,6 @@ static int damon_reclaim_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_reclaim_damon_has_started) -return -EINVAL ; -
[Severity: High]
This is a pre-existing issue, but does removing this validation expose an infinite loop in the DAMON core?
If userspace writes to this sysfs file concurrently with damon_start(), damon_call() in mm/damon/core.c can hit a race condition. The core initializes ctx->call_controls_obsolete to true. If damon_call() runs before the kthread fully starts, it can read call_controls_obsolete == true and jump to the canceled label:
mm/damon/core.c:damon_call() {
...
if (ctx->call_controls_obsolete) {
mutex_unlock(&ctx->call_controls_lock);
goto canceled;
}
...
canceled:
while (damon_is_running(ctx))
schedule_timeout_idle(msecs_to_jiffies(100));
return -ECANCELED;
}If damon_start() concurrently sets ctx->kdamond to non-NULL, damon_is_running(ctx) becomes true. Because the kthread has just started, it runs indefinitely, and damon_call() gets trapped in the while loop forever.
Quoted 3 lines
mm/damon/reclaim.c +0
-0err =damon_call (ctx, &control);return err ? err : control.return_code;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
SJ Park
maintainerRe: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
Quoted 54 lines · On Thu, 13 Aug 2026 04:01:40 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent invocation ofdamon_call()during kthread startup leads to an infinite kernel loop. ----- Patch [3]: [RFC PATCH 3/4]
mm/damon/reclaim: remove unnecessarydamon_call()param validation --- commit07f6c5438f6069b0b8225e26e3726d6aa4e3da09
Author: SJ Park <[email protected]>
mm/damon/reclaim: remove unnecessarydamon_call()param validation
DAMON_RECLAIMavoids passing NULL or unstarteddamon_ctxtodamon_call()with its own validation. The validation is no longer needed, because the DAMON core layer now handles the corner cases itself. Remove the unnecessary check.Quoted 4 lines
mm/damon/reclaim.c +0
-0[ ... ]
mm/damon/reclaim.c static int damon_reclaim_commit_inputs_store(const char *val, · line 299
Quoted 8 lines
mm/damon/reclaim.c +0
-4@@ -293,10 +291,6 @@ static int damon_reclaim_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_reclaim_damon_has_started) -return -EINVAL ; -[Severity: High]
This is a pre-existing issue, but does removing this validation expose an infinite loop in the DAMON core?If userspace writes to this sysfs file concurrently with
damon_start(),damon_call()inmm/damon/core.ccan hit a race condition. The core initializesctx->call_controls_obsoleteto true. Ifdamon_call()runs before the kthread fully starts, it can readcall_controls_obsolete== true and jump to the canceled label:mm/damon/core.c:damon_call() { ... if (ctx->call_controls_obsolete) { mutex_unlock(&ctx->call_controls_lock); goto canceled; } ... canceled: while (damon_is_running(ctx)) schedule_timeout_idle(msecs_to_jiffies(100)); return -ECANCELED; }
Quoted 4 lines
If
damon_start()concurrently setsctx->kdamondto non-NULL,damon_is_running(ctx)becomes true. Because the kthread has just started, it runs indefinitely, anddamon_call()gets trapped in the while loop forever.
Because of the param_lock, that race is impossible.
Quoted 7 lines
Quoted 4 lines
mm/damon/reclaim.c +0
-0err =damon_call (ctx, &control);return err ? err : control.return_code;Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Thanks,
SJ
SJ Park
maintainerRFC PATCH 4/4 mm/damon/lru_sort: remove unnecessary damon_call() param validation
In reply to SJ ParkDAMON_LRU_SORT avoids passing NULL or unstarted damon_ctx to damon_call() with its own validation. The validation is no longer needed, because the DAMON core layer now handles the corner cases itself. Remove the unnecessary check.
--- mm/damon/lru_sort.c | 8 -------- 1 file changed, 8 deletions(-)
mm/damon/lru_sort.c +0 -8
@@ -346,8 +346,6 @@ static int damon_lru_sort_commit_inputs_fn(void *arg)return damon_lru_sort_apply_parameters (); } -static bool damon_lru_sort_damon_has_started; -static int damon_lru_sort_commit_inputs_store (const char *val,const struct kernel_param *kp) {
@@ -368,10 +366,6 @@ static int damon_lru_sort_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_lru_sort_damon_has_started) -return -EINVAL ; - err =damon_call (ctx, &control);return err ? err : control.return_code;
@@ -422,8 +416,6 @@ static int damon_lru_sort_turn(bool on) err =damon_start (&ctx,1 , true);if (err)return err; -if (!damon_lru_sort_damon_has_started) - damon_lru_sort_damon_has_started = true;return damon_call (ctx, &call_control); }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Removing the `damon_lru_sort_damon_has_started` check exposes a race condition causing an infinite…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the damon_lru_sort_damon_has_started check exposes a race condition causing an infinite loop in damon_call() when invoked concurrently during kdamond startup. --
--- Patch [4]: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation --- commit 288d310b5c3c2bd758d5db8eef7c717db03cbd01
Author: SJ Park <[email protected]>
mm/damon/lru_sort: remove unnecessary damon_call() param validation
This commit removes custom validation in damon_lru_sort_commit_inputs_store that checked if the damon context had successfully started before calling damon_call. The commit message states this is safe because the damon core layer now handles these corner cases.
Quoted 4 lines
mm/damon/lru_sort.c +0
-0
[ ... ]
mm/damon/lru_sort.c static int damon_lru_sort_commit_inputs_store(const char *val, · line 369
Quoted 9 lines
mm/damon/lru_sort.c +0
-4@@ -368,10 +366,6 @@ static int damon_lru_sort_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_lru_sort_damon_has_started) -return -EINVAL ; - err =damon_call (ctx, &control);
[Severity: High]
Does removing this safeguard expose a race condition causing an infinite loop in damon_call when invoked concurrently during kdamond startup?
If a user writes to the enabled sysfs file to start the daemon, and concurrently writes to commit_inputs, damon_call can execute while damon_start is waiting for kdamond to fully initialize.
In this scenario, damon_call could read ctx->call_controls_obsolete == true (since the kdamond thread hasn't cleared it yet) and jump to the canceled path:
mm/damon/core.c:damon_call() {
...
canceled:
while (damon_is_running(ctx))
schedule_timeout_idle(msecs_to_jiffies(100));
return -ECANCELED;
}Because the daemon thread has already been spawned and assigned, making damon_is_running persistently true, won't this loop indefinitely, hanging the sysfs writer process?
Quoted 2 lines
mm/damon/lru_sort.c +0
-0return err ? err : control.return_code;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
SJ Park
maintainerRe: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
Quoted 50 lines · On Thu, 13 Aug 2026 04:04:35 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing thedamon_lru_sort_damon_has_startedcheck exposes a race condition causing an infinite loop indamon_call()when invoked concurrently during kdamond startup. ----- Patch [4]: [RFC PATCH 4/4]
mm/damon/lru_sort: remove unnecessarydamon_call()param validation --- commit288d310b5c3c2bd758d5db8eef7c717db03cbd01
Author: SJ Park <[email protected]>
mm/damon/lru_sort: remove unnecessarydamon_call()param validationThis commit removes custom validation in
damon_lru_sort_commit_inputs_storethat checked if the damon context had successfully started before callingdamon_call. The commit message states this is safe because the damon core layer now handles these corner cases.Quoted 4 lines
mm/damon/lru_sort.c +0
-0[ ... ]
mm/damon/lru_sort.c static int damon_lru_sort_commit_inputs_store(const char *val, · line 369
Quoted 9 lines
mm/damon/lru_sort.c +0
-4@@ -368,10 +366,6 @@ static int damon_lru_sort_commit_inputs_store(const char *val,if (!commit_inputs_request)return 0 ; -/* Skip damon_call() if ctx has not successfully started. */ -if (!damon_lru_sort_damon_has_started) -return -EINVAL ; - err =damon_call (ctx, &control);[Severity: High]
Does removing this safeguard expose a race condition causing an infinite loop indamon_callwhen invoked concurrently during kdamond startup?If a user writes to the enabled sysfs file to start the daemon, and concurrently writes to
commit_inputs,damon_callcan execute whiledamon_startis waiting for kdamond to fully initialize.In this scenario,
damon_callcould readctx->call_controls_obsolete== true (since the kdamond thread hasn't cleared it yet) and jump to the canceled path:mm/damon/core.c:damon_call() { ... canceled: while (damon_is_running(ctx)) schedule_timeout_idle(msecs_to_jiffies(100)); return -ECANCELED; }
Quoted 3 lines
Because the daemon thread has already been spawned and assigned, making
damon_is_runningpersistently true, won't this loop indefinitely, hanging the sysfs writer process?
No. The race doesn't happen due to params_lock.
Quoted 6 lines
Quoted 3 lines
mm/damon/lru_sort.c +0
-0return err ? err : control.return_code;Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Thanks,
SJ