| Message ID | 1461605974-4242-2-git-send-email-edumazet@google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote: >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async() >so that a SIGIO signal is sent when needed. > >tcp_sendmsg() clears the bit. >tcp_poll() sets the bit when stream is not writeable. > >We can avoid two atomic operations by first checking if socket >is actually interested in the FASYNC business (most sockets in >real applications do not use AIO, but select()/poll()/epoll()) > >This also removes one cache line miss to access sk->sk_wq->flags >in tcp_sendmsg() > >Signed-off-by: Eric Dumazet <edumazet@google.com> I just bisected down to this. This is causing a regression for me when my nfs mount becomes stuck. I can easily reproduce this if you need to test the fix. Thanks.
On Mon, 2016-05-02 at 18:16 +0200, Jiri Pirko wrote: > Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote: > >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async() > >so that a SIGIO signal is sent when needed. > > > >tcp_sendmsg() clears the bit. > >tcp_poll() sets the bit when stream is not writeable. > > > >We can avoid two atomic operations by first checking if socket > >is actually interested in the FASYNC business (most sockets in > >real applications do not use AIO, but select()/poll()/epoll()) > > > >This also removes one cache line miss to access sk->sk_wq->flags > >in tcp_sendmsg() > > > >Signed-off-by: Eric Dumazet <edumazet@google.com> > > I just bisected down to this. This is causing a regression for me when > my nfs mount becomes stuck. I can easily reproduce this if you need to > test the fix. What do you mean by 'when nfs mount becomes stuck' ? Is this patch making nfs not functional , or does it make recovery from some nfs error bad ? Thanks.
Mon, May 02, 2016 at 06:22:18PM CEST, eric.dumazet@gmail.com wrote: >On Mon, 2016-05-02 at 18:16 +0200, Jiri Pirko wrote: >> Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote: >> >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async() >> >so that a SIGIO signal is sent when needed. >> > >> >tcp_sendmsg() clears the bit. >> >tcp_poll() sets the bit when stream is not writeable. >> > >> >We can avoid two atomic operations by first checking if socket >> >is actually interested in the FASYNC business (most sockets in >> >real applications do not use AIO, but select()/poll()/epoll()) >> > >> >This also removes one cache line miss to access sk->sk_wq->flags >> >in tcp_sendmsg() >> > >> >Signed-off-by: Eric Dumazet <edumazet@google.com> >> >> I just bisected down to this. This is causing a regression for me when >> my nfs mount becomes stuck. I can easily reproduce this if you need to >> test the fix. > >What do you mean by 'when nfs mount becomes stuck' ? > >Is this patch making nfs not functional , or does it make recovery from >some nfs error bad ? I can mount nfs on the host. But when I do something (compile a kernel module in my case), it gets stuck. Then I cannot even ssh to the machine. No messages in dmesg. I didn't debug it any further. I just bisected and verified that this patch caused this behaviour.
diff --git a/include/net/sock.h b/include/net/sock.h index d63b8494124e..0f48aad9f8e8 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1940,11 +1940,17 @@ static inline unsigned long sock_wspace(struct sock *sk) */ static inline void sk_set_bit(int nr, struct sock *sk) { + if (nr == SOCKWQ_ASYNC_NOSPACE && !sock_flag(sk, SOCK_FASYNC)) + return; + set_bit(nr, &sk->sk_wq_raw->flags); } static inline void sk_clear_bit(int nr, struct sock *sk) { + if (nr == SOCKWQ_ASYNC_NOSPACE && !sock_flag(sk, SOCK_FASYNC)) + return; + clear_bit(nr, &sk->sk_wq_raw->flags); }
SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async() so that a SIGIO signal is sent when needed. tcp_sendmsg() clears the bit. tcp_poll() sets the bit when stream is not writeable. We can avoid two atomic operations by first checking if socket is actually interested in the FASYNC business (most sockets in real applications do not use AIO, but select()/poll()/epoll()) This also removes one cache line miss to access sk->sk_wq->flags in tcp_sendmsg() Signed-off-by: Eric Dumazet <edumazet@google.com> --- include/net/sock.h | 6 ++++++ 1 file changed, 6 insertions(+)