nfs_kill_super() and nfs4_kill_super() do almost the same thing, so combine
them.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfs/super.c | 45 +++++++++++++++++++++------------------------
1 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index f3a95df..d215707 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -282,13 +282,12 @@ static int nfs4_referral_get_sb(struct file_system_type *fs_type,
int flags, const char *dev_name, void *raw_data, struct vfsmount *mnt);
static int nfs4_remote_referral_get_sb(struct file_system_type *fs_type,
int flags, const char *dev_name, void *raw_data, struct vfsmount *mnt);
-static void nfs4_kill_super(struct super_block *sb);
static struct file_system_type nfs4_fs_type = {
.owner = THIS_MODULE,
.name = "nfs4",
.get_sb = nfs4_get_sb,
- .kill_sb = nfs4_kill_super,
+ .kill_sb = nfs_kill_super,
.fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
};
@@ -296,7 +295,7 @@ static struct file_system_type nfs4_remote_fs_type = {
.owner = THIS_MODULE,
.name = "nfs4",
.get_sb = nfs4_remote_get_sb,
- .kill_sb = nfs4_kill_super,
+ .kill_sb = nfs_kill_super,
.fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
};
@@ -304,7 +303,7 @@ struct file_system_type nfs4_xdev_fs_type = {
.owner = THIS_MODULE,
.name = "nfs4",
.get_sb = nfs4_xdev_get_sb,
- .kill_sb = nfs4_kill_super,
+ .kill_sb = nfs_kill_super,
.fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
};
@@ -312,7 +311,7 @@ static struct file_system_type nfs4_remote_referral_fs_type = {
.owner = THIS_MODULE,
.name = "nfs4",
.get_sb = nfs4_remote_referral_get_sb,
- .kill_sb = nfs4_kill_super,
+ .kill_sb = nfs_kill_super,
.fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
};
@@ -320,7 +319,7 @@ struct file_system_type nfs4_referral_fs_type = {
.owner = THIS_MODULE,
.name = "nfs4",
.get_sb = nfs4_referral_get_sb,
- .kill_sb = nfs4_kill_super,
+ .kill_sb = nfs_kill_super,
.fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
};
@@ -2127,16 +2126,27 @@ error_splat_super:
}
/*
- * Destroy an NFS2/3 superblock
+ * Destroy an NFS superblock
*/
-static void nfs_kill_super(struct super_block *s)
+static void nfs_kill_super(struct super_block *sb)
{
- struct nfs_server *server = NFS_SB(s);
+ 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);
- kill_anon_super(s);
- nfs_fscache_release_super_cookie(s);
+ kill_anon_super(sb);
+ nfs_fscache_release_super_cookie(sb);
nfs_free_server(server);
+
+ dprintk("<-- %s\n", __func__);
}
/*
@@ -2599,19 +2609,6 @@ out_free_data:
return error;
}
-static void nfs4_kill_super(struct super_block *sb)
-{
- struct nfs_server *server = NFS_SB(sb);
-
- dprintk("--> %s\n", __func__);
- nfs_super_return_all_delegations(sb);
- kill_anon_super(sb);
- nfs4_renewd_prepare_shutdown(server);
- nfs_fscache_release_super_cookie(sb);
- nfs_free_server(server);
- dprintk("<-- %s\n", __func__);
-}
-
/*
* Clone an NFS4 server record on xdev traversal (FSID-change)
*/
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.
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
On Aug 30, 2009, at 12:59 PM, Trond Myklebust wrote:
> 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..
Actually nfs4_kill_super didn't unregister the bdi.
> 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().
That's all fine, but as I said in the cover letter, this patch may be
dropped since the file system is still nfs4 under the covers, so I
think nfs4_kill_super would be invoked anyway for "-t nfs -o vers=4".
>
> Cheers
> Trond
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com