Re: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started
[email protected] Sun, 02 Aug 2026 19:08:37 +0000
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] A TOCTOU race between `damon_start()` and `damon_call()` can cause= `damon_call()` to loop indefinitely. -- --- Patch [2]: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx = has not started --- commit 9a410362ed5a4418640b6bf73c4ee49300c9dc16 Author: SJ Park <[email protected]> mm/damon/lru_sort: skip damon_call() if ctx has not started This commit prevents damon_call() from hanging when the DAMON context is initialized but not yet started. It introduces a boolean flag to track whet= her the context has ever successfully started, and skips the call if it has not. > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c > index 7e077084cb03a..7df45f9a0b3ae 100644 > --- a/mm/damon/lru_sort.c > +++ b/mm/damon/lru_sort.c [ ... ] > @@ -364,11 +366,8 @@ static int damon_lru_sort_commit_inputs_store(const = char *val, > if (!commit_inputs_request) > return 0; > =20 > - /* > - * Skip damon_call() if ctx is not initialized to avoid > - * NULL pointer dereference. > - */ > - if (!ctx) > + /* Skip damon_call() if ctx has not successfully started. */ > + if (!damon_lru_sort_damon_has_started) > return -EINVAL; > =20 > err =3D damon_call(ctx, &control); [Severity: High] This is a pre-existing issue, but could there be a TOCTOU race here between damon_start() and damon_call() that causes damon_call() to loop indefinitel= y? If a user starts DAMON (setting damon_lru_sort_damon_has_started =3D true), stops it (leaving call_controls_obsolete =3D true), and then concurrently starts DAMON again while calling damon_call() via commit_inputs, can damon_call() get stuck? Looking at mm/damon/core.c, __damon_start() sets ctx->kdamond before the new thread clears ctx->call_controls_obsolete: mm/damon/core.c:__damon_start() { ... ctx->kdamond =3D kthread_run(kdamond_fn, ctx, "kdamond.%d", nr_running_= ctxs); ... } If damon_call() runs concurrently, it checks call_controls_obsolete without holding kdamond_lock, sees the stale true value, and jumps to the canceled label: mm/damon/core.c:damon_call() { ... if (ctx->call_controls_obsolete) { ... goto canceled; } ... canceled: while (damon_is_running(ctx)) schedule_timeout_idle(msecs_to_jiffies(100)); return -ECANCELED; } Since damon_is_running() returns true (because ctx->kdamond is set), and the new healthy thread will run indefinitely, will this task loop forever in TASK_IDLE? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802185430.9940= [email protected]?part=3D2