From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>,
	damon@lists.linux.dev
Subject: Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
Date: Wed, 12 Aug 2026 21:12:57 -0700
Message-ID: <20260813041258.98406-1-sj@kernel.org>
In-Reply-To: <20260813040435.C13F21F000E9@smtp.kernel.org>
References: 
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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