Linux CIFS filesystem development
 help / color / mirror / Atom feed
* Questions about wake_up[_interruptible][_all]
@ 2025-08-13 20:28 Stefan Metzmacher
  2025-08-13 21:37 ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Metzmacher @ 2025-08-13 20:28 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
	Steve French, Namjae Jeon
  Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org

Hi,

there are several cases where wait queues are used in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/smb/client/smbdirect.c
and
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/smb/server/transport_rdma.c

I'm a bit confused because we the use mixed use of
wake_up(), wake_up_interruptible() and wake_up_interruptible_all()

On the wait side the following are used
wait_event(), wait_event_interruptible() and wait_event_interruptible_timeout()

The documentation of all wait_event_* macros say 'wake_up()' should be used.
And there's no documentation on the various wake_up_* macros.

I guess I understand the difference between
wait_event() and wait_event_interruptible(),
the first ignores any signal even kill and the
2nd returns -ERESTARTSYS on any signal.

I'm wondering if using wait_event_killable() should
be preferred instead of wait_event() in order to prevent
processes in state D hanging forever.

For some wait queues it would be desired that only
a single waiter is woken, so it can make good forward progress,
so maybe some wait_event_*_exclusive() would be useful for this.

As far as I understand the difference between
wake_up() and wake_up_all() is that the first
stops after the first waiter with WQ_FLAG_EXCLUSIVE.
and wake_up_all() wakes all waiters (useful for error conditions,
which all waiters should handle immediately.

But I don't understand the difference between
wake_up() and wake_up_interruptible().
My best guess would be that wake_up_interruptible()
would not wake waiters using wait_event(), but only
waiters using wait_event_interruptible() or any other
version that includes TASK_INTERRUPTIBLE.

So I guess we never want to use wake_up_interruptible(),
but always wake_up() or wake_up_all() instead...

It would be great if the documentation of
the wake_up macros and their interaction with
the wait_event macros could be improved.

Any hints are highly welcome :-)

Thanks!
metze

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Questions about wake_up[_interruptible][_all]
  2025-08-13 20:28 Questions about wake_up[_interruptible][_all] Stefan Metzmacher
@ 2025-08-13 21:37 ` Steven Rostedt
  2025-08-14  7:57   ` Stefan Metzmacher
  0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2025-08-13 21:37 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Ben Segall, Mel Gorman, Valentin Schneider, Steve French,
	Namjae Jeon, linux-cifs@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Wed, 13 Aug 2025 22:28:08 +0200
Stefan Metzmacher <metze@samba.org> wrote:

> I guess I understand the difference between
> wait_event() and wait_event_interruptible(),
> the first ignores any signal even kill and the
> 2nd returns -ERESTARTSYS on any signal.

The main difference is what the code does after the wait_event*().

If you use wait_event_interruptible() the first thing the code should do is
to check if a signal is pending or not. Or at least check some status to
know that what it is waiting for did not happen and handle it properly.

But there's places in the kernel where the task is waiting for something
and it expects that whatever it is waiting for *must* happen eventually and
it should not continue until it does.

Looking at one example: fs/jbd2/journal.c: jbd2_journal_start_thread()

It creates a thread, tests that it is created, and then waits for that
thread to acknowledge that it is running, and the function should not
return until it does.

If someone were to send a signal to that waiter and wake it up prematurely,
the following code may become buggy as it expects the thread to be
initialized and active when it is not.

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Questions about wake_up[_interruptible][_all]
  2025-08-13 21:37 ` Steven Rostedt
@ 2025-08-14  7:57   ` Stefan Metzmacher
  2025-08-15  2:47     ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Metzmacher @ 2025-08-14  7:57 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Ben Segall, Mel Gorman, Valentin Schneider, Steve French,
	Namjae Jeon, linux-cifs@vger.kernel.org,
	linux-kernel@vger.kernel.org

Am 13.08.25 um 23:37 schrieb Steven Rostedt:
> On Wed, 13 Aug 2025 22:28:08 +0200
> Stefan Metzmacher <metze@samba.org> wrote:
> 
>> I guess I understand the difference between
>> wait_event() and wait_event_interruptible(),
>> the first ignores any signal even kill and the
>> 2nd returns -ERESTARTSYS on any signal.
> 
> The main difference is what the code does after the wait_event*().
> 
> If you use wait_event_interruptible() the first thing the code should do is
> to check if a signal is pending or not. Or at least check some status to
> know that what it is waiting for did not happen and handle it properly.
> 
> But there's places in the kernel where the task is waiting for something
> and it expects that whatever it is waiting for *must* happen eventually and
> it should not continue until it does.
> 
> Looking at one example: fs/jbd2/journal.c: jbd2_journal_start_thread()
> 
> It creates a thread, tests that it is created, and then waits for that
> thread to acknowledge that it is running, and the function should not
> return until it does.
> 
> If someone were to send a signal to that waiter and wake it up prematurely,
> the following code may become buggy as it expects the thread to be
> initialized and active when it is not.

