* [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
@ 2012-06-23 10:46 Namjae Jeon
2012-06-23 15:07 ` Myklebust, Trond
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2012-06-23 10:46 UTC (permalink / raw)
To: bfields, Trond.Myklebust, akpm, bharrosh
Cc: linux-nfs, linux-kernel, Namjae Jeon, Namjae Jeon, Vivek Trivedi,
Amit Sahrawat
From: Namjae Jeon <namjae.jeon@samsung.com>
Without this patch:
On NFS Client:
$ mount -t nfs <NFS_SERVER>:/mnt /mnt
$ umount /mnt
On NFS Server:
$ umount /mnt
umount: can't umount /mnt: Device or resource busy
If user has to remove storage device user needs to unplug the
device.
This may result in file system corruption on attached media.
This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
removal of attached media at NFS Server.
With this patch:
On NFS Client:
$ mount -t nfs <NFS_SERVER>:/mnt /mnt
$ umount /mnt
On NFS Server:
$ umount /mnt --> umount successful
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
fs/nfs/super.c | 10 ++++++++++
fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
fs/nfsd/nfsd.h | 3 +++
fs/nfsd/nfssvc.c | 3 +++
fs/nfsd/xdr3.h | 14 +++++++++++++-
include/linux/nfs3.h | 1 +
include/linux/nfs_xdr.h | 9 +++++++++
11 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 2292a0f..726227f 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
}
+static int nfs3_proc_umount(struct nfs_server *server)
+{
+ /*
+ * dummy argument and response are assigned to UMOUNT request
+ * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
+ */
+ struct nfs3_umountargs arg = {
+ .dummy = 0,
+ };
+ struct nfs3_umountres res;
+
+ struct rpc_message msg = {
+ .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
+ .rpc_argp = &arg,
+ .rpc_resp = &res,
+
+ };
+ int err;
+
+ msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
+ dprintk("NFS call umount\n");
+ err = rpc_call_sync(server->client, &msg,
+ RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
+ put_rpccred(msg.rpc_cred);
+ dprintk("NFS reply umount: %d\n", err);
+ return err;
+}
+
const struct nfs_rpc_ops nfs_v3_clientops = {
.version = 3, /* protocol version */
.dentry_ops = &nfs_dentry_operations,
@@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
.clear_acl_cache = nfs3_forget_cached_acls,
.close_context = nfs_close_context,
.init_client = nfs_init_client,
+ .umount = nfs3_proc_umount,
};
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 6cbe894..874b8b2 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -61,6 +61,7 @@
#define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
#define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
#define NFS3_commitargs_sz (NFS3_fh_sz+3)
+#define NFS3_umountargs_sz (1)
#define NFS3_getattrres_sz (1+NFS3_fattr_sz)
#define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
@@ -78,6 +79,7 @@
#define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
#define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
#define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
+#define NFS3_umountres_sz (1 + 1)
#define ACL3_getaclargs_sz (NFS3_fh_sz+1)
#define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
@@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct rpc_rqst *req,
encode_commit3args(xdr, args);
}
+/*
+ * UMOUNT3args
+ */
+static void encode_umount3args(struct xdr_stream *xdr,
+ const struct nfs3_umountargs *args)
+{
+ encode_uint32(xdr, args->dummy);
+}
+
+static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
+ struct xdr_stream *xdr,
+ const struct nfs3_umountargs *args)
+{
+ encode_umount3args(xdr, args);
+}
+
#ifdef CONFIG_NFS_V3_ACL
static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
@@ -1429,6 +1447,26 @@ out_status:
}
/*
+ * UMOUNT3res
+ */
+static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
+ struct xdr_stream *xdr,
+ struct nfs3_umountres *result)
+{
+ enum nfs_stat status;
+ int error;
+
+ error = decode_nfsstat3(xdr, &status);
+ if (unlikely(error))
+ goto out;
+ if (status != NFS3_OK)
+ return nfs3_stat_to_errno(status);
+ error = decode_uint32(xdr, &result->dummy);
+out:
+ return error;
+}
+
+/*
* 3.3.3 LOOKUP3res
*
* struct LOOKUP3resok {
@@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
PROC(FSINFO, getattr, fsinfo, 0),
PROC(PATHCONF, getattr, pathconf, 0),
PROC(COMMIT, commit, commit, 5),
+ PROC(UMOUNT, umount, umount, 0),
};
const struct rpc_version nfs_version3 = {
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 906f09c..9fa295b 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
{
struct nfs_server *server = NFS_SB(s);
+ int err;
+
+ if (server->nfs_client->rpc_ops->umount) {
+ err = server->nfs_client->rpc_ops->umount(server);
+ if (err < 0) {
+ printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
+ __func__, err);
+ }
+ }
+
kill_anon_super(s);
nfs_fscache_release_super_cookie(s);
nfs_free_server(server);
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 9095f3c..12ca821 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -8,6 +8,7 @@
#include <linux/ext2_fs.h>
#include <linux/magic.h>
+#include "nfsd.h"
#include "cache.h"
#include "xdr3.h"
#include "vfs.h"
@@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
}
/*
+ * UMOUNT call.
+ */
+static __be32
+nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs *argp,
+ struct nfsd3_umountres *resp)
+{
+ dprintk("nfsd: UMOUNT(3)\n");
+
+ if (nfs_umountd_workqueue)
+ queue_work(nfs_umountd_workqueue, &nfs_umount_work);
+
+ return nfs_ok;
+}
+
+/*
* Set a file's attributes
*/
static __be32
@@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
#define pAT (1+AT) /* post attributes - conditional */
#define WC (7+pAT) /* WCC attributes */
-static struct svc_procedure nfsd_procedures3[22] = {
+static struct svc_procedure nfsd_procedures3[23] = {
[NFS3PROC_NULL] = {
.pc_func = (svc_procfunc) nfsd3_proc_null,
.pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
@@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] = {
.pc_cachetype = RC_NOCACHE,
.pc_xdrressize = ST+WC+2,
},
+ [NFS3PROC_UMOUNT] = {
+ .pc_func = (svc_procfunc) nfsd3_proc_umount,
+ .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
+ .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
+ .pc_argsize = sizeof(struct nfsd3_umountargs),
+ .pc_ressize = sizeof(struct nfsd3_umountres),
+ .pc_cachetype = RC_NOCACHE,
+ .pc_xdrressize = ST+1,
+ },
};
struct svc_version nfsd_version3 = {
.vs_vers = 3,
.vs_nproc = 22,
+ .vs_nproc = 23,
.vs_proc = nfsd_procedures3,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 43f46cd..8610282 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p,
return xdr_argsize_check(rqstp, p);
}
+int
+nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
+ struct nfsd3_umountargs *args)
+{
+ args->dummy = ntohl(*p++);
+
+ return xdr_argsize_check(rqstp, p);
+}
+
/*
* XDR encode functions
*/
@@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p,
return xdr_ressize_check(rqstp, p);
}
+/* UMOUNT */
+int
+nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
+ struct nfsd3_umountres *resp)
+{
+ if (resp->status == 0)
+ *p++ = htonl(resp->dummy);
+ return xdr_ressize_check(rqstp, p);
+}
+
/*
* XDR release functions
*/
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index c55298e..53748ac 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -50,6 +50,9 @@ enum {
#endif
};
+struct workqueue_struct *nfs_umountd_workqueue;
+struct work_struct nfs_umount_work;
+
/*
* write() for these nodes.
*/
@@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
.size = sizeof(struct nfsd_net),
};
+static void nfs_umountd_fn(struct work_struct *unused)
+{
+ nfsd_export_flush(&init_net);
+}
+
+static void nfsd_umount_destroy(void)
+{
+ if (nfs_umountd_workqueue)
+ destroy_workqueue(nfs_umountd_workqueue);
+}
+
static int __init init_nfsd(void)
{
int retval;
@@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
retval = register_filesystem(&nfsd_fs_type);
if (retval)
goto out_free_all;
+
+ nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
+ if (!nfs_umountd_workqueue)
+ printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
+ else
+ INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
+
return 0;
out_free_all:
remove_proc_entry("fs/nfs/exports", NULL);
@@ -1225,6 +1246,7 @@ out_unregister_notifier:
static void __exit exit_nfsd(void)
{
+ nfsd_umount_destroy();
nfsd_reply_cache_shutdown();
remove_proc_entry("fs/nfs/exports", NULL);
remove_proc_entry("fs/nfs", NULL);
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 1671429..691450a 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -20,6 +20,7 @@
#include <linux/nfsd/debug.h>
#include <linux/nfsd/export.h>
#include <linux/nfsd/stats.h>
+#include <linux/workqueue.h>
/*
* nfsd version
@@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
extern unsigned int nfsd_drc_mem_used;
extern const struct seq_operations nfs_exports_op;
+extern struct workqueue_struct *nfs_umountd_workqueue;
+extern struct work_struct nfs_umount_work;
/*
* Function prototypes.
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index ee709fc..1fb3598 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
{
/* When last nfsd thread exits we need to do some clean-up */
nfsd_serv = NULL;
+ if (nfs_umountd_workqueue)
+ flush_workqueue(nfs_umountd_workqueue);
+
nfsd_shutdown();
svc_rpcb_cleanup(serv, net);
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 7df980e..b542c92 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -106,6 +106,10 @@ struct nfsd3_commitargs {
__u32 count;
};
+struct nfsd3_umountargs {
+ __u32 dummy;
+};
+
struct nfsd3_getaclargs {
struct svc_fh fh;
int mask;
@@ -219,6 +223,11 @@ struct nfsd3_commitres {
struct svc_fh fh;
};
+struct nfsd3_umountres {
+ __be32 status;
+ __u32 dummy;
+};
+
struct nfsd3_getaclres {
__be32 status;
struct svc_fh fh;
@@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *, __be32 *,
struct nfsd3_readdirargs *);
int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
struct nfsd3_commitargs *);
+int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
+ struct nfsd3_umountargs *);
int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
struct nfsd3_attrstat *);
@@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *, __be32 *,
struct nfsd3_pathconfres *);
int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
struct nfsd3_commitres *);
-
+int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
+ struct nfsd3_umountres *);
int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
struct nfsd3_attrstat *);
int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
index 6ccfe3b..968e2e0 100644
--- a/include/linux/nfs3.h
+++ b/include/linux/nfs3.h
@@ -90,6 +90,7 @@ struct nfs3_fh {
#define NFS3PROC_FSINFO 19
#define NFS3PROC_PATHCONF 20
#define NFS3PROC_COMMIT 21
+#define NFS3PROC_UMOUNT 22
#define NFS_MNT3_VERSION 3
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 5c0014d..da53bd8 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -786,6 +786,14 @@ struct nfs3_readdirargs {
struct page ** pages;
};
+struct nfs3_umountargs {
+ __u32 dummy;
+};
+
+struct nfs3_umountres {
+ __u32 dummy;
+};
+
struct nfs3_diropres {
struct nfs_fattr * dir_attr;
struct nfs_fh * fh;
@@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
struct nfs_client *
(*init_client) (struct nfs_client *, const struct rpc_timeout *,
const char *, rpc_authflavor_t);
+ int (*umount)(struct nfs_server *);
};
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 10:46 [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure Namjae Jeon
@ 2012-06-23 15:07 ` Myklebust, Trond
2012-06-23 15:52 ` Myklebust, Trond
0 siblings, 1 reply; 13+ messages in thread
From: Myklebust, Trond @ 2012-06-23 15:07 UTC (permalink / raw)
To: Namjae Jeon
Cc: <bfields@fieldses.org>,
Myklebust, Trond, <akpm@linux-foundation.org>,
<bharrosh@panasas.com>, <linux-nfs@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
Namjae Jeon, Vivek Trivedi, Amit Sahrawat
BIG NACK... We don't do embrace and extend in Linux...
The whole point of the NFS protocol is to be a standard. We can't just go adding new functionality to one implementation of that standard without breaking interoperability with other implementations...
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>
> Without this patch:
>
> On NFS Client:
> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
> $ umount /mnt
>
> On NFS Server:
> $ umount /mnt
> umount: can't umount /mnt: Device or resource busy
>
> If user has to remove storage device user needs to unplug the
> device.
> This may result in file system corruption on attached media.
>
> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
> removal of attached media at NFS Server.
>
> With this patch:
>
> On NFS Client:
> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
> $ umount /mnt
>
> On NFS Server:
> $ umount /mnt --> umount successful
>
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
> ---
> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
> fs/nfs/super.c | 10 ++++++++++
> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
> fs/nfsd/nfsd.h | 3 +++
> fs/nfsd/nfssvc.c | 3 +++
> fs/nfsd/xdr3.h | 14 +++++++++++++-
> include/linux/nfs3.h | 1 +
> include/linux/nfs_xdr.h | 9 +++++++++
> 11 files changed, 175 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index 2292a0f..726227f 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
> }
>
> +static int nfs3_proc_umount(struct nfs_server *server)
> +{
> + /*
> + * dummy argument and response are assigned to UMOUNT request
> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
> + */
> + struct nfs3_umountargs arg = {
> + .dummy = 0,
> + };
> + struct nfs3_umountres res;
> +
> + struct rpc_message msg = {
> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
> + .rpc_argp = &arg,
> + .rpc_resp = &res,
> +
> + };
> + int err;
> +
> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
> + dprintk("NFS call umount\n");
> + err = rpc_call_sync(server->client, &msg,
> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
> + put_rpccred(msg.rpc_cred);
> + dprintk("NFS reply umount: %d\n", err);
> + return err;
> +}
> +
> const struct nfs_rpc_ops nfs_v3_clientops = {
> .version = 3, /* protocol version */
> .dentry_ops = &nfs_dentry_operations,
> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
> .clear_acl_cache = nfs3_forget_cached_acls,
> .close_context = nfs_close_context,
> .init_client = nfs_init_client,
> + .umount = nfs3_proc_umount,
> };
> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> index 6cbe894..874b8b2 100644
> --- a/fs/nfs/nfs3xdr.c
> +++ b/fs/nfs/nfs3xdr.c
> @@ -61,6 +61,7 @@
> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
> +#define NFS3_umountargs_sz (1)
>
> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
> @@ -78,6 +79,7 @@
> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
> +#define NFS3_umountres_sz (1 + 1)
>
> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct rpc_rqst *req,
> encode_commit3args(xdr, args);
> }
>
> +/*
> + * UMOUNT3args
> + */
> +static void encode_umount3args(struct xdr_stream *xdr,
> + const struct nfs3_umountargs *args)
> +{
> + encode_uint32(xdr, args->dummy);
> +}
> +
> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
> + struct xdr_stream *xdr,
> + const struct nfs3_umountargs *args)
> +{
> + encode_umount3args(xdr, args);
> +}
> +
> #ifdef CONFIG_NFS_V3_ACL
>
> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
> @@ -1429,6 +1447,26 @@ out_status:
> }
>
> /*
> + * UMOUNT3res
> + */
> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
> + struct xdr_stream *xdr,
> + struct nfs3_umountres *result)
> +{
> + enum nfs_stat status;
> + int error;
> +
> + error = decode_nfsstat3(xdr, &status);
> + if (unlikely(error))
> + goto out;
> + if (status != NFS3_OK)
> + return nfs3_stat_to_errno(status);
> + error = decode_uint32(xdr, &result->dummy);
> +out:
> + return error;
> +}
> +
> +/*
> * 3.3.3 LOOKUP3res
> *
> * struct LOOKUP3resok {
> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
> PROC(FSINFO, getattr, fsinfo, 0),
> PROC(PATHCONF, getattr, pathconf, 0),
> PROC(COMMIT, commit, commit, 5),
> + PROC(UMOUNT, umount, umount, 0),
> };
>
> const struct rpc_version nfs_version3 = {
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 906f09c..9fa295b 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
> {
> struct nfs_server *server = NFS_SB(s);
>
> + int err;
> +
> + if (server->nfs_client->rpc_ops->umount) {
> + err = server->nfs_client->rpc_ops->umount(server);
> + if (err < 0) {
> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
> + __func__, err);
> + }
> + }
> +
> kill_anon_super(s);
> nfs_fscache_release_super_cookie(s);
> nfs_free_server(server);
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 9095f3c..12ca821 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -8,6 +8,7 @@
> #include <linux/ext2_fs.h>
> #include <linux/magic.h>
>
> +#include "nfsd.h"
> #include "cache.h"
> #include "xdr3.h"
> #include "vfs.h"
> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
> }
>
> /*
> + * UMOUNT call.
> + */
> +static __be32
> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs *argp,
> + struct nfsd3_umountres *resp)
> +{
> + dprintk("nfsd: UMOUNT(3)\n");
> +
> + if (nfs_umountd_workqueue)
> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
> +
> + return nfs_ok;
> +}
> +
> +/*
> * Set a file's attributes
> */
> static __be32
> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
> #define pAT (1+AT) /* post attributes - conditional */
> #define WC (7+pAT) /* WCC attributes */
>
> -static struct svc_procedure nfsd_procedures3[22] = {
> +static struct svc_procedure nfsd_procedures3[23] = {
> [NFS3PROC_NULL] = {
> .pc_func = (svc_procfunc) nfsd3_proc_null,
> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] = {
> .pc_cachetype = RC_NOCACHE,
> .pc_xdrressize = ST+WC+2,
> },
> + [NFS3PROC_UMOUNT] = {
> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
> + .pc_argsize = sizeof(struct nfsd3_umountargs),
> + .pc_ressize = sizeof(struct nfsd3_umountres),
> + .pc_cachetype = RC_NOCACHE,
> + .pc_xdrressize = ST+1,
> + },
> };
>
> struct svc_version nfsd_version3 = {
> .vs_vers = 3,
> .vs_nproc = 22,
> + .vs_nproc = 23,
> .vs_proc = nfsd_procedures3,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 43f46cd..8610282 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p,
> return xdr_argsize_check(rqstp, p);
> }
>
> +int
> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
> + struct nfsd3_umountargs *args)
> +{
> + args->dummy = ntohl(*p++);
> +
> + return xdr_argsize_check(rqstp, p);
> +}
> +
> /*
> * XDR encode functions
> */
> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p,
> return xdr_ressize_check(rqstp, p);
> }
>
> +/* UMOUNT */
> +int
> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
> + struct nfsd3_umountres *resp)
> +{
> + if (resp->status == 0)
> + *p++ = htonl(resp->dummy);
> + return xdr_ressize_check(rqstp, p);
> +}
> +
> /*
> * XDR release functions
> */
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index c55298e..53748ac 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -50,6 +50,9 @@ enum {
> #endif
> };
>
> +struct workqueue_struct *nfs_umountd_workqueue;
> +struct work_struct nfs_umount_work;
> +
> /*
> * write() for these nodes.
> */
> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
> .size = sizeof(struct nfsd_net),
> };
>
> +static void nfs_umountd_fn(struct work_struct *unused)
> +{
> + nfsd_export_flush(&init_net);
> +}
> +
> +static void nfsd_umount_destroy(void)
> +{
> + if (nfs_umountd_workqueue)
> + destroy_workqueue(nfs_umountd_workqueue);
> +}
> +
> static int __init init_nfsd(void)
> {
> int retval;
> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
> retval = register_filesystem(&nfsd_fs_type);
> if (retval)
> goto out_free_all;
> +
> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
> + if (!nfs_umountd_workqueue)
> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
> + else
> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
> +
> return 0;
> out_free_all:
> remove_proc_entry("fs/nfs/exports", NULL);
> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
>
> static void __exit exit_nfsd(void)
> {
> + nfsd_umount_destroy();
> nfsd_reply_cache_shutdown();
> remove_proc_entry("fs/nfs/exports", NULL);
> remove_proc_entry("fs/nfs", NULL);
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 1671429..691450a 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -20,6 +20,7 @@
> #include <linux/nfsd/debug.h>
> #include <linux/nfsd/export.h>
> #include <linux/nfsd/stats.h>
> +#include <linux/workqueue.h>
>
> /*
> * nfsd version
> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
> extern unsigned int nfsd_drc_mem_used;
>
> extern const struct seq_operations nfs_exports_op;
> +extern struct workqueue_struct *nfs_umountd_workqueue;
> +extern struct work_struct nfs_umount_work;
>
> /*
> * Function prototypes.
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index ee709fc..1fb3598 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
> {
> /* When last nfsd thread exits we need to do some clean-up */
> nfsd_serv = NULL;
> + if (nfs_umountd_workqueue)
> + flush_workqueue(nfs_umountd_workqueue);
> +
> nfsd_shutdown();
>
> svc_rpcb_cleanup(serv, net);
> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
> index 7df980e..b542c92 100644
> --- a/fs/nfsd/xdr3.h
> +++ b/fs/nfsd/xdr3.h
> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
> __u32 count;
> };
>
> +struct nfsd3_umountargs {
> + __u32 dummy;
> +};
> +
> struct nfsd3_getaclargs {
> struct svc_fh fh;
> int mask;
> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
> struct svc_fh fh;
> };
>
> +struct nfsd3_umountres {
> + __be32 status;
> + __u32 dummy;
> +};
> +
> struct nfsd3_getaclres {
> __be32 status;
> struct svc_fh fh;
> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *, __be32 *,
> struct nfsd3_readdirargs *);
> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
> struct nfsd3_commitargs *);
> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
> + struct nfsd3_umountargs *);
> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
> struct nfsd3_attrstat *);
> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *, __be32 *,
> struct nfsd3_pathconfres *);
> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
> struct nfsd3_commitres *);
> -
> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
> + struct nfsd3_umountres *);
> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
> struct nfsd3_attrstat *);
> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
> index 6ccfe3b..968e2e0 100644
> --- a/include/linux/nfs3.h
> +++ b/include/linux/nfs3.h
> @@ -90,6 +90,7 @@ struct nfs3_fh {
> #define NFS3PROC_FSINFO 19
> #define NFS3PROC_PATHCONF 20
> #define NFS3PROC_COMMIT 21
> +#define NFS3PROC_UMOUNT 22
>
> #define NFS_MNT3_VERSION 3
>
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 5c0014d..da53bd8 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
> struct page ** pages;
> };
>
> +struct nfs3_umountargs {
> + __u32 dummy;
> +};
> +
> +struct nfs3_umountres {
> + __u32 dummy;
> +};
> +
> struct nfs3_diropres {
> struct nfs_fattr * dir_attr;
> struct nfs_fh * fh;
> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
> struct nfs_client *
> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
> const char *, rpc_authflavor_t);
> + int (*umount)(struct nfs_server *);
> };
>
> /*
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:07 ` Myklebust, Trond
@ 2012-06-23 15:52 ` Myklebust, Trond
2012-06-23 15:57 ` Fields James
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Myklebust, Trond @ 2012-06-23 15:52 UTC (permalink / raw)
To: Namjae Jeon
Cc: Fields James, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
The other objection is that NFSv3 already has a way to do this via the MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount of a filesystem.
The problem with this approach is that if the NFS client crashes before it can call this function, then the server never gets notified. That is a problem that your patch has too...
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
On Jun 23, 2012, at 11:07 AM, Myklebust, Trond wrote:
> BIG NACK... We don't do embrace and extend in Linux...
>
>
> The whole point of the NFS protocol is to be a standard. We can't just go adding new functionality to one implementation of that standard without breaking interoperability with other implementations...
>
> Trond
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>
> On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
>
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> Without this patch:
>>
>> On NFS Client:
>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>> $ umount /mnt
>>
>> On NFS Server:
>> $ umount /mnt
>> umount: can't umount /mnt: Device or resource busy
>>
>> If user has to remove storage device user needs to unplug the
>> device.
>> This may result in file system corruption on attached media.
>>
>> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
>> removal of attached media at NFS Server.
>>
>> With this patch:
>>
>> On NFS Client:
>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>> $ umount /mnt
>>
>> On NFS Server:
>> $ umount /mnt --> umount successful
>>
>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
>> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
>> ---
>> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
>> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
>> fs/nfs/super.c | 10 ++++++++++
>> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
>> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
>> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
>> fs/nfsd/nfsd.h | 3 +++
>> fs/nfsd/nfssvc.c | 3 +++
>> fs/nfsd/xdr3.h | 14 +++++++++++++-
>> include/linux/nfs3.h | 1 +
>> include/linux/nfs_xdr.h | 9 +++++++++
>> 11 files changed, 175 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
>> index 2292a0f..726227f 100644
>> --- a/fs/nfs/nfs3proc.c
>> +++ b/fs/nfs/nfs3proc.c
>> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
>> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
>> }
>>
>> +static int nfs3_proc_umount(struct nfs_server *server)
>> +{
>> + /*
>> + * dummy argument and response are assigned to UMOUNT request
>> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
>> + */
>> + struct nfs3_umountargs arg = {
>> + .dummy = 0,
>> + };
>> + struct nfs3_umountres res;
>> +
>> + struct rpc_message msg = {
>> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
>> + .rpc_argp = &arg,
>> + .rpc_resp = &res,
>> +
>> + };
>> + int err;
>> +
>> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>> + dprintk("NFS call umount\n");
>> + err = rpc_call_sync(server->client, &msg,
>> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
>> + put_rpccred(msg.rpc_cred);
>> + dprintk("NFS reply umount: %d\n", err);
>> + return err;
>> +}
>> +
>> const struct nfs_rpc_ops nfs_v3_clientops = {
>> .version = 3, /* protocol version */
>> .dentry_ops = &nfs_dentry_operations,
>> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
>> .clear_acl_cache = nfs3_forget_cached_acls,
>> .close_context = nfs_close_context,
>> .init_client = nfs_init_client,
>> + .umount = nfs3_proc_umount,
>> };
>> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
>> index 6cbe894..874b8b2 100644
>> --- a/fs/nfs/nfs3xdr.c
>> +++ b/fs/nfs/nfs3xdr.c
>> @@ -61,6 +61,7 @@
>> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
>> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
>> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
>> +#define NFS3_umountargs_sz (1)
>>
>> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
>> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
>> @@ -78,6 +79,7 @@
>> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
>> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
>> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
>> +#define NFS3_umountres_sz (1 + 1)
>>
>> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
>> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
>> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct rpc_rqst *req,
>> encode_commit3args(xdr, args);
>> }
>>
>> +/*
>> + * UMOUNT3args
>> + */
>> +static void encode_umount3args(struct xdr_stream *xdr,
>> + const struct nfs3_umountargs *args)
>> +{
>> + encode_uint32(xdr, args->dummy);
>> +}
>> +
>> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
>> + struct xdr_stream *xdr,
>> + const struct nfs3_umountargs *args)
>> +{
>> + encode_umount3args(xdr, args);
>> +}
>> +
>> #ifdef CONFIG_NFS_V3_ACL
>>
>> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
>> @@ -1429,6 +1447,26 @@ out_status:
>> }
>>
>> /*
>> + * UMOUNT3res
>> + */
>> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
>> + struct xdr_stream *xdr,
>> + struct nfs3_umountres *result)
>> +{
>> + enum nfs_stat status;
>> + int error;
>> +
>> + error = decode_nfsstat3(xdr, &status);
>> + if (unlikely(error))
>> + goto out;
>> + if (status != NFS3_OK)
>> + return nfs3_stat_to_errno(status);
>> + error = decode_uint32(xdr, &result->dummy);
>> +out:
>> + return error;
>> +}
>> +
>> +/*
>> * 3.3.3 LOOKUP3res
>> *
>> * struct LOOKUP3resok {
>> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
>> PROC(FSINFO, getattr, fsinfo, 0),
>> PROC(PATHCONF, getattr, pathconf, 0),
>> PROC(COMMIT, commit, commit, 5),
>> + PROC(UMOUNT, umount, umount, 0),
>> };
>>
>> const struct rpc_version nfs_version3 = {
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index 906f09c..9fa295b 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
>> {
>> struct nfs_server *server = NFS_SB(s);
>>
>> + int err;
>> +
>> + if (server->nfs_client->rpc_ops->umount) {
>> + err = server->nfs_client->rpc_ops->umount(server);
>> + if (err < 0) {
>> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
>> + __func__, err);
>> + }
>> + }
>> +
>> kill_anon_super(s);
>> nfs_fscache_release_super_cookie(s);
>> nfs_free_server(server);
>> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
>> index 9095f3c..12ca821 100644
>> --- a/fs/nfsd/nfs3proc.c
>> +++ b/fs/nfsd/nfs3proc.c
>> @@ -8,6 +8,7 @@
>> #include <linux/ext2_fs.h>
>> #include <linux/magic.h>
>>
>> +#include "nfsd.h"
>> #include "cache.h"
>> #include "xdr3.h"
>> #include "vfs.h"
>> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
>> }
>>
>> /*
>> + * UMOUNT call.
>> + */
>> +static __be32
>> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs *argp,
>> + struct nfsd3_umountres *resp)
>> +{
>> + dprintk("nfsd: UMOUNT(3)\n");
>> +
>> + if (nfs_umountd_workqueue)
>> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
>> +
>> + return nfs_ok;
>> +}
>> +
>> +/*
>> * Set a file's attributes
>> */
>> static __be32
>> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
>> #define pAT (1+AT) /* post attributes - conditional */
>> #define WC (7+pAT) /* WCC attributes */
>>
>> -static struct svc_procedure nfsd_procedures3[22] = {
>> +static struct svc_procedure nfsd_procedures3[23] = {
>> [NFS3PROC_NULL] = {
>> .pc_func = (svc_procfunc) nfsd3_proc_null,
>> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
>> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] = {
>> .pc_cachetype = RC_NOCACHE,
>> .pc_xdrressize = ST+WC+2,
>> },
>> + [NFS3PROC_UMOUNT] = {
>> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
>> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
>> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
>> + .pc_argsize = sizeof(struct nfsd3_umountargs),
>> + .pc_ressize = sizeof(struct nfsd3_umountres),
>> + .pc_cachetype = RC_NOCACHE,
>> + .pc_xdrressize = ST+1,
>> + },
>> };
>>
>> struct svc_version nfsd_version3 = {
>> .vs_vers = 3,
>> .vs_nproc = 22,
>> + .vs_nproc = 23,
>> .vs_proc = nfsd_procedures3,
>> .vs_dispatch = nfsd_dispatch,
>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> index 43f46cd..8610282 100644
>> --- a/fs/nfsd/nfs3xdr.c
>> +++ b/fs/nfsd/nfs3xdr.c
>> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p,
>> return xdr_argsize_check(rqstp, p);
>> }
>>
>> +int
>> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
>> + struct nfsd3_umountargs *args)
>> +{
>> + args->dummy = ntohl(*p++);
>> +
>> + return xdr_argsize_check(rqstp, p);
>> +}
>> +
>> /*
>> * XDR encode functions
>> */
>> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p,
>> return xdr_ressize_check(rqstp, p);
>> }
>>
>> +/* UMOUNT */
>> +int
>> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
>> + struct nfsd3_umountres *resp)
>> +{
>> + if (resp->status == 0)
>> + *p++ = htonl(resp->dummy);
>> + return xdr_ressize_check(rqstp, p);
>> +}
>> +
>> /*
>> * XDR release functions
>> */
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index c55298e..53748ac 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -50,6 +50,9 @@ enum {
>> #endif
>> };
>>
>> +struct workqueue_struct *nfs_umountd_workqueue;
>> +struct work_struct nfs_umount_work;
>> +
>> /*
>> * write() for these nodes.
>> */
>> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
>> .size = sizeof(struct nfsd_net),
>> };
>>
>> +static void nfs_umountd_fn(struct work_struct *unused)
>> +{
>> + nfsd_export_flush(&init_net);
>> +}
>> +
>> +static void nfsd_umount_destroy(void)
>> +{
>> + if (nfs_umountd_workqueue)
>> + destroy_workqueue(nfs_umountd_workqueue);
>> +}
>> +
>> static int __init init_nfsd(void)
>> {
>> int retval;
>> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
>> retval = register_filesystem(&nfsd_fs_type);
>> if (retval)
>> goto out_free_all;
>> +
>> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
>> + if (!nfs_umountd_workqueue)
>> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
>> + else
>> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
>> +
>> return 0;
>> out_free_all:
>> remove_proc_entry("fs/nfs/exports", NULL);
>> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
>>
>> static void __exit exit_nfsd(void)
>> {
>> + nfsd_umount_destroy();
>> nfsd_reply_cache_shutdown();
>> remove_proc_entry("fs/nfs/exports", NULL);
>> remove_proc_entry("fs/nfs", NULL);
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 1671429..691450a 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -20,6 +20,7 @@
>> #include <linux/nfsd/debug.h>
>> #include <linux/nfsd/export.h>
>> #include <linux/nfsd/stats.h>
>> +#include <linux/workqueue.h>
>>
>> /*
>> * nfsd version
>> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
>> extern unsigned int nfsd_drc_mem_used;
>>
>> extern const struct seq_operations nfs_exports_op;
>> +extern struct workqueue_struct *nfs_umountd_workqueue;
>> +extern struct work_struct nfs_umount_work;
>>
>> /*
>> * Function prototypes.
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index ee709fc..1fb3598 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
>> {
>> /* When last nfsd thread exits we need to do some clean-up */
>> nfsd_serv = NULL;
>> + if (nfs_umountd_workqueue)
>> + flush_workqueue(nfs_umountd_workqueue);
>> +
>> nfsd_shutdown();
>>
>> svc_rpcb_cleanup(serv, net);
>> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
>> index 7df980e..b542c92 100644
>> --- a/fs/nfsd/xdr3.h
>> +++ b/fs/nfsd/xdr3.h
>> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
>> __u32 count;
>> };
>>
>> +struct nfsd3_umountargs {
>> + __u32 dummy;
>> +};
>> +
>> struct nfsd3_getaclargs {
>> struct svc_fh fh;
>> int mask;
>> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
>> struct svc_fh fh;
>> };
>>
>> +struct nfsd3_umountres {
>> + __be32 status;
>> + __u32 dummy;
>> +};
>> +
>> struct nfsd3_getaclres {
>> __be32 status;
>> struct svc_fh fh;
>> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *, __be32 *,
>> struct nfsd3_readdirargs *);
>> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
>> struct nfsd3_commitargs *);
>> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
>> + struct nfsd3_umountargs *);
>> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
>> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
>> struct nfsd3_attrstat *);
>> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *, __be32 *,
>> struct nfsd3_pathconfres *);
>> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
>> struct nfsd3_commitres *);
>> -
>> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
>> + struct nfsd3_umountres *);
>> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
>> struct nfsd3_attrstat *);
>> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
>> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
>> index 6ccfe3b..968e2e0 100644
>> --- a/include/linux/nfs3.h
>> +++ b/include/linux/nfs3.h
>> @@ -90,6 +90,7 @@ struct nfs3_fh {
>> #define NFS3PROC_FSINFO 19
>> #define NFS3PROC_PATHCONF 20
>> #define NFS3PROC_COMMIT 21
>> +#define NFS3PROC_UMOUNT 22
>>
>> #define NFS_MNT3_VERSION 3
>>
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 5c0014d..da53bd8 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
>> struct page ** pages;
>> };
>>
>> +struct nfs3_umountargs {
>> + __u32 dummy;
>> +};
>> +
>> +struct nfs3_umountres {
>> + __u32 dummy;
>> +};
>> +
>> struct nfs3_diropres {
>> struct nfs_fattr * dir_attr;
>> struct nfs_fh * fh;
>> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
>> struct nfs_client *
>> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
>> const char *, rpc_authflavor_t);
>> + int (*umount)(struct nfs_server *);
>> };
>>
>> /*
>> --
>> 1.7.9.5
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:52 ` Myklebust, Trond
@ 2012-06-23 15:57 ` Fields James
2012-06-23 16:09 ` Fields James
2012-06-23 15:59 ` Chuck Lever
2012-06-24 4:44 ` Namjae Jeon
2 siblings, 1 reply; 13+ messages in thread
From: Fields James @ 2012-06-23 15:57 UTC (permalink / raw)
To: Myklebust, Trond
Cc: Namjae Jeon, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
> The other objection is that NFSv3 already has a way to do this via the MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount of a filesystem.
>
> The problem with this approach is that if the NFS client crashes before it can call this function, then the server never gets notified. That is a problem that your patch has too...
Yes. Could you instead work on describing *exactly* what the original
problem was that prompted this?
--b.
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>
> On Jun 23, 2012, at 11:07 AM, Myklebust, Trond wrote:
>
> > BIG NACK... We don't do embrace and extend in Linux...
> >
> >
> > The whole point of the NFS protocol is to be a standard. We can't just go adding new functionality to one implementation of that standard without breaking interoperability with other implementations...
> >
> > Trond
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer
> >
> > NetApp
> > Trond.Myklebust@netapp.com
> > www.netapp.com
> >
> > On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
> >
> >> From: Namjae Jeon <namjae.jeon@samsung.com>
> >>
> >> Without this patch:
> >>
> >> On NFS Client:
> >> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
> >> $ umount /mnt
> >>
> >> On NFS Server:
> >> $ umount /mnt
> >> umount: can't umount /mnt: Device or resource busy
> >>
> >> If user has to remove storage device user needs to unplug the
> >> device.
> >> This may result in file system corruption on attached media.
> >>
> >> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
> >> removal of attached media at NFS Server.
> >>
> >> With this patch:
> >>
> >> On NFS Client:
> >> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
> >> $ umount /mnt
> >>
> >> On NFS Server:
> >> $ umount /mnt --> umount successful
> >>
> >> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> >> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
> >> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
> >> ---
> >> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
> >> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
> >> fs/nfs/super.c | 10 ++++++++++
> >> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
> >> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
> >> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
> >> fs/nfsd/nfsd.h | 3 +++
> >> fs/nfsd/nfssvc.c | 3 +++
> >> fs/nfsd/xdr3.h | 14 +++++++++++++-
> >> include/linux/nfs3.h | 1 +
> >> include/linux/nfs_xdr.h | 9 +++++++++
> >> 11 files changed, 175 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> >> index 2292a0f..726227f 100644
> >> --- a/fs/nfs/nfs3proc.c
> >> +++ b/fs/nfs/nfs3proc.c
> >> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
> >> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
> >> }
> >>
> >> +static int nfs3_proc_umount(struct nfs_server *server)
> >> +{
> >> + /*
> >> + * dummy argument and response are assigned to UMOUNT request
> >> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
> >> + */
> >> + struct nfs3_umountargs arg = {
> >> + .dummy = 0,
> >> + };
> >> + struct nfs3_umountres res;
> >> +
> >> + struct rpc_message msg = {
> >> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
> >> + .rpc_argp = &arg,
> >> + .rpc_resp = &res,
> >> +
> >> + };
> >> + int err;
> >> +
> >> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
> >> + dprintk("NFS call umount\n");
> >> + err = rpc_call_sync(server->client, &msg,
> >> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
> >> + put_rpccred(msg.rpc_cred);
> >> + dprintk("NFS reply umount: %d\n", err);
> >> + return err;
> >> +}
> >> +
> >> const struct nfs_rpc_ops nfs_v3_clientops = {
> >> .version = 3, /* protocol version */
> >> .dentry_ops = &nfs_dentry_operations,
> >> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
> >> .clear_acl_cache = nfs3_forget_cached_acls,
> >> .close_context = nfs_close_context,
> >> .init_client = nfs_init_client,
> >> + .umount = nfs3_proc_umount,
> >> };
> >> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> >> index 6cbe894..874b8b2 100644
> >> --- a/fs/nfs/nfs3xdr.c
> >> +++ b/fs/nfs/nfs3xdr.c
> >> @@ -61,6 +61,7 @@
> >> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
> >> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
> >> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
> >> +#define NFS3_umountargs_sz (1)
> >>
> >> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
> >> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
> >> @@ -78,6 +79,7 @@
> >> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
> >> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
> >> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
> >> +#define NFS3_umountres_sz (1 + 1)
> >>
> >> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
> >> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
> >> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct rpc_rqst *req,
> >> encode_commit3args(xdr, args);
> >> }
> >>
> >> +/*
> >> + * UMOUNT3args
> >> + */
> >> +static void encode_umount3args(struct xdr_stream *xdr,
> >> + const struct nfs3_umountargs *args)
> >> +{
> >> + encode_uint32(xdr, args->dummy);
> >> +}
> >> +
> >> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
> >> + struct xdr_stream *xdr,
> >> + const struct nfs3_umountargs *args)
> >> +{
> >> + encode_umount3args(xdr, args);
> >> +}
> >> +
> >> #ifdef CONFIG_NFS_V3_ACL
> >>
> >> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
> >> @@ -1429,6 +1447,26 @@ out_status:
> >> }
> >>
> >> /*
> >> + * UMOUNT3res
> >> + */
> >> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
> >> + struct xdr_stream *xdr,
> >> + struct nfs3_umountres *result)
> >> +{
> >> + enum nfs_stat status;
> >> + int error;
> >> +
> >> + error = decode_nfsstat3(xdr, &status);
> >> + if (unlikely(error))
> >> + goto out;
> >> + if (status != NFS3_OK)
> >> + return nfs3_stat_to_errno(status);
> >> + error = decode_uint32(xdr, &result->dummy);
> >> +out:
> >> + return error;
> >> +}
> >> +
> >> +/*
> >> * 3.3.3 LOOKUP3res
> >> *
> >> * struct LOOKUP3resok {
> >> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
> >> PROC(FSINFO, getattr, fsinfo, 0),
> >> PROC(PATHCONF, getattr, pathconf, 0),
> >> PROC(COMMIT, commit, commit, 5),
> >> + PROC(UMOUNT, umount, umount, 0),
> >> };
> >>
> >> const struct rpc_version nfs_version3 = {
> >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> >> index 906f09c..9fa295b 100644
> >> --- a/fs/nfs/super.c
> >> +++ b/fs/nfs/super.c
> >> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
> >> {
> >> struct nfs_server *server = NFS_SB(s);
> >>
> >> + int err;
> >> +
> >> + if (server->nfs_client->rpc_ops->umount) {
> >> + err = server->nfs_client->rpc_ops->umount(server);
> >> + if (err < 0) {
> >> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
> >> + __func__, err);
> >> + }
> >> + }
> >> +
> >> kill_anon_super(s);
> >> nfs_fscache_release_super_cookie(s);
> >> nfs_free_server(server);
> >> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> >> index 9095f3c..12ca821 100644
> >> --- a/fs/nfsd/nfs3proc.c
> >> +++ b/fs/nfsd/nfs3proc.c
> >> @@ -8,6 +8,7 @@
> >> #include <linux/ext2_fs.h>
> >> #include <linux/magic.h>
> >>
> >> +#include "nfsd.h"
> >> #include "cache.h"
> >> #include "xdr3.h"
> >> #include "vfs.h"
> >> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
> >> }
> >>
> >> /*
> >> + * UMOUNT call.
> >> + */
> >> +static __be32
> >> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs *argp,
> >> + struct nfsd3_umountres *resp)
> >> +{
> >> + dprintk("nfsd: UMOUNT(3)\n");
> >> +
> >> + if (nfs_umountd_workqueue)
> >> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
> >> +
> >> + return nfs_ok;
> >> +}
> >> +
> >> +/*
> >> * Set a file's attributes
> >> */
> >> static __be32
> >> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
> >> #define pAT (1+AT) /* post attributes - conditional */
> >> #define WC (7+pAT) /* WCC attributes */
> >>
> >> -static struct svc_procedure nfsd_procedures3[22] = {
> >> +static struct svc_procedure nfsd_procedures3[23] = {
> >> [NFS3PROC_NULL] = {
> >> .pc_func = (svc_procfunc) nfsd3_proc_null,
> >> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
> >> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] = {
> >> .pc_cachetype = RC_NOCACHE,
> >> .pc_xdrressize = ST+WC+2,
> >> },
> >> + [NFS3PROC_UMOUNT] = {
> >> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
> >> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
> >> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
> >> + .pc_argsize = sizeof(struct nfsd3_umountargs),
> >> + .pc_ressize = sizeof(struct nfsd3_umountres),
> >> + .pc_cachetype = RC_NOCACHE,
> >> + .pc_xdrressize = ST+1,
> >> + },
> >> };
> >>
> >> struct svc_version nfsd_version3 = {
> >> .vs_vers = 3,
> >> .vs_nproc = 22,
> >> + .vs_nproc = 23,
> >> .vs_proc = nfsd_procedures3,
> >> .vs_dispatch = nfsd_dispatch,
> >> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> >> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> >> index 43f46cd..8610282 100644
> >> --- a/fs/nfsd/nfs3xdr.c
> >> +++ b/fs/nfsd/nfs3xdr.c
> >> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p,
> >> return xdr_argsize_check(rqstp, p);
> >> }
> >>
> >> +int
> >> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
> >> + struct nfsd3_umountargs *args)
> >> +{
> >> + args->dummy = ntohl(*p++);
> >> +
> >> + return xdr_argsize_check(rqstp, p);
> >> +}
> >> +
> >> /*
> >> * XDR encode functions
> >> */
> >> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p,
> >> return xdr_ressize_check(rqstp, p);
> >> }
> >>
> >> +/* UMOUNT */
> >> +int
> >> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
> >> + struct nfsd3_umountres *resp)
> >> +{
> >> + if (resp->status == 0)
> >> + *p++ = htonl(resp->dummy);
> >> + return xdr_ressize_check(rqstp, p);
> >> +}
> >> +
> >> /*
> >> * XDR release functions
> >> */
> >> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> >> index c55298e..53748ac 100644
> >> --- a/fs/nfsd/nfsctl.c
> >> +++ b/fs/nfsd/nfsctl.c
> >> @@ -50,6 +50,9 @@ enum {
> >> #endif
> >> };
> >>
> >> +struct workqueue_struct *nfs_umountd_workqueue;
> >> +struct work_struct nfs_umount_work;
> >> +
> >> /*
> >> * write() for these nodes.
> >> */
> >> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
> >> .size = sizeof(struct nfsd_net),
> >> };
> >>
> >> +static void nfs_umountd_fn(struct work_struct *unused)
> >> +{
> >> + nfsd_export_flush(&init_net);
> >> +}
> >> +
> >> +static void nfsd_umount_destroy(void)
> >> +{
> >> + if (nfs_umountd_workqueue)
> >> + destroy_workqueue(nfs_umountd_workqueue);
> >> +}
> >> +
> >> static int __init init_nfsd(void)
> >> {
> >> int retval;
> >> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
> >> retval = register_filesystem(&nfsd_fs_type);
> >> if (retval)
> >> goto out_free_all;
> >> +
> >> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
> >> + if (!nfs_umountd_workqueue)
> >> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
> >> + else
> >> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
> >> +
> >> return 0;
> >> out_free_all:
> >> remove_proc_entry("fs/nfs/exports", NULL);
> >> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
> >>
> >> static void __exit exit_nfsd(void)
> >> {
> >> + nfsd_umount_destroy();
> >> nfsd_reply_cache_shutdown();
> >> remove_proc_entry("fs/nfs/exports", NULL);
> >> remove_proc_entry("fs/nfs", NULL);
> >> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> >> index 1671429..691450a 100644
> >> --- a/fs/nfsd/nfsd.h
> >> +++ b/fs/nfsd/nfsd.h
> >> @@ -20,6 +20,7 @@
> >> #include <linux/nfsd/debug.h>
> >> #include <linux/nfsd/export.h>
> >> #include <linux/nfsd/stats.h>
> >> +#include <linux/workqueue.h>
> >>
> >> /*
> >> * nfsd version
> >> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
> >> extern unsigned int nfsd_drc_mem_used;
> >>
> >> extern const struct seq_operations nfs_exports_op;
> >> +extern struct workqueue_struct *nfs_umountd_workqueue;
> >> +extern struct work_struct nfs_umount_work;
> >>
> >> /*
> >> * Function prototypes.
> >> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> >> index ee709fc..1fb3598 100644
> >> --- a/fs/nfsd/nfssvc.c
> >> +++ b/fs/nfsd/nfssvc.c
> >> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
> >> {
> >> /* When last nfsd thread exits we need to do some clean-up */
> >> nfsd_serv = NULL;
> >> + if (nfs_umountd_workqueue)
> >> + flush_workqueue(nfs_umountd_workqueue);
> >> +
> >> nfsd_shutdown();
> >>
> >> svc_rpcb_cleanup(serv, net);
> >> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
> >> index 7df980e..b542c92 100644
> >> --- a/fs/nfsd/xdr3.h
> >> +++ b/fs/nfsd/xdr3.h
> >> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
> >> __u32 count;
> >> };
> >>
> >> +struct nfsd3_umountargs {
> >> + __u32 dummy;
> >> +};
> >> +
> >> struct nfsd3_getaclargs {
> >> struct svc_fh fh;
> >> int mask;
> >> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
> >> struct svc_fh fh;
> >> };
> >>
> >> +struct nfsd3_umountres {
> >> + __be32 status;
> >> + __u32 dummy;
> >> +};
> >> +
> >> struct nfsd3_getaclres {
> >> __be32 status;
> >> struct svc_fh fh;
> >> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *, __be32 *,
> >> struct nfsd3_readdirargs *);
> >> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
> >> struct nfsd3_commitargs *);
> >> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
> >> + struct nfsd3_umountargs *);
> >> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
> >> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
> >> struct nfsd3_attrstat *);
> >> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *, __be32 *,
> >> struct nfsd3_pathconfres *);
> >> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
> >> struct nfsd3_commitres *);
> >> -
> >> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
> >> + struct nfsd3_umountres *);
> >> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
> >> struct nfsd3_attrstat *);
> >> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
> >> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
> >> index 6ccfe3b..968e2e0 100644
> >> --- a/include/linux/nfs3.h
> >> +++ b/include/linux/nfs3.h
> >> @@ -90,6 +90,7 @@ struct nfs3_fh {
> >> #define NFS3PROC_FSINFO 19
> >> #define NFS3PROC_PATHCONF 20
> >> #define NFS3PROC_COMMIT 21
> >> +#define NFS3PROC_UMOUNT 22
> >>
> >> #define NFS_MNT3_VERSION 3
> >>
> >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> >> index 5c0014d..da53bd8 100644
> >> --- a/include/linux/nfs_xdr.h
> >> +++ b/include/linux/nfs_xdr.h
> >> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
> >> struct page ** pages;
> >> };
> >>
> >> +struct nfs3_umountargs {
> >> + __u32 dummy;
> >> +};
> >> +
> >> +struct nfs3_umountres {
> >> + __u32 dummy;
> >> +};
> >> +
> >> struct nfs3_diropres {
> >> struct nfs_fattr * dir_attr;
> >> struct nfs_fh * fh;
> >> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
> >> struct nfs_client *
> >> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
> >> const char *, rpc_authflavor_t);
> >> + int (*umount)(struct nfs_server *);
> >> };
> >>
> >> /*
> >> --
> >> 1.7.9.5
> >>
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:52 ` Myklebust, Trond
2012-06-23 15:57 ` Fields James
@ 2012-06-23 15:59 ` Chuck Lever
2012-06-24 4:49 ` Namjae Jeon
2012-06-24 4:44 ` Namjae Jeon
2 siblings, 1 reply; 13+ messages in thread
From: Chuck Lever @ 2012-06-23 15:59 UTC (permalink / raw)
To: Namjae Jeon
Cc: Trond Myklebust, Fields James, Andrew Morton, Harrosh Boaz,
Linux NFS Mailing List, linux-kernel, Namjae Jeon, Vivek Trivedi,
Amit Sahrawat
On Jun 23, 2012, at 11:52 AM, Myklebust, Trond wrote:
> The other objection is that NFSv3 already has a way to do this via the MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount of a filesystem.
>
> The problem with this approach is that if the NFS client crashes before it can call this function, then the server never gets notified. That is a problem that your patch has too...
Indeed. That could be addressed, however, by having the server scrub its rmtab when it receives an SM_NOTIFY after a client reboots. The client will only send an SM_NOTIFY, however, if it held NLM locks on that server.
There is also a UMNTALL procedure in the MOUNT protocol that a client could send to servers on reboot if clients kept track on permanent storage of servers they mounted.
None of these approaches is terribly reliable, though, and the issue is addressed in NFSv4, which is lease-based. You should probably focus on addressing this on the server itself.
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>
> On Jun 23, 2012, at 11:07 AM, Myklebust, Trond wrote:
>
>> BIG NACK... We don't do embrace and extend in Linux...
>>
>>
>> The whole point of the NFS protocol is to be a standard. We can't just go adding new functionality to one implementation of that standard without breaking interoperability with other implementations...
>>
>> Trond
>>
>> --
>> Trond Myklebust
>> Linux NFS client maintainer
>>
>> NetApp
>> Trond.Myklebust@netapp.com
>> www.netapp.com
>>
>> On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
>>
>>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>>
>>> Without this patch:
>>>
>>> On NFS Client:
>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>> $ umount /mnt
>>>
>>> On NFS Server:
>>> $ umount /mnt
>>> umount: can't umount /mnt: Device or resource busy
>>>
>>> If user has to remove storage device user needs to unplug the
>>> device.
>>> This may result in file system corruption on attached media.
>>>
>>> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
>>> removal of attached media at NFS Server.
>>>
>>> With this patch:
>>>
>>> On NFS Client:
>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>> $ umount /mnt
>>>
>>> On NFS Server:
>>> $ umount /mnt --> umount successful
>>>
>>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>>> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
>>> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
>>> ---
>>> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
>>> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> fs/nfs/super.c | 10 ++++++++++
>>> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
>>> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
>>> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
>>> fs/nfsd/nfsd.h | 3 +++
>>> fs/nfsd/nfssvc.c | 3 +++
>>> fs/nfsd/xdr3.h | 14 +++++++++++++-
>>> include/linux/nfs3.h | 1 +
>>> include/linux/nfs_xdr.h | 9 +++++++++
>>> 11 files changed, 175 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
>>> index 2292a0f..726227f 100644
>>> --- a/fs/nfs/nfs3proc.c
>>> +++ b/fs/nfs/nfs3proc.c
>>> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
>>> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
>>> }
>>>
>>> +static int nfs3_proc_umount(struct nfs_server *server)
>>> +{
>>> + /*
>>> + * dummy argument and response are assigned to UMOUNT request
>>> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
>>> + */
>>> + struct nfs3_umountargs arg = {
>>> + .dummy = 0,
>>> + };
>>> + struct nfs3_umountres res;
>>> +
>>> + struct rpc_message msg = {
>>> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
>>> + .rpc_argp = &arg,
>>> + .rpc_resp = &res,
>>> +
>>> + };
>>> + int err;
>>> +
>>> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>>> + dprintk("NFS call umount\n");
>>> + err = rpc_call_sync(server->client, &msg,
>>> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
>>> + put_rpccred(msg.rpc_cred);
>>> + dprintk("NFS reply umount: %d\n", err);
>>> + return err;
>>> +}
>>> +
>>> const struct nfs_rpc_ops nfs_v3_clientops = {
>>> .version = 3, /* protocol version */
>>> .dentry_ops = &nfs_dentry_operations,
>>> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
>>> .clear_acl_cache = nfs3_forget_cached_acls,
>>> .close_context = nfs_close_context,
>>> .init_client = nfs_init_client,
>>> + .umount = nfs3_proc_umount,
>>> };
>>> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
>>> index 6cbe894..874b8b2 100644
>>> --- a/fs/nfs/nfs3xdr.c
>>> +++ b/fs/nfs/nfs3xdr.c
>>> @@ -61,6 +61,7 @@
>>> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
>>> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
>>> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
>>> +#define NFS3_umountargs_sz (1)
>>>
>>> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
>>> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
>>> @@ -78,6 +79,7 @@
>>> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
>>> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
>>> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
>>> +#define NFS3_umountres_sz (1 + 1)
>>>
>>> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
>>> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
>>> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct rpc_rqst *req,
>>> encode_commit3args(xdr, args);
>>> }
>>>
>>> +/*
>>> + * UMOUNT3args
>>> + */
>>> +static void encode_umount3args(struct xdr_stream *xdr,
>>> + const struct nfs3_umountargs *args)
>>> +{
>>> + encode_uint32(xdr, args->dummy);
>>> +}
>>> +
>>> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
>>> + struct xdr_stream *xdr,
>>> + const struct nfs3_umountargs *args)
>>> +{
>>> + encode_umount3args(xdr, args);
>>> +}
>>> +
>>> #ifdef CONFIG_NFS_V3_ACL
>>>
>>> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
>>> @@ -1429,6 +1447,26 @@ out_status:
>>> }
>>>
>>> /*
>>> + * UMOUNT3res
>>> + */
>>> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
>>> + struct xdr_stream *xdr,
>>> + struct nfs3_umountres *result)
>>> +{
>>> + enum nfs_stat status;
>>> + int error;
>>> +
>>> + error = decode_nfsstat3(xdr, &status);
>>> + if (unlikely(error))
>>> + goto out;
>>> + if (status != NFS3_OK)
>>> + return nfs3_stat_to_errno(status);
>>> + error = decode_uint32(xdr, &result->dummy);
>>> +out:
>>> + return error;
>>> +}
>>> +
>>> +/*
>>> * 3.3.3 LOOKUP3res
>>> *
>>> * struct LOOKUP3resok {
>>> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
>>> PROC(FSINFO, getattr, fsinfo, 0),
>>> PROC(PATHCONF, getattr, pathconf, 0),
>>> PROC(COMMIT, commit, commit, 5),
>>> + PROC(UMOUNT, umount, umount, 0),
>>> };
>>>
>>> const struct rpc_version nfs_version3 = {
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index 906f09c..9fa295b 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
>>> {
>>> struct nfs_server *server = NFS_SB(s);
>>>
>>> + int err;
>>> +
>>> + if (server->nfs_client->rpc_ops->umount) {
>>> + err = server->nfs_client->rpc_ops->umount(server);
>>> + if (err < 0) {
>>> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
>>> + __func__, err);
>>> + }
>>> + }
>>> +
>>> kill_anon_super(s);
>>> nfs_fscache_release_super_cookie(s);
>>> nfs_free_server(server);
>>> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
>>> index 9095f3c..12ca821 100644
>>> --- a/fs/nfsd/nfs3proc.c
>>> +++ b/fs/nfsd/nfs3proc.c
>>> @@ -8,6 +8,7 @@
>>> #include <linux/ext2_fs.h>
>>> #include <linux/magic.h>
>>>
>>> +#include "nfsd.h"
>>> #include "cache.h"
>>> #include "xdr3.h"
>>> #include "vfs.h"
>>> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
>>> }
>>>
>>> /*
>>> + * UMOUNT call.
>>> + */
>>> +static __be32
>>> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs *argp,
>>> + struct nfsd3_umountres *resp)
>>> +{
>>> + dprintk("nfsd: UMOUNT(3)\n");
>>> +
>>> + if (nfs_umountd_workqueue)
>>> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
>>> +
>>> + return nfs_ok;
>>> +}
>>> +
>>> +/*
>>> * Set a file's attributes
>>> */
>>> static __be32
>>> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
>>> #define pAT (1+AT) /* post attributes - conditional */
>>> #define WC (7+pAT) /* WCC attributes */
>>>
>>> -static struct svc_procedure nfsd_procedures3[22] = {
>>> +static struct svc_procedure nfsd_procedures3[23] = {
>>> [NFS3PROC_NULL] = {
>>> .pc_func = (svc_procfunc) nfsd3_proc_null,
>>> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
>>> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] = {
>>> .pc_cachetype = RC_NOCACHE,
>>> .pc_xdrressize = ST+WC+2,
>>> },
>>> + [NFS3PROC_UMOUNT] = {
>>> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
>>> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
>>> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
>>> + .pc_argsize = sizeof(struct nfsd3_umountargs),
>>> + .pc_ressize = sizeof(struct nfsd3_umountres),
>>> + .pc_cachetype = RC_NOCACHE,
>>> + .pc_xdrressize = ST+1,
>>> + },
>>> };
>>>
>>> struct svc_version nfsd_version3 = {
>>> .vs_vers = 3,
>>> .vs_nproc = 22,
>>> + .vs_nproc = 23,
>>> .vs_proc = nfsd_procedures3,
>>> .vs_dispatch = nfsd_dispatch,
>>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>>> index 43f46cd..8610282 100644
>>> --- a/fs/nfsd/nfs3xdr.c
>>> +++ b/fs/nfsd/nfs3xdr.c
>>> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p,
>>> return xdr_argsize_check(rqstp, p);
>>> }
>>>
>>> +int
>>> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
>>> + struct nfsd3_umountargs *args)
>>> +{
>>> + args->dummy = ntohl(*p++);
>>> +
>>> + return xdr_argsize_check(rqstp, p);
>>> +}
>>> +
>>> /*
>>> * XDR encode functions
>>> */
>>> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p,
>>> return xdr_ressize_check(rqstp, p);
>>> }
>>>
>>> +/* UMOUNT */
>>> +int
>>> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
>>> + struct nfsd3_umountres *resp)
>>> +{
>>> + if (resp->status == 0)
>>> + *p++ = htonl(resp->dummy);
>>> + return xdr_ressize_check(rqstp, p);
>>> +}
>>> +
>>> /*
>>> * XDR release functions
>>> */
>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>> index c55298e..53748ac 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -50,6 +50,9 @@ enum {
>>> #endif
>>> };
>>>
>>> +struct workqueue_struct *nfs_umountd_workqueue;
>>> +struct work_struct nfs_umount_work;
>>> +
>>> /*
>>> * write() for these nodes.
>>> */
>>> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
>>> .size = sizeof(struct nfsd_net),
>>> };
>>>
>>> +static void nfs_umountd_fn(struct work_struct *unused)
>>> +{
>>> + nfsd_export_flush(&init_net);
>>> +}
>>> +
>>> +static void nfsd_umount_destroy(void)
>>> +{
>>> + if (nfs_umountd_workqueue)
>>> + destroy_workqueue(nfs_umountd_workqueue);
>>> +}
>>> +
>>> static int __init init_nfsd(void)
>>> {
>>> int retval;
>>> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
>>> retval = register_filesystem(&nfsd_fs_type);
>>> if (retval)
>>> goto out_free_all;
>>> +
>>> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
>>> + if (!nfs_umountd_workqueue)
>>> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
>>> + else
>>> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
>>> +
>>> return 0;
>>> out_free_all:
>>> remove_proc_entry("fs/nfs/exports", NULL);
>>> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
>>>
>>> static void __exit exit_nfsd(void)
>>> {
>>> + nfsd_umount_destroy();
>>> nfsd_reply_cache_shutdown();
>>> remove_proc_entry("fs/nfs/exports", NULL);
>>> remove_proc_entry("fs/nfs", NULL);
>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>> index 1671429..691450a 100644
>>> --- a/fs/nfsd/nfsd.h
>>> +++ b/fs/nfsd/nfsd.h
>>> @@ -20,6 +20,7 @@
>>> #include <linux/nfsd/debug.h>
>>> #include <linux/nfsd/export.h>
>>> #include <linux/nfsd/stats.h>
>>> +#include <linux/workqueue.h>
>>>
>>> /*
>>> * nfsd version
>>> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
>>> extern unsigned int nfsd_drc_mem_used;
>>>
>>> extern const struct seq_operations nfs_exports_op;
>>> +extern struct workqueue_struct *nfs_umountd_workqueue;
>>> +extern struct work_struct nfs_umount_work;
>>>
>>> /*
>>> * Function prototypes.
>>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>>> index ee709fc..1fb3598 100644
>>> --- a/fs/nfsd/nfssvc.c
>>> +++ b/fs/nfsd/nfssvc.c
>>> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
>>> {
>>> /* When last nfsd thread exits we need to do some clean-up */
>>> nfsd_serv = NULL;
>>> + if (nfs_umountd_workqueue)
>>> + flush_workqueue(nfs_umountd_workqueue);
>>> +
>>> nfsd_shutdown();
>>>
>>> svc_rpcb_cleanup(serv, net);
>>> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
>>> index 7df980e..b542c92 100644
>>> --- a/fs/nfsd/xdr3.h
>>> +++ b/fs/nfsd/xdr3.h
>>> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
>>> __u32 count;
>>> };
>>>
>>> +struct nfsd3_umountargs {
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfsd3_getaclargs {
>>> struct svc_fh fh;
>>> int mask;
>>> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
>>> struct svc_fh fh;
>>> };
>>>
>>> +struct nfsd3_umountres {
>>> + __be32 status;
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfsd3_getaclres {
>>> __be32 status;
>>> struct svc_fh fh;
>>> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *, __be32 *,
>>> struct nfsd3_readdirargs *);
>>> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
>>> struct nfsd3_commitargs *);
>>> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
>>> + struct nfsd3_umountargs *);
>>> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
>>> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
>>> struct nfsd3_attrstat *);
>>> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *, __be32 *,
>>> struct nfsd3_pathconfres *);
>>> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
>>> struct nfsd3_commitres *);
>>> -
>>> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
>>> + struct nfsd3_umountres *);
>>> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
>>> struct nfsd3_attrstat *);
>>> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
>>> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
>>> index 6ccfe3b..968e2e0 100644
>>> --- a/include/linux/nfs3.h
>>> +++ b/include/linux/nfs3.h
>>> @@ -90,6 +90,7 @@ struct nfs3_fh {
>>> #define NFS3PROC_FSINFO 19
>>> #define NFS3PROC_PATHCONF 20
>>> #define NFS3PROC_COMMIT 21
>>> +#define NFS3PROC_UMOUNT 22
>>>
>>> #define NFS_MNT3_VERSION 3
>>>
>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>> index 5c0014d..da53bd8 100644
>>> --- a/include/linux/nfs_xdr.h
>>> +++ b/include/linux/nfs_xdr.h
>>> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
>>> struct page ** pages;
>>> };
>>>
>>> +struct nfs3_umountargs {
>>> + __u32 dummy;
>>> +};
>>> +
>>> +struct nfs3_umountres {
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfs3_diropres {
>>> struct nfs_fattr * dir_attr;
>>> struct nfs_fh * fh;
>>> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
>>> struct nfs_client *
>>> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
>>> const char *, rpc_authflavor_t);
>>> + int (*umount)(struct nfs_server *);
>>> };
>>>
>>> /*
>>> --
>>> 1.7.9.5
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:57 ` Fields James
@ 2012-06-23 16:09 ` Fields James
2012-06-24 5:12 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Fields James @ 2012-06-23 16:09 UTC (permalink / raw)
To: Myklebust, Trond
Cc: Namjae Jeon, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
> > The other objection is that NFSv3 already has a way to do this via the MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount of a filesystem.
> >
> > The problem with this approach is that if the NFS client crashes before it can call this function, then the server never gets notified. That is a problem that your patch has too...
>
> Yes. Could you instead work on describing *exactly* what the original
> problem was that prompted this?
("You" here meaning Namjae Jeon, of course).
And note here we've all read the changelog, that's not what I'm asking
for.
I want to know the details of *your* specific case. "I have X clients
that I use for Y, periodically I want to unmount the exported filesystem
to do Z, I tried doing W but it didn't work...".
--b.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:52 ` Myklebust, Trond
2012-06-23 15:57 ` Fields James
2012-06-23 15:59 ` Chuck Lever
@ 2012-06-24 4:44 ` Namjae Jeon
2 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2012-06-24 4:44 UTC (permalink / raw)
To: Myklebust, Trond
Cc: Fields James, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
2012/6/24, Myklebust, Trond <Trond.Myklebust@netapp.com>:
> The other objection is that NFSv3 already has a way to do this via the MOUNT
> protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount of a
> filesystem.
I will check current MOUNTPROC3_UMNT/MOUNTPROC_UMNT.
>
> The problem with this approach is that if the NFS client crashes before it
> can call this function, then the server never gets notified. That is a
> problem that your patch has too...
You're right. Good point.. Acutally, I have been concerning about this.
Thanks.
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>
> On Jun 23, 2012, at 11:07 AM, Myklebust, Trond wrote:
>
>> BIG NACK... We don't do embrace and extend in Linux...
>>
>>
>> The whole point of the NFS protocol is to be a standard. We can't just go
>> adding new functionality to one implementation of that standard without
>> breaking interoperability with other implementations...
>>
>> Trond
>>
>> --
>> Trond Myklebust
>> Linux NFS client maintainer
>>
>> NetApp
>> Trond.Myklebust@netapp.com
>> www.netapp.com
>>
>> On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
>>
>>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>>
>>> Without this patch:
>>>
>>> On NFS Client:
>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>> $ umount /mnt
>>>
>>> On NFS Server:
>>> $ umount /mnt
>>> umount: can't umount /mnt: Device or resource busy
>>>
>>> If user has to remove storage device user needs to unplug the
>>> device.
>>> This may result in file system corruption on attached media.
>>>
>>> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
>>> removal of attached media at NFS Server.
>>>
>>> With this patch:
>>>
>>> On NFS Client:
>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>> $ umount /mnt
>>>
>>> On NFS Server:
>>> $ umount /mnt --> umount successful
>>>
>>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>>> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
>>> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
>>> ---
>>> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
>>> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> fs/nfs/super.c | 10 ++++++++++
>>> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
>>> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
>>> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
>>> fs/nfsd/nfsd.h | 3 +++
>>> fs/nfsd/nfssvc.c | 3 +++
>>> fs/nfsd/xdr3.h | 14 +++++++++++++-
>>> include/linux/nfs3.h | 1 +
>>> include/linux/nfs_xdr.h | 9 +++++++++
>>> 11 files changed, 175 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
>>> index 2292a0f..726227f 100644
>>> --- a/fs/nfs/nfs3proc.c
>>> +++ b/fs/nfs/nfs3proc.c
>>> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct
>>> file_lock *fl)
>>> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
>>> }
>>>
>>> +static int nfs3_proc_umount(struct nfs_server *server)
>>> +{
>>> + /*
>>> + * dummy argument and response are assigned to UMOUNT request
>>> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
>>> + */
>>> + struct nfs3_umountargs arg = {
>>> + .dummy = 0,
>>> + };
>>> + struct nfs3_umountres res;
>>> +
>>> + struct rpc_message msg = {
>>> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
>>> + .rpc_argp = &arg,
>>> + .rpc_resp = &res,
>>> +
>>> + };
>>> + int err;
>>> +
>>> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>>> + dprintk("NFS call umount\n");
>>> + err = rpc_call_sync(server->client, &msg,
>>> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
>>> + put_rpccred(msg.rpc_cred);
>>> + dprintk("NFS reply umount: %d\n", err);
>>> + return err;
>>> +}
>>> +
>>> const struct nfs_rpc_ops nfs_v3_clientops = {
>>> .version = 3, /* protocol version */
>>> .dentry_ops = &nfs_dentry_operations,
>>> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
>>> .clear_acl_cache = nfs3_forget_cached_acls,
>>> .close_context = nfs_close_context,
>>> .init_client = nfs_init_client,
>>> + .umount = nfs3_proc_umount,
>>> };
>>> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
>>> index 6cbe894..874b8b2 100644
>>> --- a/fs/nfs/nfs3xdr.c
>>> +++ b/fs/nfs/nfs3xdr.c
>>> @@ -61,6 +61,7 @@
>>> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
>>> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
>>> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
>>> +#define NFS3_umountargs_sz (1)
>>>
>>> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
>>> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
>>> @@ -78,6 +79,7 @@
>>> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
>>> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
>>> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
>>> +#define NFS3_umountres_sz (1 + 1)
>>>
>>> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
>>> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
>>> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct
>>> rpc_rqst *req,
>>> encode_commit3args(xdr, args);
>>> }
>>>
>>> +/*
>>> + * UMOUNT3args
>>> + */
>>> +static void encode_umount3args(struct xdr_stream *xdr,
>>> + const struct nfs3_umountargs *args)
>>> +{
>>> + encode_uint32(xdr, args->dummy);
>>> +}
>>> +
>>> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
>>> + struct xdr_stream *xdr,
>>> + const struct nfs3_umountargs *args)
>>> +{
>>> + encode_umount3args(xdr, args);
>>> +}
>>> +
>>> #ifdef CONFIG_NFS_V3_ACL
>>>
>>> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
>>> @@ -1429,6 +1447,26 @@ out_status:
>>> }
>>>
>>> /*
>>> + * UMOUNT3res
>>> + */
>>> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
>>> + struct xdr_stream *xdr,
>>> + struct nfs3_umountres *result)
>>> +{
>>> + enum nfs_stat status;
>>> + int error;
>>> +
>>> + error = decode_nfsstat3(xdr, &status);
>>> + if (unlikely(error))
>>> + goto out;
>>> + if (status != NFS3_OK)
>>> + return nfs3_stat_to_errno(status);
>>> + error = decode_uint32(xdr, &result->dummy);
>>> +out:
>>> + return error;
>>> +}
>>> +
>>> +/*
>>> * 3.3.3 LOOKUP3res
>>> *
>>> * struct LOOKUP3resok {
>>> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
>>> PROC(FSINFO, getattr, fsinfo, 0),
>>> PROC(PATHCONF, getattr, pathconf, 0),
>>> PROC(COMMIT, commit, commit, 5),
>>> + PROC(UMOUNT, umount, umount, 0),
>>> };
>>>
>>> const struct rpc_version nfs_version3 = {
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index 906f09c..9fa295b 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block *s)
>>> {
>>> struct nfs_server *server = NFS_SB(s);
>>>
>>> + int err;
>>> +
>>> + if (server->nfs_client->rpc_ops->umount) {
>>> + err = server->nfs_client->rpc_ops->umount(server);
>>> + if (err < 0) {
>>> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
>>> + __func__, err);
>>> + }
>>> + }
>>> +
>>> kill_anon_super(s);
>>> nfs_fscache_release_super_cookie(s);
>>> nfs_free_server(server);
>>> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
>>> index 9095f3c..12ca821 100644
>>> --- a/fs/nfsd/nfs3proc.c
>>> +++ b/fs/nfsd/nfs3proc.c
>>> @@ -8,6 +8,7 @@
>>> #include <linux/ext2_fs.h>
>>> #include <linux/magic.h>
>>>
>>> +#include "nfsd.h"
>>> #include "cache.h"
>>> #include "xdr3.h"
>>> #include "vfs.h"
>>> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct
>>> nfsd_fhandle *argp,
>>> }
>>>
>>> /*
>>> + * UMOUNT call.
>>> + */
>>> +static __be32
>>> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs
>>> *argp,
>>> + struct nfsd3_umountres *resp)
>>> +{
>>> + dprintk("nfsd: UMOUNT(3)\n");
>>> +
>>> + if (nfs_umountd_workqueue)
>>> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
>>> +
>>> + return nfs_ok;
>>> +}
>>> +
>>> +/*
>>> * Set a file's attributes
>>> */
>>> static __be32
>>> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
>>> #define pAT (1+AT) /* post attributes - conditional */
>>> #define WC (7+pAT) /* WCC attributes */
>>>
>>> -static struct svc_procedure nfsd_procedures3[22] = {
>>> +static struct svc_procedure nfsd_procedures3[23] = {
>>> [NFS3PROC_NULL] = {
>>> .pc_func = (svc_procfunc) nfsd3_proc_null,
>>> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
>>> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22] =
>>> {
>>> .pc_cachetype = RC_NOCACHE,
>>> .pc_xdrressize = ST+WC+2,
>>> },
>>> + [NFS3PROC_UMOUNT] = {
>>> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
>>> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
>>> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
>>> + .pc_argsize = sizeof(struct nfsd3_umountargs),
>>> + .pc_ressize = sizeof(struct nfsd3_umountres),
>>> + .pc_cachetype = RC_NOCACHE,
>>> + .pc_xdrressize = ST+1,
>>> + },
>>> };
>>>
>>> struct svc_version nfsd_version3 = {
>>> .vs_vers = 3,
>>> .vs_nproc = 22,
>>> + .vs_nproc = 23,
>>> .vs_proc = nfsd_procedures3,
>>> .vs_dispatch = nfsd_dispatch,
>>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>>> index 43f46cd..8610282 100644
>>> --- a/fs/nfsd/nfs3xdr.c
>>> +++ b/fs/nfsd/nfs3xdr.c
>>> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp,
>>> __be32 *p,
>>> return xdr_argsize_check(rqstp, p);
>>> }
>>>
>>> +int
>>> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
>>> + struct nfsd3_umountargs *args)
>>> +{
>>> + args->dummy = ntohl(*p++);
>>> +
>>> + return xdr_argsize_check(rqstp, p);
>>> +}
>>> +
>>> /*
>>> * XDR encode functions
>>> */
>>> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp,
>>> __be32 *p,
>>> return xdr_ressize_check(rqstp, p);
>>> }
>>>
>>> +/* UMOUNT */
>>> +int
>>> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
>>> + struct nfsd3_umountres *resp)
>>> +{
>>> + if (resp->status == 0)
>>> + *p++ = htonl(resp->dummy);
>>> + return xdr_ressize_check(rqstp, p);
>>> +}
>>> +
>>> /*
>>> * XDR release functions
>>> */
>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>> index c55298e..53748ac 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -50,6 +50,9 @@ enum {
>>> #endif
>>> };
>>>
>>> +struct workqueue_struct *nfs_umountd_workqueue;
>>> +struct work_struct nfs_umount_work;
>>> +
>>> /*
>>> * write() for these nodes.
>>> */
>>> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
>>> .size = sizeof(struct nfsd_net),
>>> };
>>>
>>> +static void nfs_umountd_fn(struct work_struct *unused)
>>> +{
>>> + nfsd_export_flush(&init_net);
>>> +}
>>> +
>>> +static void nfsd_umount_destroy(void)
>>> +{
>>> + if (nfs_umountd_workqueue)
>>> + destroy_workqueue(nfs_umountd_workqueue);
>>> +}
>>> +
>>> static int __init init_nfsd(void)
>>> {
>>> int retval;
>>> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
>>> retval = register_filesystem(&nfsd_fs_type);
>>> if (retval)
>>> goto out_free_all;
>>> +
>>> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
>>> + if (!nfs_umountd_workqueue)
>>> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
>>> + else
>>> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
>>> +
>>> return 0;
>>> out_free_all:
>>> remove_proc_entry("fs/nfs/exports", NULL);
>>> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
>>>
>>> static void __exit exit_nfsd(void)
>>> {
>>> + nfsd_umount_destroy();
>>> nfsd_reply_cache_shutdown();
>>> remove_proc_entry("fs/nfs/exports", NULL);
>>> remove_proc_entry("fs/nfs", NULL);
>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>> index 1671429..691450a 100644
>>> --- a/fs/nfsd/nfsd.h
>>> +++ b/fs/nfsd/nfsd.h
>>> @@ -20,6 +20,7 @@
>>> #include <linux/nfsd/debug.h>
>>> #include <linux/nfsd/export.h>
>>> #include <linux/nfsd/stats.h>
>>> +#include <linux/workqueue.h>
>>>
>>> /*
>>> * nfsd version
>>> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
>>> extern unsigned int nfsd_drc_mem_used;
>>>
>>> extern const struct seq_operations nfs_exports_op;
>>> +extern struct workqueue_struct *nfs_umountd_workqueue;
>>> +extern struct work_struct nfs_umount_work;
>>>
>>> /*
>>> * Function prototypes.
>>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>>> index ee709fc..1fb3598 100644
>>> --- a/fs/nfsd/nfssvc.c
>>> +++ b/fs/nfsd/nfssvc.c
>>> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv,
>>> struct net *net)
>>> {
>>> /* When last nfsd thread exits we need to do some clean-up */
>>> nfsd_serv = NULL;
>>> + if (nfs_umountd_workqueue)
>>> + flush_workqueue(nfs_umountd_workqueue);
>>> +
>>> nfsd_shutdown();
>>>
>>> svc_rpcb_cleanup(serv, net);
>>> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
>>> index 7df980e..b542c92 100644
>>> --- a/fs/nfsd/xdr3.h
>>> +++ b/fs/nfsd/xdr3.h
>>> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
>>> __u32 count;
>>> };
>>>
>>> +struct nfsd3_umountargs {
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfsd3_getaclargs {
>>> struct svc_fh fh;
>>> int mask;
>>> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
>>> struct svc_fh fh;
>>> };
>>>
>>> +struct nfsd3_umountres {
>>> + __be32 status;
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfsd3_getaclres {
>>> __be32 status;
>>> struct svc_fh fh;
>>> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst *,
>>> __be32 *,
>>> struct nfsd3_readdirargs *);
>>> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
>>> struct nfsd3_commitargs *);
>>> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
>>> + struct nfsd3_umountargs *);
>>> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
>>> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
>>> struct nfsd3_attrstat *);
>>> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *,
>>> __be32 *,
>>> struct nfsd3_pathconfres *);
>>> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
>>> struct nfsd3_commitres *);
>>> -
>>> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
>>> + struct nfsd3_umountres *);
>>> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
>>> struct nfsd3_attrstat *);
>>> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
>>> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
>>> index 6ccfe3b..968e2e0 100644
>>> --- a/include/linux/nfs3.h
>>> +++ b/include/linux/nfs3.h
>>> @@ -90,6 +90,7 @@ struct nfs3_fh {
>>> #define NFS3PROC_FSINFO 19
>>> #define NFS3PROC_PATHCONF 20
>>> #define NFS3PROC_COMMIT 21
>>> +#define NFS3PROC_UMOUNT 22
>>>
>>> #define NFS_MNT3_VERSION 3
>>>
>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>> index 5c0014d..da53bd8 100644
>>> --- a/include/linux/nfs_xdr.h
>>> +++ b/include/linux/nfs_xdr.h
>>> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
>>> struct page ** pages;
>>> };
>>>
>>> +struct nfs3_umountargs {
>>> + __u32 dummy;
>>> +};
>>> +
>>> +struct nfs3_umountres {
>>> + __u32 dummy;
>>> +};
>>> +
>>> struct nfs3_diropres {
>>> struct nfs_fattr * dir_attr;
>>> struct nfs_fh * fh;
>>> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
>>> struct nfs_client *
>>> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
>>> const char *, rpc_authflavor_t);
>>> + int (*umount)(struct nfs_server *);
>>> };
>>>
>>> /*
>>> --
>>> 1.7.9.5
>>>
>>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 15:59 ` Chuck Lever
@ 2012-06-24 4:49 ` Namjae Jeon
0 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2012-06-24 4:49 UTC (permalink / raw)
To: Chuck Lever
Cc: Trond Myklebust, Fields James, Andrew Morton, Harrosh Boaz,
Linux NFS Mailing List, linux-kernel, Namjae Jeon, Vivek Trivedi,
Amit Sahrawat
2012/6/24, Chuck Lever <chuck.lever@oracle.com>:
>
> On Jun 23, 2012, at 11:52 AM, Myklebust, Trond wrote:
>
>> The other objection is that NFSv3 already has a way to do this via the
>> MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on unmount
>> of a filesystem.
>>
>> The problem with this approach is that if the NFS client crashes before it
>> can call this function, then the server never gets notified. That is a
>> problem that your patch has too...
>
> Indeed. That could be addressed, however, by having the server scrub its
> rmtab when it receives an SM_NOTIFY after a client reboots. The client will
> only send an SM_NOTIFY, however, if it held NLM locks on that server.
>
> There is also a UMNTALL procedure in the MOUNT protocol that a client could
> send to servers on reboot if clients kept track on permanent storage of
> servers they mounted.
>
> None of these approaches is terribly reliable, though, and the issue is
> addressed in NFSv4, which is lease-based. You should probably focus on
> addressing this on the server itself.
Okay, I will check about your advice.
Thanks.
>
>> --
>> Trond Myklebust
>> Linux NFS client maintainer
>>
>> NetApp
>> Trond.Myklebust@netapp.com
>> www.netapp.com
>>
>> On Jun 23, 2012, at 11:07 AM, Myklebust, Trond wrote:
>>
>>> BIG NACK... We don't do embrace and extend in Linux...
>>>
>>>
>>> The whole point of the NFS protocol is to be a standard. We can't just go
>>> adding new functionality to one implementation of that standard without
>>> breaking interoperability with other implementations...
>>>
>>> Trond
>>>
>>> --
>>> Trond Myklebust
>>> Linux NFS client maintainer
>>>
>>> NetApp
>>> Trond.Myklebust@netapp.com
>>> www.netapp.com
>>>
>>> On Jun 23, 2012, at 6:46 AM, Namjae Jeon wrote:
>>>
>>>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>>>
>>>> Without this patch:
>>>>
>>>> On NFS Client:
>>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>>> $ umount /mnt
>>>>
>>>> On NFS Server:
>>>> $ umount /mnt
>>>> umount: can't umount /mnt: Device or resource busy
>>>>
>>>> If user has to remove storage device user needs to unplug the
>>>> device.
>>>> This may result in file system corruption on attached media.
>>>>
>>>> This patch tries to add a NFS UMOUNT procedure for NFSv3 for safe
>>>> removal of attached media at NFS Server.
>>>>
>>>> With this patch:
>>>>
>>>> On NFS Client:
>>>> $ mount -t nfs <NFS_SERVER>:/mnt /mnt
>>>> $ umount /mnt
>>>>
>>>> On NFS Server:
>>>> $ umount /mnt --> umount successful
>>>>
>>>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>>>> Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
>>>> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
>>>> ---
>>>> fs/nfs/nfs3proc.c | 29 +++++++++++++++++++++++++++++
>>>> fs/nfs/nfs3xdr.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>> fs/nfs/super.c | 10 ++++++++++
>>>> fs/nfsd/nfs3proc.c | 28 +++++++++++++++++++++++++++-
>>>> fs/nfsd/nfs3xdr.c | 19 +++++++++++++++++++
>>>> fs/nfsd/nfsctl.c | 22 ++++++++++++++++++++++
>>>> fs/nfsd/nfsd.h | 3 +++
>>>> fs/nfsd/nfssvc.c | 3 +++
>>>> fs/nfsd/xdr3.h | 14 +++++++++++++-
>>>> include/linux/nfs3.h | 1 +
>>>> include/linux/nfs_xdr.h | 9 +++++++++
>>>> 11 files changed, 175 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
>>>> index 2292a0f..726227f 100644
>>>> --- a/fs/nfs/nfs3proc.c
>>>> +++ b/fs/nfs/nfs3proc.c
>>>> @@ -877,6 +877,34 @@ nfs3_proc_lock(struct file *filp, int cmd, struct
>>>> file_lock *fl)
>>>> return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
>>>> }
>>>>
>>>> +static int nfs3_proc_umount(struct nfs_server *server)
>>>> +{
>>>> + /*
>>>> + * dummy argument and response are assigned to UMOUNT request
>>>> + * to avoid BUG_ON(proc->p_arglen == 0); in net/sunrpc/clnt.c
>>>> + */
>>>> + struct nfs3_umountargs arg = {
>>>> + .dummy = 0,
>>>> + };
>>>> + struct nfs3_umountres res;
>>>> +
>>>> + struct rpc_message msg = {
>>>> + .rpc_proc = &nfs3_procedures[NFS3PROC_UMOUNT],
>>>> + .rpc_argp = &arg,
>>>> + .rpc_resp = &res,
>>>> +
>>>> + };
>>>> + int err;
>>>> +
>>>> + msg.rpc_cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>>>> + dprintk("NFS call umount\n");
>>>> + err = rpc_call_sync(server->client, &msg,
>>>> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN);
>>>> + put_rpccred(msg.rpc_cred);
>>>> + dprintk("NFS reply umount: %d\n", err);
>>>> + return err;
>>>> +}
>>>> +
>>>> const struct nfs_rpc_ops nfs_v3_clientops = {
>>>> .version = 3, /* protocol version */
>>>> .dentry_ops = &nfs_dentry_operations,
>>>> @@ -922,4 +950,5 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
>>>> .clear_acl_cache = nfs3_forget_cached_acls,
>>>> .close_context = nfs_close_context,
>>>> .init_client = nfs_init_client,
>>>> + .umount = nfs3_proc_umount,
>>>> };
>>>> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
>>>> index 6cbe894..874b8b2 100644
>>>> --- a/fs/nfs/nfs3xdr.c
>>>> +++ b/fs/nfs/nfs3xdr.c
>>>> @@ -61,6 +61,7 @@
>>>> #define NFS3_readdirargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+3)
>>>> #define NFS3_readdirplusargs_sz (NFS3_fh_sz+NFS3_cookieverf_sz+4)
>>>> #define NFS3_commitargs_sz (NFS3_fh_sz+3)
>>>> +#define NFS3_umountargs_sz (1)
>>>>
>>>> #define NFS3_getattrres_sz (1+NFS3_fattr_sz)
>>>> #define NFS3_setattrres_sz (1+NFS3_wcc_data_sz)
>>>> @@ -78,6 +79,7 @@
>>>> #define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
>>>> #define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
>>>> #define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
>>>> +#define NFS3_umountres_sz (1 + 1)
>>>>
>>>> #define ACL3_getaclargs_sz (NFS3_fh_sz+1)
>>>> #define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
>>>> @@ -1306,6 +1308,22 @@ static void nfs3_xdr_enc_commit3args(struct
>>>> rpc_rqst *req,
>>>> encode_commit3args(xdr, args);
>>>> }
>>>>
>>>> +/*
>>>> + * UMOUNT3args
>>>> + */
>>>> +static void encode_umount3args(struct xdr_stream *xdr,
>>>> + const struct nfs3_umountargs *args)
>>>> +{
>>>> + encode_uint32(xdr, args->dummy);
>>>> +}
>>>> +
>>>> +static void nfs3_xdr_enc_umount3args(struct rpc_rqst *req,
>>>> + struct xdr_stream *xdr,
>>>> + const struct nfs3_umountargs *args)
>>>> +{
>>>> + encode_umount3args(xdr, args);
>>>> +}
>>>> +
>>>> #ifdef CONFIG_NFS_V3_ACL
>>>>
>>>> static void nfs3_xdr_enc_getacl3args(struct rpc_rqst *req,
>>>> @@ -1429,6 +1447,26 @@ out_status:
>>>> }
>>>>
>>>> /*
>>>> + * UMOUNT3res
>>>> + */
>>>> +static int nfs3_xdr_dec_umount3res(struct rpc_rqst *req,
>>>> + struct xdr_stream *xdr,
>>>> + struct nfs3_umountres *result)
>>>> +{
>>>> + enum nfs_stat status;
>>>> + int error;
>>>> +
>>>> + error = decode_nfsstat3(xdr, &status);
>>>> + if (unlikely(error))
>>>> + goto out;
>>>> + if (status != NFS3_OK)
>>>> + return nfs3_stat_to_errno(status);
>>>> + error = decode_uint32(xdr, &result->dummy);
>>>> +out:
>>>> + return error;
>>>> +}
>>>> +
>>>> +/*
>>>> * 3.3.3 LOOKUP3res
>>>> *
>>>> * struct LOOKUP3resok {
>>>> @@ -2508,6 +2546,7 @@ struct rpc_procinfo nfs3_procedures[] = {
>>>> PROC(FSINFO, getattr, fsinfo, 0),
>>>> PROC(PATHCONF, getattr, pathconf, 0),
>>>> PROC(COMMIT, commit, commit, 5),
>>>> + PROC(UMOUNT, umount, umount, 0),
>>>> };
>>>>
>>>> const struct rpc_version nfs_version3 = {
>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>> index 906f09c..9fa295b 100644
>>>> --- a/fs/nfs/super.c
>>>> +++ b/fs/nfs/super.c
>>>> @@ -2525,6 +2525,16 @@ static void nfs_kill_super(struct super_block
>>>> *s)
>>>> {
>>>> struct nfs_server *server = NFS_SB(s);
>>>>
>>>> + int err;
>>>> +
>>>> + if (server->nfs_client->rpc_ops->umount) {
>>>> + err = server->nfs_client->rpc_ops->umount(server);
>>>> + if (err < 0) {
>>>> + printk(KERN_ERR "%s: nfs_proc3_umount() failed err = %d\n",
>>>> + __func__, err);
>>>> + }
>>>> + }
>>>> +
>>>> kill_anon_super(s);
>>>> nfs_fscache_release_super_cookie(s);
>>>> nfs_free_server(server);
>>>> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
>>>> index 9095f3c..12ca821 100644
>>>> --- a/fs/nfsd/nfs3proc.c
>>>> +++ b/fs/nfsd/nfs3proc.c
>>>> @@ -8,6 +8,7 @@
>>>> #include <linux/ext2_fs.h>
>>>> #include <linux/magic.h>
>>>>
>>>> +#include "nfsd.h"
>>>> #include "cache.h"
>>>> #include "xdr3.h"
>>>> #include "vfs.h"
>>>> @@ -63,6 +64,21 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct
>>>> nfsd_fhandle *argp,
>>>> }
>>>>
>>>> /*
>>>> + * UMOUNT call.
>>>> + */
>>>> +static __be32
>>>> +nfsd3_proc_umount(struct svc_rqst *rqstp, struct nfsd3_umountargs
>>>> *argp,
>>>> + struct nfsd3_umountres *resp)
>>>> +{
>>>> + dprintk("nfsd: UMOUNT(3)\n");
>>>> +
>>>> + if (nfs_umountd_workqueue)
>>>> + queue_work(nfs_umountd_workqueue, &nfs_umount_work);
>>>> +
>>>> + return nfs_ok;
>>>> +}
>>>> +
>>>> +/*
>>>> * Set a file's attributes
>>>> */
>>>> static __be32
>>>> @@ -671,7 +687,7 @@ struct nfsd3_voidargs { int dummy; };
>>>> #define pAT (1+AT) /* post attributes - conditional */
>>>> #define WC (7+pAT) /* WCC attributes */
>>>>
>>>> -static struct svc_procedure nfsd_procedures3[22] = {
>>>> +static struct svc_procedure nfsd_procedures3[23] = {
>>>> [NFS3PROC_NULL] = {
>>>> .pc_func = (svc_procfunc) nfsd3_proc_null,
>>>> .pc_encode = (kxdrproc_t) nfs3svc_encode_voidres,
>>>> @@ -885,11 +901,21 @@ static struct svc_procedure nfsd_procedures3[22]
>>>> = {
>>>> .pc_cachetype = RC_NOCACHE,
>>>> .pc_xdrressize = ST+WC+2,
>>>> },
>>>> + [NFS3PROC_UMOUNT] = {
>>>> + .pc_func = (svc_procfunc) nfsd3_proc_umount,
>>>> + .pc_decode = (kxdrproc_t) nfs3svc_decode_umountargs,
>>>> + .pc_encode = (kxdrproc_t) nfs3svc_encode_umountres,
>>>> + .pc_argsize = sizeof(struct nfsd3_umountargs),
>>>> + .pc_ressize = sizeof(struct nfsd3_umountres),
>>>> + .pc_cachetype = RC_NOCACHE,
>>>> + .pc_xdrressize = ST+1,
>>>> + },
>>>> };
>>>>
>>>> struct svc_version nfsd_version3 = {
>>>> .vs_vers = 3,
>>>> .vs_nproc = 22,
>>>> + .vs_nproc = 23,
>>>> .vs_proc = nfsd_procedures3,
>>>> .vs_dispatch = nfsd_dispatch,
>>>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>>>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>>>> index 43f46cd..8610282 100644
>>>> --- a/fs/nfsd/nfs3xdr.c
>>>> +++ b/fs/nfsd/nfs3xdr.c
>>>> @@ -611,6 +611,15 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp,
>>>> __be32 *p,
>>>> return xdr_argsize_check(rqstp, p);
>>>> }
>>>>
>>>> +int
>>>> +nfs3svc_decode_umountargs(struct svc_rqst *rqstp, __be32 *p,
>>>> + struct nfsd3_umountargs *args)
>>>> +{
>>>> + args->dummy = ntohl(*p++);
>>>> +
>>>> + return xdr_argsize_check(rqstp, p);
>>>> +}
>>>> +
>>>> /*
>>>> * XDR encode functions
>>>> */
>>>> @@ -1091,6 +1100,16 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp,
>>>> __be32 *p,
>>>> return xdr_ressize_check(rqstp, p);
>>>> }
>>>>
>>>> +/* UMOUNT */
>>>> +int
>>>> +nfs3svc_encode_umountres(struct svc_rqst *rqstp, __be32 *p,
>>>> + struct nfsd3_umountres *resp)
>>>> +{
>>>> + if (resp->status == 0)
>>>> + *p++ = htonl(resp->dummy);
>>>> + return xdr_ressize_check(rqstp, p);
>>>> +}
>>>> +
>>>> /*
>>>> * XDR release functions
>>>> */
>>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>>> index c55298e..53748ac 100644
>>>> --- a/fs/nfsd/nfsctl.c
>>>> +++ b/fs/nfsd/nfsctl.c
>>>> @@ -50,6 +50,9 @@ enum {
>>>> #endif
>>>> };
>>>>
>>>> +struct workqueue_struct *nfs_umountd_workqueue;
>>>> +struct work_struct nfs_umount_work;
>>>> +
>>>> /*
>>>> * write() for these nodes.
>>>> */
>>>> @@ -1175,6 +1178,17 @@ static struct pernet_operations nfsd_net_ops = {
>>>> .size = sizeof(struct nfsd_net),
>>>> };
>>>>
>>>> +static void nfs_umountd_fn(struct work_struct *unused)
>>>> +{
>>>> + nfsd_export_flush(&init_net);
>>>> +}
>>>> +
>>>> +static void nfsd_umount_destroy(void)
>>>> +{
>>>> + if (nfs_umountd_workqueue)
>>>> + destroy_workqueue(nfs_umountd_workqueue);
>>>> +}
>>>> +
>>>> static int __init init_nfsd(void)
>>>> {
>>>> int retval;
>>>> @@ -1204,6 +1218,13 @@ static int __init init_nfsd(void)
>>>> retval = register_filesystem(&nfsd_fs_type);
>>>> if (retval)
>>>> goto out_free_all;
>>>> +
>>>> + nfs_umountd_workqueue = create_singlethread_workqueue("nfs.umountd");
>>>> + if (!nfs_umountd_workqueue)
>>>> + printk(KERN_ERR "nfsd: Failed to create nfs.umountd workqueue\n");
>>>> + else
>>>> + INIT_WORK(&nfs_umount_work, nfs_umountd_fn);
>>>> +
>>>> return 0;
>>>> out_free_all:
>>>> remove_proc_entry("fs/nfs/exports", NULL);
>>>> @@ -1225,6 +1246,7 @@ out_unregister_notifier:
>>>>
>>>> static void __exit exit_nfsd(void)
>>>> {
>>>> + nfsd_umount_destroy();
>>>> nfsd_reply_cache_shutdown();
>>>> remove_proc_entry("fs/nfs/exports", NULL);
>>>> remove_proc_entry("fs/nfs", NULL);
>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>> index 1671429..691450a 100644
>>>> --- a/fs/nfsd/nfsd.h
>>>> +++ b/fs/nfsd/nfsd.h
>>>> @@ -20,6 +20,7 @@
>>>> #include <linux/nfsd/debug.h>
>>>> #include <linux/nfsd/export.h>
>>>> #include <linux/nfsd/stats.h>
>>>> +#include <linux/workqueue.h>
>>>>
>>>> /*
>>>> * nfsd version
>>>> @@ -61,6 +62,8 @@ extern unsigned int nfsd_drc_max_mem;
>>>> extern unsigned int nfsd_drc_mem_used;
>>>>
>>>> extern const struct seq_operations nfs_exports_op;
>>>> +extern struct workqueue_struct *nfs_umountd_workqueue;
>>>> +extern struct work_struct nfs_umount_work;
>>>>
>>>> /*
>>>> * Function prototypes.
>>>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>>>> index ee709fc..1fb3598 100644
>>>> --- a/fs/nfsd/nfssvc.c
>>>> +++ b/fs/nfsd/nfssvc.c
>>>> @@ -256,6 +256,9 @@ static void nfsd_last_thread(struct svc_serv *serv,
>>>> struct net *net)
>>>> {
>>>> /* When last nfsd thread exits we need to do some clean-up */
>>>> nfsd_serv = NULL;
>>>> + if (nfs_umountd_workqueue)
>>>> + flush_workqueue(nfs_umountd_workqueue);
>>>> +
>>>> nfsd_shutdown();
>>>>
>>>> svc_rpcb_cleanup(serv, net);
>>>> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
>>>> index 7df980e..b542c92 100644
>>>> --- a/fs/nfsd/xdr3.h
>>>> +++ b/fs/nfsd/xdr3.h
>>>> @@ -106,6 +106,10 @@ struct nfsd3_commitargs {
>>>> __u32 count;
>>>> };
>>>>
>>>> +struct nfsd3_umountargs {
>>>> + __u32 dummy;
>>>> +};
>>>> +
>>>> struct nfsd3_getaclargs {
>>>> struct svc_fh fh;
>>>> int mask;
>>>> @@ -219,6 +223,11 @@ struct nfsd3_commitres {
>>>> struct svc_fh fh;
>>>> };
>>>>
>>>> +struct nfsd3_umountres {
>>>> + __be32 status;
>>>> + __u32 dummy;
>>>> +};
>>>> +
>>>> struct nfsd3_getaclres {
>>>> __be32 status;
>>>> struct svc_fh fh;
>>>> @@ -295,6 +304,8 @@ int nfs3svc_decode_readdirplusargs(struct svc_rqst
>>>> *, __be32 *,
>>>> struct nfsd3_readdirargs *);
>>>> int nfs3svc_decode_commitargs(struct svc_rqst *, __be32 *,
>>>> struct nfsd3_commitargs *);
>>>> +int nfs3svc_decode_umountargs(struct svc_rqst *, __be32 *,
>>>> + struct nfsd3_umountargs *);
>>>> int nfs3svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
>>>> int nfs3svc_encode_attrstat(struct svc_rqst *, __be32 *,
>>>> struct nfsd3_attrstat *);
>>>> @@ -324,7 +335,8 @@ int nfs3svc_encode_pathconfres(struct svc_rqst *,
>>>> __be32 *,
>>>> struct nfsd3_pathconfres *);
>>>> int nfs3svc_encode_commitres(struct svc_rqst *, __be32 *,
>>>> struct nfsd3_commitres *);
>>>> -
>>>> +int nfs3svc_encode_umountres(struct svc_rqst *, __be32 *,
>>>> + struct nfsd3_umountres *);
>>>> int nfs3svc_release_fhandle(struct svc_rqst *, __be32 *,
>>>> struct nfsd3_attrstat *);
>>>> int nfs3svc_release_fhandle2(struct svc_rqst *, __be32 *,
>>>> diff --git a/include/linux/nfs3.h b/include/linux/nfs3.h
>>>> index 6ccfe3b..968e2e0 100644
>>>> --- a/include/linux/nfs3.h
>>>> +++ b/include/linux/nfs3.h
>>>> @@ -90,6 +90,7 @@ struct nfs3_fh {
>>>> #define NFS3PROC_FSINFO 19
>>>> #define NFS3PROC_PATHCONF 20
>>>> #define NFS3PROC_COMMIT 21
>>>> +#define NFS3PROC_UMOUNT 22
>>>>
>>>> #define NFS_MNT3_VERSION 3
>>>>
>>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>>> index 5c0014d..da53bd8 100644
>>>> --- a/include/linux/nfs_xdr.h
>>>> +++ b/include/linux/nfs_xdr.h
>>>> @@ -786,6 +786,14 @@ struct nfs3_readdirargs {
>>>> struct page ** pages;
>>>> };
>>>>
>>>> +struct nfs3_umountargs {
>>>> + __u32 dummy;
>>>> +};
>>>> +
>>>> +struct nfs3_umountres {
>>>> + __u32 dummy;
>>>> +};
>>>> +
>>>> struct nfs3_diropres {
>>>> struct nfs_fattr * dir_attr;
>>>> struct nfs_fh * fh;
>>>> @@ -1425,6 +1433,7 @@ struct nfs_rpc_ops {
>>>> struct nfs_client *
>>>> (*init_client) (struct nfs_client *, const struct rpc_timeout *,
>>>> const char *, rpc_authflavor_t);
>>>> + int (*umount)(struct nfs_server *);
>>>> };
>>>>
>>>> /*
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-23 16:09 ` Fields James
@ 2012-06-24 5:12 ` Namjae Jeon
2012-06-25 11:30 ` Fields James
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2012-06-24 5:12 UTC (permalink / raw)
To: Fields James
Cc: Myklebust, Trond, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
2012/6/24, Fields James <bfields@fieldses.org>:
> On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
>> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
>> > The other objection is that NFSv3 already has a way to do this via the
>> > MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on
>> > unmount of a filesystem.
>> >
>> > The problem with this approach is that if the NFS client crashes before
>> > it can call this function, then the server never gets notified. That is
>> > a problem that your patch has too...
>>
>> Yes. Could you instead work on describing *exactly* what the original
>> problem was that prompted this?
>
> ("You" here meaning Namjae Jeon, of course).
>
> And note here we've all read the changelog, that's not what I'm asking
> for.
>
> I want to know the details of *your* specific case. "I have X clients
> that I use for Y, periodically I want to unmount the exported filesystem
> to do Z, I tried doing W but it didn't work...".
As you remember, We discussed this
issue(http://www.spinics.net/lists/linux-nfs/msg29997.html) with you
before. you guided me to use exportfs -f call on nfs server.
It is hard to use this method on our system and nomally the user is
thinking it is problem device busy happen after umounting.
The details of specific case.
1. There are two target or two PC. and Client want to access files of
usb device on server.
2. the client try to mount usb mount point using nfs on sever after
plugging usb device.
3. we try to umount mount point directory on client.
4. and when we try to umount usb device mount directory server, device
busy message occured like this (umount: can't umount /mnt: Device or
resource busy)
Let me know in case you have any question.
Thanks.
>
> --b.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-24 5:12 ` Namjae Jeon
@ 2012-06-25 11:30 ` Fields James
2012-06-26 5:51 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Fields James @ 2012-06-25 11:30 UTC (permalink / raw)
To: Namjae Jeon
Cc: Myklebust, Trond, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
On Sun, Jun 24, 2012 at 02:12:49PM +0900, Namjae Jeon wrote:
> 2012/6/24, Fields James <bfields@fieldses.org>:
> > On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
> >> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
> >> > The other objection is that NFSv3 already has a way to do this via the
> >> > MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on
> >> > unmount of a filesystem.
> >> >
> >> > The problem with this approach is that if the NFS client crashes before
> >> > it can call this function, then the server never gets notified. That is
> >> > a problem that your patch has too...
> >>
> >> Yes. Could you instead work on describing *exactly* what the original
> >> problem was that prompted this?
> >
> > ("You" here meaning Namjae Jeon, of course).
> >
> > And note here we've all read the changelog, that's not what I'm asking
> > for.
> >
> > I want to know the details of *your* specific case. "I have X clients
> > that I use for Y, periodically I want to unmount the exported filesystem
> > to do Z, I tried doing W but it didn't work...".
> As you remember, We discussed this
> issue(http://www.spinics.net/lists/linux-nfs/msg29997.html) with you
> before. you guided me to use exportfs -f call on nfs server.
> It is hard to use this method on our system
Why is it hard?
Could you hack umount to do the equivalent of "exportfs -f" before
calling the umount?
> and nomally the user is
> thinking it is problem device busy happen after umounting.
>
> The details of specific case.
> 1. There are two target or two PC. and Client want to access files of
> usb device on server.
> 2. the client try to mount usb mount point using nfs on sever after
> plugging usb device.
> 3. we try to umount mount point directory on client.
> 4. and when we try to umount usb device mount directory server, device
> busy message occured like this (umount: can't umount /mnt: Device or
> resource busy)
Right, so I don't think you want protocol modifications. They take a
few years to get hammered out and then make it into implementations, and
they won't help a great deal with your problem anyway. (Because of
problems like the possibility of lost "unmount" calls that Trond
mentions.)
Either modify umount itself to flush the export cache before attempting
the unmount, or if that doesn't work perhaps you could patch the kernel
to notify nfsd on unmount and let it attemt to clear its caches before
deciding whether the filesystem is busy.
--b.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-25 11:30 ` Fields James
@ 2012-06-26 5:51 ` Namjae Jeon
2012-06-26 11:10 ` Fields James
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2012-06-26 5:51 UTC (permalink / raw)
To: Fields James
Cc: Myklebust, Trond, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
2012/6/25, Fields James <bfields@fieldses.org>:
> On Sun, Jun 24, 2012 at 02:12:49PM +0900, Namjae Jeon wrote:
>> 2012/6/24, Fields James <bfields@fieldses.org>:
>> > On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
>> >> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
>> >> > The other objection is that NFSv3 already has a way to do this via
>> >> > the
>> >> > MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on
>> >> > unmount of a filesystem.
>> >> >
>> >> > The problem with this approach is that if the NFS client crashes
>> >> > before
>> >> > it can call this function, then the server never gets notified. That
>> >> > is
>> >> > a problem that your patch has too...
>> >>
>> >> Yes. Could you instead work on describing *exactly* what the original
>> >> problem was that prompted this?
>> >
>> > ("You" here meaning Namjae Jeon, of course).
>> >
>> > And note here we've all read the changelog, that's not what I'm asking
>> > for.
>> >
>> > I want to know the details of *your* specific case. "I have X clients
>> > that I use for Y, periodically I want to unmount the exported
>> > filesystem
>> > to do Z, I tried doing W but it didn't work...".
>> As you remember, We discussed this
>> issue(http://www.spinics.net/lists/linux-nfs/msg29997.html) with you
>> before. you guided me to use exportfs -f call on nfs server.
>> It is hard to use this method on our system
>
> Why is it hard?
Two target are connected by only nfs.. we can't manually hand exportfs -f..
And I should make each new App on client / server to call exportfs -f.
>
> Could you hack umount to do the equivalent of "exportfs -f" before
> calling the umount?
Good Idea! I will check your suggestion.
>
>> and nomally the user is
>> thinking it is problem device busy happen after umounting.
>>
>> The details of specific case.
>> 1. There are two target or two PC. and Client want to access files of
>> usb device on server.
>> 2. the client try to mount usb mount point using nfs on sever after
>> plugging usb device.
>> 3. we try to umount mount point directory on client.
>> 4. and when we try to umount usb device mount directory server, device
>> busy message occured like this (umount: can't umount /mnt: Device or
>> resource busy)
>
> Right, so I don't think you want protocol modifications. They take a
> few years to get hammered out and then make it into implementations, and
> they won't help a great deal with your problem anyway. (Because of
> problems like the possibility of lost "unmount" calls that Trond
> mentions.)
>
> Either modify umount itself to flush the export cache before attempting
> the unmount, or if that doesn't work perhaps you could patch the kernel
> to notify nfsd on unmount and let it attemt to clear its caches before
> deciding whether the filesystem is busy.
After checking your suggestion I will share with you.
Thanks a lot. James.
>
> --b.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-26 5:51 ` Namjae Jeon
@ 2012-06-26 11:10 ` Fields James
2012-06-26 11:57 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Fields James @ 2012-06-26 11:10 UTC (permalink / raw)
To: Namjae Jeon
Cc: Myklebust, Trond, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
On Tue, Jun 26, 2012 at 02:51:52PM +0900, Namjae Jeon wrote:
> 2012/6/25, Fields James <bfields@fieldses.org>:
> > On Sun, Jun 24, 2012 at 02:12:49PM +0900, Namjae Jeon wrote:
> >> 2012/6/24, Fields James <bfields@fieldses.org>:
> >> > On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
> >> >> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
> >> >> > The other objection is that NFSv3 already has a way to do this via
> >> >> > the
> >> >> > MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on
> >> >> > unmount of a filesystem.
> >> >> >
> >> >> > The problem with this approach is that if the NFS client crashes
> >> >> > before
> >> >> > it can call this function, then the server never gets notified. That
> >> >> > is
> >> >> > a problem that your patch has too...
> >> >>
> >> >> Yes. Could you instead work on describing *exactly* what the original
> >> >> problem was that prompted this?
> >> >
> >> > ("You" here meaning Namjae Jeon, of course).
> >> >
> >> > And note here we've all read the changelog, that's not what I'm asking
> >> > for.
> >> >
> >> > I want to know the details of *your* specific case. "I have X clients
> >> > that I use for Y, periodically I want to unmount the exported
> >> > filesystem
> >> > to do Z, I tried doing W but it didn't work...".
> >> As you remember, We discussed this
> >> issue(http://www.spinics.net/lists/linux-nfs/msg29997.html) with you
> >> before. you guided me to use exportfs -f call on nfs server.
> >> It is hard to use this method on our system
> >
> > Why is it hard?
> Two target are connected by only nfs.. we can't manually hand exportfs -f..
So how can you call umount?
--b.
> And I should make each new App on client / server to call exportfs -f.
> >
> > Could you hack umount to do the equivalent of "exportfs -f" before
> > calling the umount?
> Good Idea! I will check your suggestion.
>
> >
> >> and nomally the user is
> >> thinking it is problem device busy happen after umounting.
> >>
> >> The details of specific case.
> >> 1. There are two target or two PC. and Client want to access files of
> >> usb device on server.
> >> 2. the client try to mount usb mount point using nfs on sever after
> >> plugging usb device.
> >> 3. we try to umount mount point directory on client.
> >> 4. and when we try to umount usb device mount directory server, device
> >> busy message occured like this (umount: can't umount /mnt: Device or
> >> resource busy)
> >
> > Right, so I don't think you want protocol modifications. They take a
> > few years to get hammered out and then make it into implementations, and
> > they won't help a great deal with your problem anyway. (Because of
> > problems like the possibility of lost "unmount" calls that Trond
> > mentions.)
> >
> > Either modify umount itself to flush the export cache before attempting
> > the unmount, or if that doesn't work perhaps you could patch the kernel
> > to notify nfsd on unmount and let it attemt to clear its caches before
> > deciding whether the filesystem is busy.
> After checking your suggestion I will share with you.
> Thanks a lot. James.
> >
> > --b.
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure.
2012-06-26 11:10 ` Fields James
@ 2012-06-26 11:57 ` Namjae Jeon
0 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2012-06-26 11:57 UTC (permalink / raw)
To: Fields James
Cc: Myklebust, Trond, Morton Andrew, Harrosh Boaz,
<linux-nfs@vger.kernel.org>,
linux-kernel, Namjae Jeon, Vivek Trivedi, Amit Sahrawat
2012/6/26, Fields James <bfields@fieldses.org>:
> On Tue, Jun 26, 2012 at 02:51:52PM +0900, Namjae Jeon wrote:
>> 2012/6/25, Fields James <bfields@fieldses.org>:
>> > On Sun, Jun 24, 2012 at 02:12:49PM +0900, Namjae Jeon wrote:
>> >> 2012/6/24, Fields James <bfields@fieldses.org>:
>> >> > On Sat, Jun 23, 2012 at 11:57:10AM -0400, Fields James wrote:
>> >> >> On Sat, Jun 23, 2012 at 03:52:20PM +0000, Myklebust, Trond wrote:
>> >> >> > The other objection is that NFSv3 already has a way to do this
>> >> >> > via
>> >> >> > the
>> >> >> > MOUNT protocol. We already call MOUNTPROC3_UMNT/MOUNTPROC_UMNT on
>> >> >> > unmount of a filesystem.
>> >> >> >
>> >> >> > The problem with this approach is that if the NFS client crashes
>> >> >> > before
>> >> >> > it can call this function, then the server never gets notified.
>> >> >> > That
>> >> >> > is
>> >> >> > a problem that your patch has too...
>> >> >>
>> >> >> Yes. Could you instead work on describing *exactly* what the
>> >> >> original
>> >> >> problem was that prompted this?
>> >> >
>> >> > ("You" here meaning Namjae Jeon, of course).
>> >> >
>> >> > And note here we've all read the changelog, that's not what I'm
>> >> > asking
>> >> > for.
>> >> >
>> >> > I want to know the details of *your* specific case. "I have X
>> >> > clients
>> >> > that I use for Y, periodically I want to unmount the exported
>> >> > filesystem
>> >> > to do Z, I tried doing W but it didn't work...".
>> >> As you remember, We discussed this
>> >> issue(http://www.spinics.net/lists/linux-nfs/msg29997.html) with you
>> >> before. you guided me to use exportfs -f call on nfs server.
>> >> It is hard to use this method on our system
>> >
>> > Why is it hard?
>> Two target are connected by only nfs.. we can't manually hand exportfs
>> -f..
>
> So how can you call umount?
I am currently checking two approaches.
1. Add flush cache code in current MOUNTPROC3_UMNT/MOUNTPROC_UMNT.
-> I heard from Trond that there already are
MOUNTPROC3_UMNT/MOUNTPROC_UMNT. So I will try to fix this issue on
current protocols without many changes. It is not specification
violation.
2. Check your suggestion.
-> I think that your suggestion is very good. it will cover device
unplug case(automount will try to umount about unplugged device) and
current busy umount issue.
If I have one concern, It need to change VFS code. I need your review
and Ack to commit if I post this patch.
Thanks.
>
> --b.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-06-26 11:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-23 10:46 [RFC PATCH] nfs: Add NFS3PROC_UMOUNT procedure Namjae Jeon
2012-06-23 15:07 ` Myklebust, Trond
2012-06-23 15:52 ` Myklebust, Trond
2012-06-23 15:57 ` Fields James
2012-06-23 16:09 ` Fields James
2012-06-24 5:12 ` Namjae Jeon
2012-06-25 11:30 ` Fields James
2012-06-26 5:51 ` Namjae Jeon
2012-06-26 11:10 ` Fields James
2012-06-26 11:57 ` Namjae Jeon
2012-06-23 15:59 ` Chuck Lever
2012-06-24 4:49 ` Namjae Jeon
2012-06-24 4:44 ` Namjae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.