On Wed, May 09, 2012 at 10:26:17AM -0400, J. Bruce Fields wrote:
> Reviewing your more recent patches I think we have a problem with some
> of the code that's already merged. See the comment in svc_shutdown_net:
>
> void svc_shutdown_net(struct svc_serv *serv, struct net *net)
> {
By the way, note there's some preexisting trouble here:
> /*
> * The set of xprts (contained in the sv_tempsocks and
> * sv_permsocks lists) is now constant, since it is modified
> * only by accepting new sockets (done by service threads in
> * svc_recv) or aging old ones (done by sv_temptimer), or
> * configuration changes (excluded by whatever locking the
> * caller is using--nfsd_mutex in the case of nfsd).
I don't think the callers are as careful about this as they should be,
so I think there may be some cases where we could crash if there are
multiple processes concurrently trying to start, stop, and/or modify the
listening sockets of a server.
We need to fix that too.
(I haven't actually seen that bug in practice. We *did* see people hit
bugs on shutdown of a busy server before fixing the receive/shutdown
races, though.)
--b.
> So it's
> * safe to traverse those lists and shut everything down:
> */
> svc_close_net(serv, net);
>
> if (serv->sv_shutdown)
> serv->sv_shutdown(serv, net);
> }
>
> So we depend on the fact that neither the server threads nor
> sv_temptimer are running here to be able to safely traverse those lists
> of sockets.
>
> But it looks to me like that's no longer true--we're shutting down just
> one namespace here, and others may still be running. If so and if they
> modify sv_tempsocks or sv_permsocks while we're running through them
> then we're going to get a crash.
>
> --b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
