* [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
2026-08-13 3:49 [RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
@ 2026-08-13 3:49 ` SJ Park
2026-08-13 3:58 ` sashiko-bot
2026-08-13 8:33 ` Gutierrez Asier
2026-08-13 3:49 ` [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: SJ Park @ 2026-08-13 3:49 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
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.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 92631a36d7b51..5882f9c94c47f 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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;
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
2026-08-13 3:49 ` [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
@ 2026-08-13 3:58 ` sashiko-bot
2026-08-13 8:33 ` Gutierrez Asier
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 3:58 UTC (permalink / raw)
To: SJ Park; +Cc: damon
> 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.
>
> Signed-off-by: SJ Park <sj@kernel.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
2026-08-13 3:49 ` [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
2026-08-13 3:58 ` sashiko-bot
@ 2026-08-13 8:33 ` Gutierrez Asier
1 sibling, 0 replies; 13+ messages in thread
From: Gutierrez Asier @ 2026-08-13 8:33 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, damon, linux-kernel, linux-mm
Hi SJ,
On 8/13/2026 6:49 AM, SJ Park wrote:
> 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.
>
> Signed-off-by: SJ Park <sj@kernel.org>
> ---
> mm/damon/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 92631a36d7b51..5882f9c94c47f 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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.
--
Asier Gutierrez
Huawei
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
2026-08-13 3:49 [RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
2026-08-13 3:49 ` [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
@ 2026-08-13 3:49 ` SJ Park
2026-08-13 4:02 ` sashiko-bot
2026-08-13 3:49 ` [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
2026-08-13 3:49 ` [RFC PATCH 4/4] mm/damon/lru_sort: " SJ Park
3 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-08-13 3:49 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
damon_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/20260803134646.16640-1-sj@kernel.org
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 5882f9c94c47f..5a92e4fac6d92 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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.
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
2026-08-13 3:49 ` [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
@ 2026-08-13 4:02 ` sashiko-bot
2026-08-13 4:10 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 4:02 UTC (permalink / raw)
To: SJ Park; +Cc: damon
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 <sj@kernel.org>
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().
Link: https://lore.kernel.org/20260803134646.16640-1-sj@kernel.org
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index af7b77710745d..5080cb12220d2 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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?
> prandom_seed_state(&ctx->rnd_state, get_random_u64());
>
> return ctx;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
2026-08-13 4:02 ` sashiko-bot
@ 2026-08-13 4:10 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-08-13 4:10 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 13 Aug 2026 04:02:07 +0000 sashiko-bot@kernel.org wrote:
> 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 <sj@kernel.org>
>
> 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().
>
> Link: https://lore.kernel.org/20260803134646.16640-1-sj@kernel.org
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index af7b77710745d..5080cb12220d2 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -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?
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.
>
> > prandom_seed_state(&ctx->rnd_state, get_random_u64());
> >
> > return ctx;
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=2
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
2026-08-13 3:49 [RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
2026-08-13 3:49 ` [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
2026-08-13 3:49 ` [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
@ 2026-08-13 3:49 ` SJ Park
2026-08-13 4:01 ` sashiko-bot
2026-08-13 3:49 ` [RFC PATCH 4/4] mm/damon/lru_sort: " SJ Park
3 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-08-13 3:49 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
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.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/reclaim.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 45d5557cc575a..42a2c9cb13431 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -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);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
2026-08-13 3:49 ` [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
@ 2026-08-13 4:01 ` sashiko-bot
2026-08-13 4:11 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 4:01 UTC (permalink / raw)
To: SJ Park; +Cc: damon
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 <sj@kernel.org>
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.
> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 45d5557cc575a..42a2c9cb13431 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
[ ... ]
> @@ -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.
> err = damon_call(ctx, &control);
>
> return err ? err : control.return_code;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
2026-08-13 4:01 ` sashiko-bot
@ 2026-08-13 4:11 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-08-13 4:11 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 13 Aug 2026 04:01:40 +0000 sashiko-bot@kernel.org wrote:
> 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 <sj@kernel.org>
>
> 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.
>
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 45d5557cc575a..42a2c9cb13431 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> [ ... ]
> > @@ -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.
Because of the param_lock, that race is impossible.
>
> > err = damon_call(ctx, &control);
> >
> > return err ? err : control.return_code;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=3
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
2026-08-13 3:49 [RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
` (2 preceding siblings ...)
2026-08-13 3:49 ` [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
@ 2026-08-13 3:49 ` SJ Park
2026-08-13 4:04 ` sashiko-bot
3 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-08-13 3:49 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
DAMON_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.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/lru_sort.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index bd847829a9907..f25ee7326e87c 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -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);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
2026-08-13 3:49 ` [RFC PATCH 4/4] mm/damon/lru_sort: " SJ Park
@ 2026-08-13 4:04 ` sashiko-bot
2026-08-13 4:12 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 4:04 UTC (permalink / raw)
To: SJ Park; +Cc: damon
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 <sj@kernel.org>
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.
> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index bd847829a9907..f25ee7326e87c 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
[ ... ]
> @@ -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?
>
> return err ? err : control.return_code;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
2026-08-13 4:04 ` sashiko-bot
@ 2026-08-13 4:12 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-08-13 4:12 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Thu, 13 Aug 2026 04:04:35 +0000 sashiko-bot@kernel.org wrote:
> 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 <sj@kernel.org>
>
> 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.
>
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index bd847829a9907..f25ee7326e87c 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> [ ... ]
> > @@ -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?
No. The race doesn't happen due to params_lock.
>
> >
> > return err ? err : control.return_code;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=4
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread