Re: [PATCH 1/2] af_unix: fix unix_dgram_poll() behavior for EPOLLOUT event
From: Davide Libenzi <hidden>
Date: 2010-10-31 19:08:53
On Sun, 31 Oct 2010, Eric Dumazet wrote:
Le samedi 30 octobre 2010 à 22:47 +0100, Alban Crequy a écrit :quoted
Le Sat, 30 Oct 2010 15:17:58 +0200, Eric Dumazet [off-list ref] a écrit :quoted
quoted
Problem is the peer_wait, that epoll doesnt seem to be plugged into. Bug is in unix_dgram_poll() It calls sock_poll_wait( ... &unix_sk(other)->peer_wait,) only if socket is 'writable'. Its a clear bugYes, it looks weird...quoted
quoted
Try this patch please ?I will be away from computer and I will continue to work on this from the 20th of November.OK, no problem. I tried it and it solves the problem. Here is an official patch submission. David, for your convenience, I cooked a patch serie for the 2 patches against af_unix unix_dgram_poll().
Looks good to me... Acked-by: Davide Libenzi <redacted>
quoted hunk ↗ jump to hunk
The third patch (af_unix: unix_write_space() use keyed wakeups)) can be applied as is. Thanks ! [PATCH 1/2] af_unix: fix unix_dgram_poll() behavior for EPOLLOUT event Alban Crequy reported a problem with connected dgram af_unix sockets and provided a test program. epoll() would miss to send an EPOLLOUT event when a thread unqueues a packet from the other peer, making its receive queue not full. This is because unix_dgram_poll() fails to call sock_poll_wait(file, &unix_sk(other)->peer_wait, wait); if the socket is not writeable at the time epoll_ctl(ADD) is called. We must call sock_poll_wait(), regardless of 'writable' status, so that epoll can be notified later of states changes. Misc: avoids testing twice (sk->sk_shutdown & RCV_SHUTDOWN) Reported-by: Alban Crequy <redacted> Cc: Davide Libenzi <redacted> Signed-off-by: Eric Dumazet <redacted> --- net/unix/af_unix.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 0ebc777..7375131 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c@@ -2072,13 +2072,12 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock, if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue)) mask |= POLLERR; if (sk->sk_shutdown & RCV_SHUTDOWN) - mask |= POLLRDHUP; + mask |= POLLRDHUP | POLLIN | POLLRDNORM; if (sk->sk_shutdown == SHUTDOWN_MASK) mask |= POLLHUP; /* readable? */ - if (!skb_queue_empty(&sk->sk_receive_queue) || - (sk->sk_shutdown & RCV_SHUTDOWN)) + if (!skb_queue_empty(&sk->sk_receive_queue)) mask |= POLLIN | POLLRDNORM; /* Connection-based need to check for termination and startup */@@ -2090,20 +2089,15 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock, return mask; } - /* writable? */ writable = unix_writable(sk); - if (writable) { - other = unix_peer_get(sk); - if (other) { - if (unix_peer(other) != sk) { - sock_poll_wait(file, &unix_sk(other)->peer_wait, - wait); - if (unix_recvq_full(other)) - writable = 0; - } - - sock_put(other); + other = unix_peer_get(sk); + if (other) { + if (unix_peer(other) != sk) { + sock_poll_wait(file, &unix_sk(other)->peer_wait, wait); + if (unix_recvq_full(other)) + writable = 0; } + sock_put(other); } if (writable)
- Davide