SUNRPC: Ensure that we wait for connections to complete before retrying

Commit 718ba5b87343, moved the responsibility for unlocking the socket to
xs_tcp_setup_socket, meaning that the socket will be unlocked before we
know that it has finished trying to connect. The following patch is based on
an initial patch by Russell King to ensure that we delay clearing the
XPRT_CONNECTING flag until we either know that we failed to initiate
a connection attempt, or the connection attempt itself failed.

Fixes: 718ba5b87343 ("SUNRPC: Add helpers to prevent socket create from racing")
Reported-by: Russell King <linux@arm.linux.org.uk>
Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 7591788e9fbff3533214cf02435c3d7c1bbb1352..357e44c1a46b1b78d401fded6309b7e4b668f6a6 100644 (file)
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -42,6 +42,7 @@ struct sock_xprt {
        /*
         * Connection of transports
         */
+       unsigned long           sock_state;
        struct delayed_work     connect_worker;
        struct sockaddr_storage srcaddr;
        unsigned short          srcport;
@@ -76,6 +77,8 @@ struct sock_xprt {
  */
 #define TCP_RPC_REPLY          (1UL << 6)
 
+#define XPRT_SOCK_CONNECTING   1U
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SUNRPC_XPRTSOCK_H */

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index d2dfbd043bea9fd5e34a68cfb12082b9102761ac..c35038511686ad53bb042b0135138f13b4ee18aa 100644 (file)
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1438,6 +1438,7 @@ out:
 static void xs_tcp_state_change(struct sock *sk)
 {
        struct rpc_xprt *xprt;
+       struct sock_xprt *transport;
 
        read_lock_bh(&sk->sk_callback_lock);
        if (!(xprt = xprt_from_sock(sk)))
@@ -1449,13 +1450,12 @@ static void xs_tcp_state_change(struct sock *sk)
                        sock_flag(sk, SOCK_ZAPPED),
                        sk->sk_shutdown);
 
+       transport = container_of(xprt, struct sock_xprt, xprt);
        trace_rpc_socket_state_change(xprt, sk->sk_socket);
        switch (sk->sk_state) {
        case TCP_ESTABLISHED:
                spin_lock(&xprt->transport_lock);
                if (!xprt_test_and_set_connected(xprt)) {
-                       struct sock_xprt *transport = container_of(xprt,
-                                       struct sock_xprt, xprt);
 
                        /* Reset TCP record info */
                        transport->tcp_offset = 0;
@@ -1464,6 +1464,8 @@ static void xs_tcp_state_change(struct sock *sk)
                        transport->tcp_flags =
                                TCP_RCV_COPY_FRAGHDR | TCP_RCV_COPY_XID;
                        xprt->connect_cookie++;
+                       clear_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
+                       xprt_clear_connecting(xprt);
 
                        xprt_wake_pending_tasks(xprt, -EAGAIN);
                }
@@ -1499,6 +1501,9 @@ static void xs_tcp_state_change(struct sock *sk)
                smp_mb__after_atomic();
                break;
        case TCP_CLOSE:
+               if (test_and_clear_bit(XPRT_SOCK_CONNECTING,
+                                       &transport->sock_state))
+                       xprt_clear_connecting(xprt);
                xs_sock_mark_closed(xprt);
        }
  out:
@@ -2182,6 +2187,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
        /* Tell the socket layer to start connecting... */
        xprt->stat.connect_count++;
        xprt->stat.connect_start = jiffies;
+       set_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
        ret = kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
        switch (ret) {
        case 0:
@@ -2243,7 +2249,6 @@ static void xs_tcp_setup_socket(struct work_struct *work)
        case -EINPROGRESS:
        case -EALREADY:
                xprt_unlock_connect(xprt, transport);
-               xprt_clear_connecting(xprt);
                return;
        case -EINVAL:
                /* Happens, for instance, if the user specified a link