Re: [RFC PATCH 5/9] mm/damon/core: document damon_call()/damon_start() race hang issue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Brittle cancellation polling loop in damon_call() can cause indefinite hangs on rapid context restarts.
- [High] Missing initialization of `walk_control_obsolete` in `damon_new_ctx()` causes unrecoverable hangs in `damos_walk()`.
--

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

mm/damon/core: document damon_call()/damon_start() race hang issue

This commit documents a potential race condition and indefinite hang between
damon_start() and damon_call() if they are executed in parallel for the same
DAMON context. It adds a cautionary comment to damon_call() to warn API
users about this behavior.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 6e4b0144c3dd1..f304813a08460 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2218,6 +2218,9 @@ int damon_kdamond_pid(struct damon_ctx *ctx)
>   *
>   * When this function is failed, the @ctx is guaranteed to be stopped.
>   *
> + * This function should not be called in parallel to damon_start() for the
> + * @ctx.  In the case, this function could indefinitely hang.
> + *

[Severity: High]
This is a pre-existing issue, but does the cancellation polling loop in
damon_call() cause indefinite hangs on rapid context restarts?

If Thread A is blocked in damon_call(), and Thread B stops the DAMON context
(e.g., via module parameters), the monitoring thread cancels the call and
exits.

If Thread A is preempted before evaluating the canceled polling loop,
and Thread C concurrently restarts the context, Thread A resumes, evaluates
damon_is_running(ctx) to true (due to the newly spawned thread), and hangs
forever waiting for the new thread to stop:

mm/damon/core.c:damon_call() {
    ...
canceled:
        while (damon_is_running(ctx))
                schedule_timeout_idle(msecs_to_jiffies(100));
        return -ECANCELED;
}

This race can be triggered by root via concurrent sysfs or module parameter
writes (e.g., in damon_reclaim).

[Severity: High]
This is a pre-existing issue, but does the missing initialization of
walk_control_obsolete in damon_new_ctx() cause unrecoverable hangs in
damos_walk()?

damon_new_ctx() explicitly initializes call_controls_obsolete = true to
protect damon_call() from hanging if invoked before the context is started.
However, it fails to initialize walk_control_obsolete:

mm/damon/core.c:damon_new_ctx() {
    ...
        ctx->call_controls_obsolete = true;
        prandom_seed_state(&ctx->rnd_state, get_random_u64());
    ...
}

If damos_walk() is called before damon_start(), it bypasses the safety check
and blocks forever on wait_for_completion(), because no thread exists to
signal it:

mm/damon/core.c:damos_walk() {
    ...
        mutex_lock(&ctx->walk_control_lock);
        if (ctx->walk_control_obsolete) {
                mutex_unlock(&ctx->walk_control_lock);
                return -ECANCELED;
        }
    ...
}

Even though it requires erroneous API usage, the API intends to gracefully
reject this via the obsolete flag. The missing initialization breaks this
boundary, leading to an unkillable hang.

>   * Return: 0 on success, negative error code otherwise.
>   */
>  int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)

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