On Sun, 2009-08-30 at 12:42 -0400, Christoph Hellwig wrote:
> On Sun, Aug 30, 2009 at 12:34:33PM -0400, Chuck Lever wrote:
> > +static void nfs_kill_super(struct super_block *sb)
> > {
> > + struct nfs_server *server = NFS_SB(sb);
> > +
> > + dprintk("--> %s\n", __func__);
> > +
> > +#ifdef CONFIG_NFS_V4
> > + if (server->nfs_client->rpc_ops->version == 4) {
> > + nfs_super_return_all_delegations(sb);
> > + nfs4_renewd_prepare_shutdown(server);
> > + }
> > +#endif /* CONFIG_NFS_V4 */
> >
> > bdi_unregister(&server->backing_dev_info);
>
> This was previously not done for nfs4. If it is a bug-fixed that
> should be documented in the patch description, and if not it needs
> to be changed.
It has always been done, but it was in a separate function
(nfs4_kill_super()). I don't really see what we gain by inlining it into
nfs_kill_super..
Also, I'm concerned about the growth of "if (version == X)" type
constructs. We shouldn't be looking at the version number in order to
figure out whether or not we're holding delegations, or whether or not a
particular daemon thread is running.
AFAICS, in this case it should in any case be safe to call
nfs_super_return_all_delegations() (as long as CONFIG_NFS_V4 is defined
- that we might want to fix). Ditto for nfs4_renewd_prepare_shutdown().
Cheers
Trond
--
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
