Re: [RFC PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
SJ Park <[email protected]>
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 13 Aug 2026 04:04:35 +0000 [email protected] 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 <[email protected]> > > 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/[email protected]?part=4 Thanks, SJ