diff options
| -rw-r--r-- | net/rds/tcp.c | 70 |
1 files changed, 45 insertions, 25 deletions
diff --git a/net/rds/tcp.c b/net/rds/tcp.c index ad14217867a4..f4c83e368390 100644 --- a/net/rds/tcp.c +++ b/net/rds/tcp.c @@ -115,42 +115,48 @@ void rds_tcp_restore_callbacks(struct socket *sock, } /* - * rds_tcp_reset_callbacks() switches the to the new sock and - * returns the existing tc->t_sock. + * rds_tcp_reset_callbacks() switches a path to a new socket and + * releases the old one it finds in tc->t_sock, resolving a duelling + * SYN. * - * The only functions that set tc->t_sock are rds_tcp_set_callbacks - * and rds_tcp_reset_callbacks. Send and receive trust that - * it is set. The absence of RDS_CONN_UP bit protects those paths - * from being called while it isn't set. + * tc->t_sock is set by rds_tcp_set_callbacks() and cleared by + * rds_tcp_restore_callbacks(). Four paths write it: the active + * connect in rds_tcp_conn_path_connect(), which sets it and clears it + * again on failure; the accept path in rds_tcp_accept_one(), which + * sets it for a path with no socket yet; the teardown in + * rds_tcp_conn_path_shutdown(), which clears it; and the swap done + * here, which does both. The connect and accept paths are serialized + * against each other by t_conn_path_lock. Send and receive trust + * that it is set: the absence of RDS_CONN_UP protects those paths + * from being called while it isn't, and the swap done here runs under + * RDS_IN_XMIT so that it cannot interleave with a sender already + * inside rds_send_xmit(). */ void rds_tcp_reset_callbacks(struct socket *sock, struct rds_conn_path *cp) { struct rds_tcp_connection *tc = cp->cp_transport_data; - struct socket *osock = tc->t_sock; - - if (!osock) - goto newsock; + struct socket *osock; /* Need to resolve a duelling SYN between peers. * We have an outstanding SYN to this peer, which may * potentially have transitioned to the RDS_CONN_UP state, * so we must quiesce any send threads before resetting - * cp_transport_data. We quiesce these threads by setting - * cp_state to something other than RDS_CONN_UP, and then - * waiting for any existing threads in rds_send_xmit to - * complete release_in_xmit(). (Subsequent threads entering - * rds_send_xmit() will bail on !rds_conn_up(). + * cp_transport_data. Setting cp_state to something other + * than RDS_CONN_UP stops new senders, and owning RDS_IN_XMIT + * excludes any thread already inside rds_send_xmit() for the + * whole socket swap and the rds_send_path_reset() below. * - * However an incoming syn-ack at this point would end up - * marking the conn as RDS_CONN_UP, and would again permit - * rds_send_xmi() threads through, so ideally we would - * synchronize on RDS_CONN_UP after lock_sock(), but cannot - * do that: waiting on !RDS_IN_XMIT after lock_sock() may - * end up deadlocking with tcp_sendmsg(), and the RDS_IN_XMIT - * would not get set. As a result, we set c_state to - * RDS_CONN_RESETTTING, to ensure that rds_tcp_state_change - * cannot mark rds_conn_path_up() in the window before lock_sock(). + * An incoming syn-ack at this point would end up marking the + * conn as RDS_CONN_UP, and would again permit rds_send_xmit() + * threads through, so ideally we would synchronize on + * RDS_CONN_UP after lock_sock(), but cannot do that: acquiring + * RDS_IN_XMIT after lock_sock() may end up deadlocking with + * tcp_sendmsg(), which takes the socket lock while holding + * RDS_IN_XMIT. As a result, we set c_state to + * RDS_CONN_RESETTING, to ensure that rds_tcp_state_change + * cannot mark rds_conn_path_up() in the window before + * lock_sock(). * * Only make that transition if the path is still connecting * (or already resetting from an earlier duel). A path in any @@ -166,7 +172,18 @@ void rds_tcp_reset_callbacks(struct socket *sock, !rds_conn_path_transition(cp, RDS_CONN_RESETTING, RDS_CONN_RESETTING)) rds_conn_path_drop(cp, 0); - wait_event(cp->cp_waitq, !test_bit(RDS_IN_XMIT, &cp->cp_flags)); + wait_event(cp->cp_waitq, + !test_and_set_bit_lock(RDS_IN_XMIT, &cp->cp_flags)); + + /* Read t_sock only while owning RDS_IN_XMIT, never before the + * wait: the teardown in rds_conn_shutdown() releases the old + * socket and clears t_sock, so a pointer sampled earlier can + * be stale by the time we wake up. + */ + osock = tc->t_sock; + if (!osock) + goto newsock; + /* reset receive side state for rds_tcp_data_recv() for osock */ cancel_delayed_work_sync(&cp->cp_send_w); cancel_delayed_work_sync(&cp->cp_recv_w); @@ -185,6 +202,9 @@ newsock: lock_sock(sock->sk); rds_tcp_set_callbacks(sock, cp); release_sock(sock->sk); + + clear_bit_unlock(RDS_IN_XMIT, &cp->cp_flags); + wake_up_all(&cp->cp_waitq); } /* Add tc to rds_tcp_tc_list and set tc->t_sock. See comments |