Skip to content

Commit

Permalink
SUNRPC: Ensure that we wait for connections to complete before retrying
Browse files Browse the repository at this point in the history
Commit 718ba5b, 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: 718ba5b ("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>
  • Loading branch information
trondmypd committed Sep 17, 2015
1 parent 17a9618 commit 0fdea1e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
3 changes: 3 additions & 0 deletions include/linux/sunrpc/xprtsock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
11 changes: 8 additions & 3 deletions net/sunrpc/xprtsock.c
Original file line number Diff line number Diff line change
Expand Up @@ -1438,6 +1438,7 @@ static void xs_tcp_data_ready(struct sock *sk)
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)))
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0fdea1e

Please sign in to comment.