* [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required
@ 2015-10-22 8:01 Kosuke Tatsukawa
2015-10-22 12:56 ` Peter Zijlstra
0 siblings, 1 reply; 12+ messages in thread
From: Kosuke Tatsukawa @ 2015-10-22 8:01 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
This patch adds a comment before waitqueue_active noting that memory
barriers are required.
In the following code, the wake_up thread might fail to wake up the
waiting thread and leave it sleeping due to lack of memory barriers.
wake_up thread waiting thread
------------------------------------------------------------------------
CONDITION = 1; add_wait_queue(wq, &wait);
if (waitqueue_active(wq)) for (;;) {
wake_up(wq); if (CONDITION)
break;
wait_woken(&wait, ...);
}
------------------------------------------------------------------------
There are two problems that can occur.
First, on the wake_up thread side, the CPU can reorder waitqueue_active
to happen before the store.
wake_up thread waiting thread
(reordered)
------------------------------------------------------------------------
if (waitqueue_active(wq))
add_wait_queue(wq, &wait);
for (;;) {
if (CONDITION)
break;
CONDITION = 1;
wait_woken(&wait, ...);
}
------------------------------------------------------------------------
Second, on the waiting thread side, the CPU can reorder the load of
CONDITION to occur during add_wait_queue active, before the entry is
added to the wait queue.
wake_up thread waiting thread
(reordered)
------------------------------------------------------------------------
spin_lock_irqsave(...) <add_wait_queue>
if (CONDITION)
CONDITION = 1;
if (waitqueue_active(wq))
__add_wait_queue(...) <add_wait_queue>
spin_unlock_irqrestore(...) <add_wait_queue>
wait_woken(&wait, ...);
------------------------------------------------------------------------
Both problems can be fixed by removing the waitqueue_active() call at
the cost of calling spin_lock and spin_unlock even when the queue is
empty.
However, if that is too expensive, the reordering could be prevented by
adding memory barriers in the following places.
wake_up thread waiting thread
------------------------------------------------------------------------
CONDITION = 1; add_wait_queue(wq, &wait);
smp_mb(); smp_mb();
if (waitqueue_active(wq)) for (;;) {
wake_up(wq); if (CONDITION)
break;
wait_woken(&wait, ...);
}
------------------------------------------------------------------------
If the waiting thread uses prepare_to_wait() or wait_event*() instead of
directly calling add_wait_queue(), set_current_state() called within
those functions contains the necessary memory barrier. The memory
barrier in the wake_up thread is still needed.
There were several places in the linux kernel source code which lacked
the memory barrier. Hopefully, the comment will make people using
waitqueue_active a little more cautious.
Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
---
include/linux/wait.h | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1e1bf9f..4a4c6fc 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -102,6 +102,19 @@ init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func)
q->func = func;
}
+/*
+ * Note: When adding waitqueue_active before calling wake_up for
+ * optimization, some sort of memory barrier is required on SMP so
+ * that the waiting thread does not miss the wake up.
+ *
+ * A memory barrier is required before waitqueue_active to prevent
+ * waitqueue_active from being reordered by the CPU before any writes
+ * done prior to it.
+ *
+ * The waiting side also needs a memory barrier which pairs with the
+ * wake_up side. If prepare_to_wait() or wait_event*() is used, they
+ * contain the memory barrier in set_current_state().
+ */
static inline int waitqueue_active(wait_queue_head_t *q)
{
return !list_empty(&q->task_list);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required
2015-10-22 8:01 [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required Kosuke Tatsukawa
@ 2015-10-22 12:56 ` Peter Zijlstra
2015-10-22 23:18 ` Kosuke Tatsukawa
0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2015-10-22 12:56 UTC (permalink / raw)
To: Kosuke Tatsukawa; +Cc: Ingo Molnar, linux-kernel
On Thu, Oct 22, 2015 at 08:01:37AM +0000, Kosuke Tatsukawa wrote:
Its somewhat unfortunate you chose the whole wait_woken() thing, its
'rare'.
> Second, on the waiting thread side, the CPU can reorder the load of
> CONDITION to occur during add_wait_queue active, before the entry is
> added to the wait queue.
> wake_up thread waiting thread
> (reordered)
> ------------------------------------------------------------------------
> spin_lock_irqsave(...) <add_wait_queue>
> if (CONDITION)
> CONDITION = 1;
> if (waitqueue_active(wq))
wake_up();
> __add_wait_queue(...) <add_wait_queue>
> spin_unlock_irqrestore(...) <add_wait_queue>
> wait_woken(&wait, ...);
> ------------------------------------------------------------------------
This isn't actually a problem IIRC, because wait_woken() will test
WQ_FLAG_WOKEN and not actually sleep.
> However, if that is too expensive, the reordering could be prevented by
> adding memory barriers in the following places.
> wake_up thread waiting thread
> ------------------------------------------------------------------------
> CONDITION = 1; add_wait_queue(wq, &wait);
> smp_mb(); smp_mb();
> if (waitqueue_active(wq)) for (;;) {
> wake_up(wq); if (CONDITION)
> break;
> wait_woken(&wait, ...);
> }
So for wait_woken, WQ_FLAG_WOKEN should 'fix' that, and for pretty much
anything else you must have a set_current_state() before testing
CONDITION and you're good (as you state elsewhere).
> +++ b/include/linux/wait.h
> @@ -102,6 +102,19 @@ init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func)
> q->func = func;
> }
>
> +/*
> + * Note: When adding waitqueue_active before calling wake_up for
> + * optimization, some sort of memory barrier is required on SMP so
> + * that the waiting thread does not miss the wake up.
> + *
> + * A memory barrier is required before waitqueue_active to prevent
> + * waitqueue_active from being reordered by the CPU before any writes
> + * done prior to it.
> + *
> + * The waiting side also needs a memory barrier which pairs with the
> + * wake_up side. If prepare_to_wait() or wait_event*() is used, they
> + * contain the memory barrier in set_current_state().
> + */
> static inline int waitqueue_active(wait_queue_head_t *q)
> {
> return !list_empty(&q->task_list);
How about something like:
/**
* waitqueue_active -- locklessly test for waiters on the queue
* @q: the waitqueue to test for waiters
*
* returns true if the wait list is not empty
*
* NOTE: this function is lockless and requires care, incorrect usage
* _will_ lead to sporadic and non-obvious failure.
*
* Use either while holding wait_queue_head_t::lock or when used for
* wakeups with an extra smp_mb() like:
*
* CPU0 - waker CPU1 - waiter
*
* for (;;) {
* @cond = true; prepare_to_wait(&wq, &wait, state);
* smp_mb(); /* smp_mb() from set_current_state() */
* if (waitqueue_active(wq)) if (@cond)
* wake_up(wq); break;
* schedule();
* }
*
* Because without the explicit smp_mb() its possible for the
* waitqueue_active() load to get hoisted over the @cond store such that
* we'll observe an empty wait list while the waiter might not observe
* @cond.
*
* Also note that this 'optimization' trades a spin_lock() for an
* smp_mb(), which (when the lock is uncontended) are of roughly equal
* cost.
*/
Does that work for you?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required
2015-10-22 12:56 ` Peter Zijlstra
@ 2015-10-22 23:18 ` Kosuke Tatsukawa
2015-10-23 12:40 ` Peter Zijlstra
0 siblings, 1 reply; 12+ messages in thread
From: Kosuke Tatsukawa @ 2015-10-22 23:18 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, linux-kernel
Peter Zijlstra wrote:
> On Thu, Oct 22, 2015 at 08:01:37AM +0000, Kosuke Tatsukawa wrote:
>
> Its somewhat unfortunate you chose the whole wait_woken() thing, its
> 'rare'.
Yes. I first noticed this lack of memory barrier before
waitqueue_active() issue in drivers/tty/n_tty.c which was using
wait_woken(). However, other places were mostly using prepare_to_wait()
or wait_event*(), so wait_woken() is 'rare'.
>> Second, on the waiting thread side, the CPU can reorder the load of
>> CONDITION to occur during add_wait_queue active, before the entry is
>> added to the wait queue.
>> wake_up thread waiting thread
>> (reordered)
>> ------------------------------------------------------------------------
>> spin_lock_irqsave(...) <add_wait_queue>
>> if (CONDITION)
>> CONDITION = 1;
>> if (waitqueue_active(wq))
> wake_up();
>> __add_wait_queue(...) <add_wait_queue>
>> spin_unlock_irqrestore(...) <add_wait_queue>
>> wait_woken(&wait, ...);
>> ------------------------------------------------------------------------
>
> This isn't actually a problem IIRC, because wait_woken() will test
> WQ_FLAG_WOKEN and not actually sleep.
In the above figure, waitqueue_active(wq) will return 0 (queue is
inactive) and skip the whole wake_up() call, because __add_wait_queue()
hasn't been called yet. This actually does occur using a reproducer.
>> However, if that is too expensive, the reordering could be prevented by
>> adding memory barriers in the following places.
>> wake_up thread waiting thread
>> ------------------------------------------------------------------------
>> CONDITION = 1; add_wait_queue(wq, &wait);
>> smp_mb(); smp_mb();
>> if (waitqueue_active(wq)) for (;;) {
>> wake_up(wq); if (CONDITION)
>> break;
>> wait_woken(&wait, ...);
>> }
>
> So for wait_woken, WQ_FLAG_WOKEN should 'fix' that, and for pretty much
> anything else you must have a set_current_state() before testing
> CONDITION and you're good (as you state elsewhere).
wait_woken() calls set_current_state(), but that is after the CONDITION
test.
>> +++ b/include/linux/wait.h
>> @@ -102,6 +102,19 @@ init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func)
>> q->func = func;
>> }
>>
>> +/*
>> + * Note: When adding waitqueue_active before calling wake_up for
>> + * optimization, some sort of memory barrier is required on SMP so
>> + * that the waiting thread does not miss the wake up.
>> + *
>> + * A memory barrier is required before waitqueue_active to prevent
>> + * waitqueue_active from being reordered by the CPU before any writes
>> + * done prior to it.
>> + *
>> + * The waiting side also needs a memory barrier which pairs with the
>> + * wake_up side. If prepare_to_wait() or wait_event*() is used, they
>> + * contain the memory barrier in set_current_state().
>> + */
>> static inline int waitqueue_active(wait_queue_head_t *q)
>> {
>> return !list_empty(&q->task_list);
>
> How about something like:
>
> /**
> * waitqueue_active -- locklessly test for waiters on the queue
> * @q: the waitqueue to test for waiters
> *
> * returns true if the wait list is not empty
> *
> * NOTE: this function is lockless and requires care, incorrect usage
> * _will_ lead to sporadic and non-obvious failure.
> *
> * Use either while holding wait_queue_head_t::lock or when used for
> * wakeups with an extra smp_mb() like:
> *
> * CPU0 - waker CPU1 - waiter
> *
> * for (;;) {
> * @cond = true; prepare_to_wait(&wq, &wait, state);
> * smp_mb(); /* smp_mb() from set_current_state() */
> * if (waitqueue_active(wq)) if (@cond)
> * wake_up(wq); break;
> * schedule();
> * }
> *
> * Because without the explicit smp_mb() its possible for the
> * waitqueue_active() load to get hoisted over the @cond store such that
> * we'll observe an empty wait list while the waiter might not observe
> * @cond.
> *
> * Also note that this 'optimization' trades a spin_lock() for an
> * smp_mb(), which (when the lock is uncontended) are of roughly equal
> * cost.
> */
>
> Does that work for you?
Yes. Considering that the use of wait_woken is pretty rare, I think the
explanation is more focused and easier to understand this way.
Best regards.
---
Kosuke TATSUKAWA | 3rd IT Platform Department
| IT Platform Division, NEC Corporation
| tatsu@ab.jp.nec.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required
2015-10-22 23:18 ` Kosuke Tatsukawa
@ 2015-10-23 12:40 ` Peter Zijlstra
2015-11-11 9:48 ` Herbert Xu
0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2015-10-23 12:40 UTC (permalink / raw)
To: Kosuke Tatsukawa; +Cc: Ingo Molnar, linux-kernel
On Thu, Oct 22, 2015 at 11:18:33PM +0000, Kosuke Tatsukawa wrote:
> Peter Zijlstra wrote:
> > On Thu, Oct 22, 2015 at 08:01:37AM +0000, Kosuke Tatsukawa wrote:
> >
> > Its somewhat unfortunate you chose the whole wait_woken() thing, its
> > 'rare'.
>
> Yes. I first noticed this lack of memory barrier before
> waitqueue_active() issue in drivers/tty/n_tty.c which was using
> wait_woken(). However, other places were mostly using prepare_to_wait()
> or wait_event*(), so wait_woken() is 'rare'.
Which I no doubt introduced there (the wait_woken thing), and it would
have been nice if I'd been Cc to that discussion.
In any case, I found the patch in next and dropping the
waitqueue_active() think is in deed the sane solution. It will serialize
everything on the queue lock.
> >> Second, on the waiting thread side, the CPU can reorder the load of
> >> CONDITION to occur during add_wait_queue active, before the entry is
> >> added to the wait queue.
> >> wake_up thread waiting thread
> >> (reordered)
> >> ------------------------------------------------------------------------
> >> spin_lock_irqsave(...) <add_wait_queue>
> >> if (CONDITION)
> >> CONDITION = 1;
> >> if (waitqueue_active(wq))
> > wake_up();
> >> __add_wait_queue(...) <add_wait_queue>
> >> spin_unlock_irqrestore(...) <add_wait_queue>
> >> wait_woken(&wait, ...);
> >> ------------------------------------------------------------------------
> >
> > This isn't actually a problem IIRC, because wait_woken() will test
> > WQ_FLAG_WOKEN and not actually sleep.
>
> In the above figure, waitqueue_active(wq) will return 0 (queue is
> inactive) and skip the whole wake_up() call, because __add_wait_queue()
> hasn't been called yet. This actually does occur using a reproducer.
Duh, indeed.
> > Does that work for you?
>
> Yes. Considering that the use of wait_woken is pretty rare, I think the
> explanation is more focused and easier to understand this way.
OK, thanks, I'll queue the below.
---
Subject: sched, wait: Document waitqueue_active
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri Oct 23 14:32:34 CEST 2015
Kosuku reports that there were a fair number of buggy
waitqueue_active() users and this function deserves a big comment in
order to avoid growing more.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/wait.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -102,6 +102,36 @@ init_waitqueue_func_entry(wait_queue_t *
q->func = func;
}
+/**
+ * waitqueue_active -- locklessly test for waiters on the queue
+ * @q: the waitqueue to test for waiters
+ *
+ * returns true if the wait list is not empty
+ *
+ * NOTE: this function is lockless and requires care, incorrect usage _will_
+ * lead to sporadic and non-obvious failure.
+ *
+ * Use either while holding wait_queue_head_t::lock or when used for wakeups
+ * with an extra smp_mb() like:
+ *
+ * CPU0 - waker CPU1 - waiter
+ *
+ * for (;;) {
+ * @cond = true; prepare_to_wait(&wq, &wait, state);
+ * smp_mb(); // smp_mb() from set_current_state()
+ * if (waitqueue_active(wq)) if (@cond)
+ * wake_up(wq); break;
+ * schedule();
+ * }
+ * finish_wait(&wq, &wait);
+ *
+ * Because without the explicit smp_mb() it's possible for the
+ * waitqueue_active() load to get hoisted over the @cond store such that we'll
+ * observe an empty wait list while the waiter might not observe @cond.
+ *
+ * Also note that this 'optimization' trades a spin_lock() for an smp_mb(),
+ * which (when the lock is uncontended) are of roughly equal cost.
+ */
static inline int waitqueue_active(wait_queue_head_t *q)
{
return !list_empty(&q->task_list);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required
2015-10-23 12:40 ` Peter Zijlstra
@ 2015-11-11 9:48 ` Herbert Xu
2015-11-24 5:54 ` net: Generalise wq_has_sleeper helper Herbert Xu
0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2015-11-11 9:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: tatsu, mingo, linux-kernel, David S. Miller, netdev, Jiri Olsa,
Eric Dumazet
Peter Zijlstra <peterz@infradead.org> wrote:
>
>> >> Second, on the waiting thread side, the CPU can reorder the load of
>> >> CONDITION to occur during add_wait_queue active, before the entry is
>> >> added to the wait queue.
>> >> wake_up thread waiting thread
>> >> (reordered)
>> >> ------------------------------------------------------------------------
>> >> spin_lock_irqsave(...) <add_wait_queue>
>> >> if (CONDITION)
>> >> CONDITION = 1;
>> >> if (waitqueue_active(wq))
>> > wake_up();
>> >> __add_wait_queue(...) <add_wait_queue>
>> >> spin_unlock_irqrestore(...) <add_wait_queue>
>> >> wait_woken(&wait, ...);
>> >> ------------------------------------------------------------------------
>> >
>> > This isn't actually a problem IIRC, because wait_woken() will test
>> > WQ_FLAG_WOKEN and not actually sleep.
>>
>> In the above figure, waitqueue_active(wq) will return 0 (queue is
>> inactive) and skip the whole wake_up() call, because __add_wait_queue()
>> hasn't been called yet. This actually does occur using a reproducer.
>
> Duh, indeed.
BTW, the networking folks found this years ago and even added
helpers to deal with this. See for example wq_has_sleeper in
include/net/sock.h. It would be good if we can move some of
those helpers into wait.h instead.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 12+ messages in thread
* net: Generalise wq_has_sleeper helper
2015-11-11 9:48 ` Herbert Xu
@ 2015-11-24 5:54 ` Herbert Xu
2015-11-24 21:30 ` David Miller
2015-11-25 9:15 ` Peter Zijlstra
0 siblings, 2 replies; 12+ messages in thread
From: Herbert Xu @ 2015-11-24 5:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: tatsu, mingo, linux-kernel, David S. Miller, netdev, Jiri Olsa,
Eric Dumazet
On Wed, Nov 11, 2015 at 05:48:29PM +0800, Herbert Xu wrote:
>
> BTW, the networking folks found this years ago and even added
> helpers to deal with this. See for example wq_has_sleeper in
> include/net/sock.h. It would be good if we can move some of
> those helpers into wait.h instead.
Here is a patch against net-next which makes the wq_has_sleeper
helper available to non-next users:
---8<---
The memory barrier in the helper wq_has_sleeper is needed by just
about every user of waitqueue_active. This patch generalises it
by making it take a wait_queue_head_t directly. The existing
helper is renamed to skwq_has_sleeper.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 0aa6fdf..fb99f30 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -106,7 +106,7 @@ static void aead_wmem_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM |
POLLRDBAND);
@@ -157,7 +157,7 @@ static void aead_data_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLRDNORM |
POLLRDBAND);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index af31a0e..0e6702e 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -238,7 +238,7 @@ static void skcipher_wmem_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM |
POLLRDBAND);
@@ -288,7 +288,7 @@ static void skcipher_data_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLRDNORM |
POLLRDBAND);
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1e1bf9f..bd1157f 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -107,6 +107,50 @@ static inline int waitqueue_active(wait_queue_head_t *q)
return !list_empty(&q->task_list);
}
+/**
+ * wq_has_sleeper - check if there are any waiting processes
+ * @wq: wait queue head
+ *
+ * Returns true if wq has waiting processes
+ *
+ * The purpose of the wq_has_sleeper and sock_poll_wait is to wrap the memory
+ * barrier call. They were added due to the race found within the tcp code.
+ *
+ * Consider following tcp code paths:
+ *
+ * CPU1 CPU2
+ *
+ * sys_select receive packet
+ * ... ...
+ * __add_wait_queue update tp->rcv_nxt
+ * ... ...
+ * tp->rcv_nxt check sock_def_readable
+ * ... {
+ * schedule rcu_read_lock();
+ * wq = rcu_dereference(sk->sk_wq);
+ * if (wq && waitqueue_active(&wq->wait))
+ * wake_up_interruptible(&wq->wait)
+ * ...
+ * }
+ *
+ * The race for tcp fires when the __add_wait_queue changes done by CPU1 stay
+ * in its cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1
+ * could then endup calling schedule and sleep forever if there are no more
+ * data on the socket.
+ *
+ */
+static inline bool wq_has_sleeper(wait_queue_head_t *wq)
+{
+ /* We need to be sure we are in sync with the
+ * add_wait_queue modifications to the wait queue.
+ *
+ * This memory barrier should be paired with one on the
+ * waiting side.
+ */
+ smp_mb();
+ return waitqueue_active(wq);
+}
+
extern void add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
extern void add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait);
extern void remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
diff --git a/include/net/sock.h b/include/net/sock.h
index bbf7c2c..4a6e9b6 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -58,6 +58,7 @@
#include <linux/memcontrol.h>
#include <linux/static_key.h>
#include <linux/sched.h>
+#include <linux/wait.h>
#include <linux/filter.h>
#include <linux/rculist_nulls.h>
@@ -1879,46 +1880,14 @@ static inline bool sk_has_allocations(const struct sock *sk)
}
/**
- * wq_has_sleeper - check if there are any waiting processes
+ * skwq_has_sleeper - check if there are any waiting processes
* @wq: struct socket_wq
*
* Returns true if socket_wq has waiting processes
- *
- * The purpose of the wq_has_sleeper and sock_poll_wait is to wrap the memory
- * barrier call. They were added due to the race found within the tcp code.
- *
- * Consider following tcp code paths:
- *
- * CPU1 CPU2
- *
- * sys_select receive packet
- * ... ...
- * __add_wait_queue update tp->rcv_nxt
- * ... ...
- * tp->rcv_nxt check sock_def_readable
- * ... {
- * schedule rcu_read_lock();
- * wq = rcu_dereference(sk->sk_wq);
- * if (wq && waitqueue_active(&wq->wait))
- * wake_up_interruptible(&wq->wait)
- * ...
- * }
- *
- * The race for tcp fires when the __add_wait_queue changes done by CPU1 stay
- * in its cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1
- * could then endup calling schedule and sleep forever if there are no more
- * data on the socket.
- *
*/
-static inline bool wq_has_sleeper(struct socket_wq *wq)
+static inline bool skwq_has_sleeper(struct socket_wq *wq)
{
- /* We need to be sure we are in sync with the
- * add_wait_queue modifications to the wait queue.
- *
- * This memory barrier is paired in the sock_poll_wait.
- */
- smp_mb();
- return wq && waitqueue_active(&wq->wait);
+ return wq && wq_has_sleeper(&wq->wait);
}
/**
diff --git a/net/atm/common.c b/net/atm/common.c
index 49a872d..6dc1230 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -96,7 +96,7 @@ static void vcc_def_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up(&wq->wait);
rcu_read_unlock();
}
@@ -117,7 +117,7 @@ static void vcc_write_space(struct sock *sk)
if (vcc_writable(sk)) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
diff --git a/net/core/sock.c b/net/core/sock.c
index 1e4dd54..2769bd3a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2283,7 +2283,7 @@ static void sock_def_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_all(&wq->wait);
rcu_read_unlock();
}
@@ -2294,7 +2294,7 @@ static void sock_def_error_report(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_poll(&wq->wait, POLLERR);
sk_wake_async(sk, SOCK_WAKE_IO, POLL_ERR);
rcu_read_unlock();
@@ -2306,7 +2306,7 @@ static void sock_def_readable(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLPRI |
POLLRDNORM | POLLRDBAND);
sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
@@ -2324,7 +2324,7 @@ static void sock_def_write_space(struct sock *sk)
*/
if ((atomic_read(&sk->sk_wmem_alloc) << 1) <= sk->sk_sndbuf) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
diff --git a/net/core/stream.c b/net/core/stream.c
index d70f77a..8ff9d63 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -35,7 +35,7 @@ void sk_stream_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
if (wq && wq->fasync_list && !(sk->sk_shutdown & SEND_SHUTDOWN))
diff --git a/net/dccp/output.c b/net/dccp/output.c
index 4ce912e..b66c84d 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -201,7 +201,7 @@ void dccp_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
/* Should agree with poll, otherwise some programs break */
if (sock_writeable(sk))
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fcb2752..4f0aa91 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -303,7 +303,7 @@ static void iucv_sock_wake_msglim(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_all(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
rcu_read_unlock();
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 1f8a144..7e2d105 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -67,7 +67,7 @@ static void rxrpc_write_space(struct sock *sk)
if (rxrpc_writable(sk)) {
struct socket_wq *wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 897c01c..ec10b66 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6978,7 +6978,7 @@ void sctp_data_ready(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM | POLLRDBAND);
sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 552dbab..525acf6 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1492,7 +1492,7 @@ static void tipc_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
rcu_read_unlock();
@@ -1509,7 +1509,7 @@ static void tipc_data_ready(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM | POLLRDBAND);
rcu_read_unlock();
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index aaa0b58..0446ff1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -339,7 +339,7 @@ static void unix_write_space(struct sock *sk)
rcu_read_lock();
if (unix_writable(sk)) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait,
POLLOUT | POLLWRNORM | POLLWRBAND);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: net: Generalise wq_has_sleeper helper
2015-11-24 5:54 ` net: Generalise wq_has_sleeper helper Herbert Xu
@ 2015-11-24 21:30 ` David Miller
2015-11-25 1:10 ` Herbert Xu
2015-11-25 9:15 ` Peter Zijlstra
1 sibling, 1 reply; 12+ messages in thread
From: David Miller @ 2015-11-24 21:30 UTC (permalink / raw)
To: herbert; +Cc: peterz, tatsu, mingo, linux-kernel, netdev, jolsa, eric.dumazet
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 24 Nov 2015 13:54:23 +0800
> On Wed, Nov 11, 2015 at 05:48:29PM +0800, Herbert Xu wrote:
>>
>> BTW, the networking folks found this years ago and even added
>> helpers to deal with this. See for example wq_has_sleeper in
>> include/net/sock.h. It would be good if we can move some of
>> those helpers into wait.h instead.
>
> Here is a patch against net-next which makes the wq_has_sleeper
> helper available to non-next users:
>
> ---8<---
> The memory barrier in the helper wq_has_sleeper is needed by just
> about every user of waitqueue_active. This patch generalises it
> by making it take a wait_queue_head_t directly. The existing
> helper is renamed to skwq_has_sleeper.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
I'm fine with wherever this patch goes. Herbert is there any
particular tree where it'll facilitate another user quickest?
Or should I just toss it into net-next?
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: net: Generalise wq_has_sleeper helper
2015-11-24 21:30 ` David Miller
@ 2015-11-25 1:10 ` Herbert Xu
0 siblings, 0 replies; 12+ messages in thread
From: Herbert Xu @ 2015-11-25 1:10 UTC (permalink / raw)
To: David Miller
Cc: peterz, tatsu, mingo, linux-kernel, netdev, jolsa, eric.dumazet
On Tue, Nov 24, 2015 at 04:30:25PM -0500, David Miller wrote:
>
> I'm fine with wherever this patch goes. Herbert is there any
> particular tree where it'll facilitate another user quickest?
>
> Or should I just toss it into net-next?
>
> Acked-by: David S. Miller <davem@davemloft.net>
No Dave net-next is fine I think. This was prompted by Tatsukawa-san's
patches to fix waitqueue users affected by this very race and they
were all over the tree.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: net: Generalise wq_has_sleeper helper
2015-11-24 5:54 ` net: Generalise wq_has_sleeper helper Herbert Xu
2015-11-24 21:30 ` David Miller
@ 2015-11-25 9:15 ` Peter Zijlstra
2015-11-25 16:37 ` David Miller
1 sibling, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2015-11-25 9:15 UTC (permalink / raw)
To: Herbert Xu
Cc: tatsu, mingo, linux-kernel, David S. Miller, netdev, Jiri Olsa,
Eric Dumazet
On Tue, Nov 24, 2015 at 01:54:23PM +0800, Herbert Xu wrote:
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index 1e1bf9f..bd1157f 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -107,6 +107,50 @@ static inline int waitqueue_active(wait_queue_head_t *q)
> return !list_empty(&q->task_list);
> }
>
> +/**
> + * wq_has_sleeper - check if there are any waiting processes
> + * @wq: wait queue head
> + *
> + * Returns true if wq has waiting processes
> + *
> + * The purpose of the wq_has_sleeper and sock_poll_wait is to wrap the memory
> + * barrier call. They were added due to the race found within the tcp code.
> + *
> + * Consider following tcp code paths:
> + *
> + * CPU1 CPU2
> + *
> + * sys_select receive packet
> + * ... ...
> + * __add_wait_queue update tp->rcv_nxt
> + * ... ...
> + * tp->rcv_nxt check sock_def_readable
> + * ... {
> + * schedule rcu_read_lock();
> + * wq = rcu_dereference(sk->sk_wq);
> + * if (wq && waitqueue_active(&wq->wait))
> + * wake_up_interruptible(&wq->wait)
> + * ...
> + * }
> + *
> + * The race for tcp fires when the __add_wait_queue changes done by CPU1 stay
> + * in its cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1
> + * could then endup calling schedule and sleep forever if there are no more
> + * data on the socket.
> + *
Would be easier to refer to the comment that now adorns
waitqueue_active().
> + */
> +static inline bool wq_has_sleeper(wait_queue_head_t *wq)
> +{
> + /* We need to be sure we are in sync with the
broken comment style.
> + * add_wait_queue modifications to the wait queue.
> + *
> + * This memory barrier should be paired with one on the
> + * waiting side.
> + */
> + smp_mb();
> + return waitqueue_active(wq);
> +}
> +
> extern void add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
> extern void add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait);
> extern void remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: net: Generalise wq_has_sleeper helper
2015-11-25 9:15 ` Peter Zijlstra
@ 2015-11-25 16:37 ` David Miller
2015-11-26 5:55 ` [PATCH v2] " Herbert Xu
0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2015-11-25 16:37 UTC (permalink / raw)
To: peterz; +Cc: herbert, tatsu, mingo, linux-kernel, netdev, jolsa, eric.dumazet
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed, 25 Nov 2015 10:15:33 +0100
> On Tue, Nov 24, 2015 at 01:54:23PM +0800, Herbert Xu wrote:
>> + * The race for tcp fires when the __add_wait_queue changes done by CPU1 stay
>> + * in its cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1
>> + * could then endup calling schedule and sleep forever if there are no more
>> + * data on the socket.
>> + *
>
> Would be easier to refer to the comment that now adorns
> waitqueue_active().
Yeah, that might be a good idea. Herbert can you adjust this?
>> + */
>> +static inline bool wq_has_sleeper(wait_queue_head_t *wq)
>> +{
>> + /* We need to be sure we are in sync with the
>
> broken comment style.
This is how we do it in the networking, so that's why it's formatted
this way, but yes he will need to fix it up.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] net: Generalise wq_has_sleeper helper
2015-11-25 16:37 ` David Miller
@ 2015-11-26 5:55 ` Herbert Xu
2015-11-30 19:46 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2015-11-26 5:55 UTC (permalink / raw)
To: David Miller
Cc: peterz, tatsu, mingo, linux-kernel, netdev, jolsa, eric.dumazet
On Wed, Nov 25, 2015 at 11:37:29AM -0500, David Miller wrote:
>
> > Would be easier to refer to the comment that now adorns
> > waitqueue_active().
>
> Yeah, that might be a good idea. Herbert can you adjust this?
Sure, here is an updated patch. Note that this patch is based
on net-next where there is actually no comment above waitqueue_active.
But hopefully this will right itself once the trees are merged.
Thanks,
---8<---
The memory barrier in the helper wq_has_sleeper is needed by just
about every user of waitqueue_active. This patch generalises it
by making it take a wait_queue_head_t directly. The existing
helper is renamed to skwq_has_sleeper.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 0aa6fdf..fb99f30 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -106,7 +106,7 @@ static void aead_wmem_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM |
POLLRDBAND);
@@ -157,7 +157,7 @@ static void aead_data_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLRDNORM |
POLLRDBAND);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index af31a0e..0e6702e 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -238,7 +238,7 @@ static void skcipher_wmem_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM |
POLLRDBAND);
@@ -288,7 +288,7 @@ static void skcipher_data_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLRDNORM |
POLLRDBAND);
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1e1bf9f..6aa09a8 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -107,6 +107,27 @@ static inline int waitqueue_active(wait_queue_head_t *q)
return !list_empty(&q->task_list);
}
+/**
+ * wq_has_sleeper - check if there are any waiting processes
+ * @wq: wait queue head
+ *
+ * Returns true if wq has waiting processes
+ *
+ * Please refer to the comment for waitqueue_active.
+ */
+static inline bool wq_has_sleeper(wait_queue_head_t *wq)
+{
+ /*
+ * We need to be sure we are in sync with the
+ * add_wait_queue modifications to the wait queue.
+ *
+ * This memory barrier should be paired with one on the
+ * waiting side.
+ */
+ smp_mb();
+ return waitqueue_active(wq);
+}
+
extern void add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
extern void add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait);
extern void remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
diff --git a/include/net/sock.h b/include/net/sock.h
index bbf7c2c..4adbe00 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -58,6 +58,7 @@
#include <linux/memcontrol.h>
#include <linux/static_key.h>
#include <linux/sched.h>
+#include <linux/wait.h>
#include <linux/filter.h>
#include <linux/rculist_nulls.h>
@@ -1879,12 +1880,12 @@ static inline bool sk_has_allocations(const struct sock *sk)
}
/**
- * wq_has_sleeper - check if there are any waiting processes
+ * skwq_has_sleeper - check if there are any waiting processes
* @wq: struct socket_wq
*
* Returns true if socket_wq has waiting processes
*
- * The purpose of the wq_has_sleeper and sock_poll_wait is to wrap the memory
+ * The purpose of the skwq_has_sleeper and sock_poll_wait is to wrap the memory
* barrier call. They were added due to the race found within the tcp code.
*
* Consider following tcp code paths:
@@ -1910,15 +1911,9 @@ static inline bool sk_has_allocations(const struct sock *sk)
* data on the socket.
*
*/
-static inline bool wq_has_sleeper(struct socket_wq *wq)
+static inline bool skwq_has_sleeper(struct socket_wq *wq)
{
- /* We need to be sure we are in sync with the
- * add_wait_queue modifications to the wait queue.
- *
- * This memory barrier is paired in the sock_poll_wait.
- */
- smp_mb();
- return wq && waitqueue_active(&wq->wait);
+ return wq && wq_has_sleeper(&wq->wait);
}
/**
diff --git a/net/atm/common.c b/net/atm/common.c
index 49a872d..6dc1230 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -96,7 +96,7 @@ static void vcc_def_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up(&wq->wait);
rcu_read_unlock();
}
@@ -117,7 +117,7 @@ static void vcc_write_space(struct sock *sk)
if (vcc_writable(sk)) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
diff --git a/net/core/sock.c b/net/core/sock.c
index 1e4dd54..2769bd3a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2283,7 +2283,7 @@ static void sock_def_wakeup(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_all(&wq->wait);
rcu_read_unlock();
}
@@ -2294,7 +2294,7 @@ static void sock_def_error_report(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_poll(&wq->wait, POLLERR);
sk_wake_async(sk, SOCK_WAKE_IO, POLL_ERR);
rcu_read_unlock();
@@ -2306,7 +2306,7 @@ static void sock_def_readable(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLPRI |
POLLRDNORM | POLLRDBAND);
sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
@@ -2324,7 +2324,7 @@ static void sock_def_write_space(struct sock *sk)
*/
if ((atomic_read(&sk->sk_wmem_alloc) << 1) <= sk->sk_sndbuf) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
diff --git a/net/core/stream.c b/net/core/stream.c
index d70f77a..8ff9d63 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -35,7 +35,7 @@ void sk_stream_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
if (wq && wq->fasync_list && !(sk->sk_shutdown & SEND_SHUTDOWN))
diff --git a/net/dccp/output.c b/net/dccp/output.c
index 4ce912e..b66c84d 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -201,7 +201,7 @@ void dccp_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
/* Should agree with poll, otherwise some programs break */
if (sock_writeable(sk))
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fcb2752..4f0aa91 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -303,7 +303,7 @@ static void iucv_sock_wake_msglim(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_all(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
rcu_read_unlock();
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 1f8a144..7e2d105 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -67,7 +67,7 @@ static void rxrpc_write_space(struct sock *sk)
if (rxrpc_writable(sk)) {
struct socket_wq *wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible(&wq->wait);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 897c01c..ec10b66 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6978,7 +6978,7 @@ void sctp_data_ready(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM | POLLRDBAND);
sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 552dbab..525acf6 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1492,7 +1492,7 @@ static void tipc_write_space(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
POLLWRNORM | POLLWRBAND);
rcu_read_unlock();
@@ -1509,7 +1509,7 @@ static void tipc_data_ready(struct sock *sk)
rcu_read_lock();
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
POLLRDNORM | POLLRDBAND);
rcu_read_unlock();
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index aaa0b58..0446ff1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -339,7 +339,7 @@ static void unix_write_space(struct sock *sk)
rcu_read_lock();
if (unix_writable(sk)) {
wq = rcu_dereference(sk->sk_wq);
- if (wq_has_sleeper(wq))
+ if (skwq_has_sleeper(wq))
wake_up_interruptible_sync_poll(&wq->wait,
POLLOUT | POLLWRNORM | POLLWRBAND);
sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] net: Generalise wq_has_sleeper helper
2015-11-26 5:55 ` [PATCH v2] " Herbert Xu
@ 2015-11-30 19:46 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2015-11-30 19:46 UTC (permalink / raw)
To: herbert; +Cc: peterz, tatsu, mingo, linux-kernel, netdev, jolsa, eric.dumazet
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 26 Nov 2015 13:55:39 +0800
> The memory barrier in the helper wq_has_sleeper is needed by just
> about every user of waitqueue_active. This patch generalises it
> by making it take a wait_queue_head_t directly. The existing
> helper is renamed to skwq_has_sleeper.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks a lot Herbert.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-11-30 19:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-22 8:01 [PATCH v2] wait: add comment before waitqueue_active noting memory barrier is required Kosuke Tatsukawa
2015-10-22 12:56 ` Peter Zijlstra
2015-10-22 23:18 ` Kosuke Tatsukawa
2015-10-23 12:40 ` Peter Zijlstra
2015-11-11 9:48 ` Herbert Xu
2015-11-24 5:54 ` net: Generalise wq_has_sleeper helper Herbert Xu
2015-11-24 21:30 ` David Miller
2015-11-25 1:10 ` Herbert Xu
2015-11-25 9:15 ` Peter Zijlstra
2015-11-25 16:37 ` David Miller
2015-11-26 5:55 ` [PATCH v2] " Herbert Xu
2015-11-30 19:46 ` David Miller
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.