Re: [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:

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.

> 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/[email protected]?part=3