[RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()

[RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()

[SJ Park]

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

[RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()

[SJ Park]

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

[RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()

[SJ Park]

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

[RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation

[SJ Park]

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

[RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation

[SJ Park]

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

Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()

[sashiko-bot]

> 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

Re: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation

[sashiko-bot]

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

Re: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()

[sashiko-bot]

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

Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation

[sashiko-bot]

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

Re: [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()

[SJ Park]

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

Re: [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation

[SJ Park]

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

Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation

[SJ Park]

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

Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()

[Gutierrez Asier]

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

Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()

[SJ Park]

Hi Asier,


Thank you for your review!

On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:

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

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?

> 
> 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()

[Gutierrez Asier]

On 8/13/2026 4:55 PM, SJ Park wrote:
> Hi Asier,
> 
> 
> Thank you for your review!
> 
> On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:
> 
>> 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.
> 
> 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?
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.
> 
>>
>> Maybe this suggestion should go in a different patch.
> 
> I agree.
> 
> Thanks,
> SJ
> 
> [...]

-- 
Asier Gutierrez
Huawei

Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()

[SJ Park]

On Thu, 13 Aug 2026 17:27:51 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:

> 
> 
> On 8/13/2026 4:55 PM, SJ Park wrote:
> > Hi Asier,
> > 
> > 
> > Thank you for your review!
> > 
> > On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:
> > 
> >> 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.
> > 
> > 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?
> 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.

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

[...]