| Message ID | 4FA345DA4F4AE44899BD2B03EEEC2FA90928C74A@SACEXCMBX04-PRD.hq.netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 23, 2012 at 04:02:27PM +0000, Myklebust, Trond wrote: > call_transmit() should normally just cause the rpc_task to detect that > the connection is down, and should cause it to go to call_connect in > order to fix everything. > > If it is causing a race, then I'm thinking that is probably due to the > call to xs_tcp_shutdown() when we get an EPIPE error in > xs_tcp_send_request(). That shutdown seems to be needed only in the case > when we're in TCP_CLOSE_WAIT, in which case what we really want is to > force the rpc_task to go to xprt_connect, which means clearing > xprt_connected(). > > Hmm.... Can you try applying the attached 2 patches? I rebuilt with those two patches as well as the first one that removed the `xs_error_report' callback. Good news, it works! However, looking at the interactions, it still seems a little bit racy. Clearly you know more about this than I do, so I'll defer to your better judgement, but let me lay out what I believe is happening: Stat Thread rpciod ------------- -------- - woken up with tk_status set to EAGAIN via xs_tcp_state_change - tries call_transmit again, which calls xs_tcp_send_request which comes back with ECONNRESET (b/c this is what sk_err is) - call_status sees this and goes to sleep for 3 seconds and then sets tk_action to call_bind - call_bind - call_connect, queues execution of xs_tcp_setup_socket with rpciod and goes to sleep - starts doing its thing, but wakes up our other thread before its done trying to connect the socket (or failing to do so) via the call stack mentioned before (via xprt_disconnect_done) - tk_status is set to EAGAIN, so call_connect_status moves on to call_transmit - call_transmit calls xs_tcp_send_request again which finds the socket with sk_shutdown != 0 and so returns EPIPE - call_status sees the EPIPE and sets tk_action back to call_bind - continues on and updates the state of the socket to TCP_SYN_SENT - When the connection is established, xs_tcp_state_change takes care of marking the xprt connected - call_bind - call_connect, however this time it schedules no work because it finds that the transport is already connected - call_transmit, succeeds, we recover This is how the recovery went when I was testing, but I could imagine a situation where different timing comes into play and we wind up asking rpciod to execute xs_tcp_setup_socket twice (because the first connect hasn't been established yet). I haven't thought too hard about what that would mean, but I imagine it probably wouldn't be good. Like I said, you know way more about this than I do, I just wanted to point this out. Its entirely possible that I'm missing or misunderstanding something. Thanks! Chris -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From ed42390cf42a0ea483d7a8d7e2e2b397ec0a8f00 Mon Sep 17 00:00:00 2001 From: Trond Myklebust <Trond.Myklebust@netapp.com> Date: Tue, 23 Oct 2012 11:40:02 -0400 Subject: [PATCH 2/2] Revert "SUNRPC: Ensure we close the socket on EPIPE errors too..." This reverts commit 55420c24a0d4d1fce70ca713f84aa00b6b74a70e. Now that we clear the connected flag when entering TCP_CLOSE_WAIT, the deadlock described in this commit is no longer possible. Instead, the resulting call to xs_tcp_shutdown() can interfere with pending reconnection attempts. Reported-by: Chris Perl <chris.perl@gmail.com> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com> Cc: stable@vger.kernel.org --- net/sunrpc/xprtsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index 6e6967d..7e2dd0d 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -737,10 +737,10 @@ static int xs_tcp_send_request(struct rpc_task *task) dprintk("RPC: sendmsg returned unrecognized error %d\n", -status); case -ECONNRESET: - case -EPIPE: xs_tcp_shutdown(xprt); case -ECONNREFUSED: case -ENOTCONN: + case -EPIPE: clear_bit(SOCK_ASYNC_NOSPACE, &transport->sock->flags); } -- 1.7.11.7