Thanks!

Via a private channel I also got this answer:

wake_up_interruptible() only wakes tasks that are in the
TASK_INTERRUPTIBLE state.

wake_up() wakes tasks that are in either the TASK_INTERRUPTIBLE or
TASK_UNINTERRUPTIBLE state, as per the TASK_NORMAL macro used in the
definition of wake_up().

Call chain:

wake_up_interruptible
   __wake_up(mode = TASK_INTERRUPTIBLE)
     __wake_up_common_lock(mode = TASK_INTERRUPTIBLE)
       __wake_up_common(mode = TASK_INTERRUPTIBLE)
         curr->func(mode = TASK_INTERRUPTIBLE)
           // curr->func is usually default_wake_function
           default_wake_function(mode = TASK_INTERRUPTIBLE)
             try_to_wake_up(state = TASK_INTERRUPTIBLE)
               ttwu_state_match(state = TASK_INTERRUPTIBLE)
                 __task_state_match(state = TASK_INTERRUPTIBLE):
                 if (READ_ONCE(p->__state) & state) ...

metze

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Questions about wake_up[_interruptible][_all]
  2025-08-14  7:57   ` Stefan Metzmacher
@ 2025-08-15  2:47     ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2025-08-15  2:47 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Ben Segall, Mel Gorman, Valentin Schneider, Steve French,
	Namjae Jeon, linux-cifs@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Thu, 14 Aug 2025 09:57:14 +0200
Stefan Metzmacher <metze@samba.org> wrote:

> Am 13.08.25 um 23:37 schrieb Steven Rostedt:
> > On Wed, 13 Aug 2025 22:28:08 +0200
> > Stefan Metzmacher <metze@samba.org> wrote:
> >   
> >> I guess I understand the difference between
> >> wait_event() and wait_event_interruptible(),
> >> the first ignores any signal even kill and the
> >> 2nd returns -ERESTARTSYS on any signal.  
> > 
> > The main difference is what the code does after the wait_event*().
> > 
> > If you use wait_event_interruptible() the first thing the code should do is
> > to check if a signal is pending or not. Or at least check some status to
> > know that what it is waiting for did not happen and handle it properly.
> > 
> > But there's places in the kernel where the task is waiting for something
> > and it expects that whatever it is waiting for *must* happen eventually and
> > it should not continue until it does.
> > 
> > Looking at one example: fs/jbd2/journal.c: jbd2_journal_start_thread()
> > 
> > It creates a thread, tests that it is created, and then waits for that
> > thread to acknowledge that it is running, and the function should not
> > return until it does.
> > 
> > If someone were to send a signal to that waiter and wake it up prematurely,
> > the following code may become buggy as it expects the thread to be
> > initialized and active when it is not.  
> 
> Thanks!
> 
> Via a private channel I also got this answer:
> 
> wake_up_interruptible() only wakes tasks that are in the
> TASK_INTERRUPTIBLE state.
> 
> wake_up() wakes tasks that are in either the TASK_INTERRUPTIBLE or
> TASK_UNINTERRUPTIBLE state, as per the TASK_NORMAL macro used in the
> definition of wake_up().
> 
> Call chain:
> 
> wake_up_interruptible
>    __wake_up(mode = TASK_INTERRUPTIBLE)
>      __wake_up_common_lock(mode = TASK_INTERRUPTIBLE)
>        __wake_up_common(mode = TASK_INTERRUPTIBLE)
>          curr->func(mode = TASK_INTERRUPTIBLE)
>            // curr->func is usually default_wake_function
>            default_wake_function(mode = TASK_INTERRUPTIBLE)
>              try_to_wake_up(state = TASK_INTERRUPTIBLE)
>                ttwu_state_match(state = TASK_INTERRUPTIBLE)
>                  __task_state_match(state = TASK_INTERRUPTIBLE):
>                  if (READ_ONCE(p->__state) & state) ...
> 

That's a differentiation in implementation, but I thought your question
was more about what the differentiation in the purpose of the two.

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-15  2:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-13 20:28 Questions about wake_up[_interruptible][_all] Stefan Metzmacher
2025-08-13 21:37 ` Steven Rostedt
2025-08-14  7:57   ` Stefan Metzmacher
2025-08-15  2:47     ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox