Re: [PATCH/RFC] - hard-to-hit race in xprtsock.

["Myklebust, Trond" <Trond.Myklebust@netapp.com>]

On Tue, 2013-10-29 at 17:42 +1100, NeilBrown wrote:
> We have a customer who hit a rare race in sunrpc (in a 3.0 based kernel,
> but the relevant code doesn't seem to have changed much).
> 
> The thread that crashed was in 
>   xs_tcp_setup_socket -> inet_stream_connect -> lock_sock_nested.
> 
> 'sock' in this last function is NULL.
> 
> The only way I can imagine this happening is if some other thread called
> 
>  xs_close -> xs_reset_transport -> sock_release -> inet_release
> 
> in a very small window a moment earlier.
> 
> As far as I can tell, xs_close is only called with XPRT_LOCKED set.
> 
> xs_tcp_setup_socket is mostly scheduled with XPRT_LOCKED set to which would
> exclude them from running at the same time.
> 
> 
> However xs_tcp_schedule_linger_timeout can schedule the thread which runs
> xs_tcp_setup_socket without first claiming XPRT_LOCKED.
> So I assume that is what is happening.
> 
> I imagine some race between the client closing the socket, and getting
> TCP_FIN_WAIT1 from the server and somehow the two threads racing.
> 
> I wonder if it might make sense to always abort 'connect_worker' in
> xs_close()?
> I think the connect_worker really mustn't be running or queued at this point,
> so cancelling it is either a no-op, or vitally important.
> 
> So: does the following patch seem reasonable?  If so I'll submit it properly
> with a coherent description etc.

Hi Neil,

Will that do the right thing if the connect_worker and close are running
on the same rpciod thread? I think it should, but I never manage to keep
100% up to date with the ever changing semantics of
cancel_delayed_work_sync() and friends...

Cheers,
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com