damon.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
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 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
Date: Wed, 12 Aug 2026 21:11:41 -0700	[thread overview]
Message-ID: <20260813041142.98270-1-sj@kernel.org> (raw)
In-Reply-To: <20260813040141.2A72A1F000E9@smtp.kernel.org>

On Thu, 13 Aug 2026 04:01:40 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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.

Because of the param_lock, that race is impossible.

> 
> >  	err = damon_call(ctx, &control);
> >  
> >  	return err ? err : control.return_code;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260813035001.97364-1-sj@kernel.org?part=3


Thanks,
SJ

  reply	other threads:[~2026-08-13  4:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13  3:49 [RFC PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
2026-08-13  3:49 ` [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
2026-08-13  3:58   ` sashiko-bot
2026-08-13  8:33   ` Gutierrez Asier
2026-08-13 13:55     ` SJ Park
2026-08-13 14:27       ` Gutierrez Asier
2026-08-13 15:12         ` SJ Park
2026-08-13  3:49 ` [RFC PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
2026-08-13  4:02   ` sashiko-bot
2026-08-13  4:10     ` SJ Park
2026-08-13  3:49 ` [RFC PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
2026-08-13  4:01   ` sashiko-bot
2026-08-13  4:11     ` SJ Park [this message]
2026-08-13  3:49 ` [RFC PATCH 4/4] mm/damon/lru_sort: " SJ Park
2026-08-13  4:04   ` sashiko-bot
2026-08-13  4:12     ` SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260813041142.98270-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-bot@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).