On Tue, 29 Oct 2013 15:02:36 +0000 "Myklebust, Trond"
<Trond.Myklebust@xxxxxxxxxx> wrote:
> 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
Thanks for asking that! I had the exact same concern when I first conceived
the patch.
I managed to convince my self that there wasn't a problem as long as
xs_tcp_setup_socket never called into xs_close.
Otherwise the worst case is that one thread running xs_close could block
while some other thread runs xs_{tcp,udp}_setup_socket.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
