Skip to content

Commit 0fdea1e

Browse files
committed
SUNRPC: Ensure that we wait for connections to complete before retrying
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>
1 parent 17a9618 commit 0fdea1e

File tree

2 files changed

+11
-3
lines changed

2 files changed

+11
-3
lines changed

include/linux/sunrpc/xprtsock.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ struct sock_xprt {
4242
/*
4343
* Connection of transports
4444
*/
45+
unsigned long sock_state;
4546
struct delayed_work connect_worker;
4647
struct sockaddr_storage srcaddr;
4748
unsigned short srcport;
@@ -76,6 +77,8 @@ struct sock_xprt {
7677
*/
7778
#define TCP_RPC_REPLY (1UL << 6)
7879

80+
#define XPRT_SOCK_CONNECTING 1U
81+
7982
#endif /* __KERNEL__ */
8083

8184
#endif /* _LINUX_SUNRPC_XPRTSOCK_H */

net/sunrpc/xprtsock.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,6 +1438,7 @@ static void xs_tcp_data_ready(struct sock *sk)
14381438
static void xs_tcp_state_change(struct sock *sk)
14391439
{
14401440
struct rpc_xprt *xprt;
1441+
struct sock_xprt *transport;
14411442

14421443
read_lock_bh(&sk->sk_callback_lock);
14431444
if (!(xprt = xprt_from_sock(sk)))
@@ -1449,13 +1450,12 @@ static void xs_tcp_state_change(struct sock *sk)
14491450
sock_flag(sk, SOCK_ZAPPED),
14501451
sk->sk_shutdown);
14511452

1453+
transport = container_of(xprt, struct sock_xprt, xprt);
14521454
trace_rpc_socket_state_change(xprt, sk->sk_socket);
14531455
switch (sk->sk_state) {
14541456
case TCP_ESTABLISHED:
14551457
spin_lock(&xprt->transport_lock);
14561458
if (!xprt_test_and_set_connected(xprt)) {
1457-
struct sock_xprt *transport = container_of(xprt,
1458-
struct sock_xprt, xprt);
14591459

14601460
/* Reset TCP record info */
14611461
transport->tcp_offset = 0;
@@ -1464,6 +1464,8 @@ static void xs_tcp_state_change(struct sock *sk)
14641464
transport->tcp_flags =
14651465
TCP_RCV_COPY_FRAGHDR | TCP_RCV_COPY_XID;
14661466
xprt->connect_cookie++;
1467+
clear_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
1468+
xprt_clear_connecting(xprt);
14671469

14681470
xprt_wake_pending_tasks(xprt, -EAGAIN);
14691471
}
@@ -1499,6 +1501,9 @@ static void xs_tcp_state_change(struct sock *sk)
14991501
smp_mb__after_atomic();
15001502
break;
15011503
case TCP_CLOSE:
1504+
if (test_and_clear_bit(XPRT_SOCK_CONNECTING,
1505+
&transport->sock_state))
1506+
xprt_clear_connecting(xprt);
15021507
xs_sock_mark_closed(xprt);
15031508
}
15041509
out:
@@ -2182,6 +2187,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
21822187
/* Tell the socket layer to start connecting... */
21832188
xprt->stat.connect_count++;
21842189
xprt->stat.connect_start = jiffies;
2190+
set_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
21852191
ret = kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
21862192
switch (ret) {
21872193
case 0:
@@ -2243,7 +2249,6 @@ static void xs_tcp_setup_socket(struct work_struct *work)
22432249
case -EINPROGRESS:
22442250
case -EALREADY:
22452251
xprt_unlock_connect(xprt, transport);
2246-
xprt_clear_connecting(xprt);
22472252
return;
22482253
case -EINVAL:
22492254
/* Happens, for instance, if the user specified a link

0 commit comments

Comments
 (0)