All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liew Rui Yan <aethernet65535@gmail.com>
To: sj@kernel.org
Cc: aethernet65535@gmail.com, damon@lists.linux.dev, linux-mm@kvack.org
Subject: Re: [RFC v4] mm/damon: add synchronous commit for commit_inputs
Date: Tue, 24 Mar 2026 02:38:30 +0800	[thread overview]
Message-ID: <20260323183830.42248-1-aethernet65535@gmail.com> (raw)
In-Reply-To: <20260323151611.81358-1-sj@kernel.org>

> > Hi SeongJae,
> > 
> > Follow-up on my previous email regarding the locking concerns.
> > 
> > > I admit I don't have an elegant solution yet. Here are my current
> > > thought:
> > > 
> > > Add a timeout to wait_for_completion() (e.g.,
> > > wait_for_completion_timeout()), using 'damos_watermarks.interval' as the
> > > upper bound. This prevents indefinite blocking of 'param_lock', though
> > > it still holds the global lock for up to several seconds.
> > 
> > After further analysis, I realized that since both 'commit_inputs' and
> > 'enabled' module parameters are protected by the global 'param_lock',
> > stopping kdamond gracefully (via enabled=N) is serialized with
> > 'commit_inputs' writes. Therefore, a classic deadlock scenario should
> > not occur in normal operation.
> > 
> > However, I'm considering edge cases where kdamond might terminate
> > unexpectedly. In such cases, commit_inputs_store() could hold
> > 'param_lock' indefinitely, blocking other module parameter updates
> > system-wide.
> > 
> > To mitigate this risk defensively, would you accept adding a timeout to
> > wait_for_completion()? This ensures 'param_lock' is eventually released
> > even if kdamond fails unexpectedly.
> > 
> > Please let me know your preference. :>
> 
> As I mentioned on the reply to the original mail of this mail, I think such
> infinite wait will not happen because damon_call() is aware of the kdamond
> stop, and therefore the timeout is not needed.  Please double check and let me
> know if I'm missing something, though, as a reply to my reply.

Yes, you are absolutely right. My concern stemmed from a misconception
that kdamond could be forcibly terminated by userspace signals like
'kill -9'.

I just verified this experimentally in virtme-ng, even 'sudo kill -9
<kdamond_pid>' fails to terminate the kdamond thread. This confirms that
kdamond, asa kthread, must exit gracefully via kthread_stop(), ensuring
the completion will always be signaled.

Thank you for guiding me to verify this. I agree that no additional
timeout is needed.

Best regards,
Rui Yan

  reply	other threads:[~2026-03-23 18:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-23  2:16 [RFC v4] mm/damon: add synchronous commit for commit_inputs Liew Rui Yan
2026-03-23  7:27 ` Liew Rui Yan
2026-03-23 14:19   ` Liew Rui Yan
2026-03-23 15:16     ` SeongJae Park
2026-03-23 18:38       ` Liew Rui Yan [this message]
2026-03-23 15:12   ` SeongJae Park
2026-03-23 18:37     ` Liew Rui Yan
2026-03-23 15:05 ` SeongJae Park
2026-03-23 18:37   ` Liew Rui Yan

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=20260323183830.42248-1-aethernet65535@gmail.com \
    --to=aethernet65535@gmail.com \
    --cc=damon@lists.linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=sj@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.