All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
@ 2010-09-09 19:36 Suresh Jayaraman
  2010-09-09 20:20 ` Jeff Layton
  0 siblings, 1 reply; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-09 19:36 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS mailing list, Jeff Layton, Neil Brown

NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
locks. Due to this, some windows applications which seem to use both flock
(share mode lock mapped as flock by Samba) and fcntl locks sequentially on
the same file, can't lock as they falsely assume the file is already locked.
The problem was reported on a setup with windows clients accessing excel files
on a Samba exported share which is originally a NFS mount from a NetApp filer.

Older NFS clients (< 2.6.12) did not see this problem as flock locks were
considered local. To support legacy flock behavior, this patch adds a mount
option "-olocal_lock=" which can take the following values:

   'none'  		- Neither flock locks or fcntl/posix locks are local
   'flock'/'posix' 	- flock locks are local
   'fcntl' 		- fcntl locks are local
   'all'		- Both flock locks and fcntl/posix locks are local

Testing:

This patch was tested by using -olocal_lock option with different values and
the NLM calls were noted from the network packet capture.

   'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
   	     was granted, fcntl was denied
   'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
   	     granted
   'fcntl' - NLM call was seen for flock() - granted, no NLM call for fcntl()
   'all'   - no NLM calls were seen during both flock() and fcntl()

Also, with these patches ensured that the exisiting behavior of '-olock' and
'-onolock' didn't change.

Cc: Neil Brown <neilb@suse.de>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---
 fs/nfs/client.c           |    6 +++-
 fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
 fs/nfs/super.c            |   47 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_mount.h |    5 +++-
 4 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4e7df2a..5f01f42 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
  */
 static void nfs_destroy_server(struct nfs_server *server)
 {
-	if (!(server->flags & NFS_MOUNT_NONLM))
+	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
+			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		nlmclnt_done(server->nlm_host);
 }
 
@@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
 
 	if (nlm_init.nfs_version > 3)
 		return 0;
-	if (server->flags & NFS_MOUNT_NONLM)
+	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
+			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		return 0;
 
 	switch (clp->cl_proto) {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index eb51bd6..e338d37 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
 	return ret;
 }
 
-static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status = 0;
@@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (nfs_have_delegation(inode, FMODE_READ))
 		goto out_noconflict;
 
-	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
+	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
 		goto out_noconflict;
 
 	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
@@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
 	return res;
 }
 
-static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -746,14 +748,15 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
 	 * 	still need to complete the unlock.
 	 */
 	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
 	return status;
 }
 
-static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -767,7 +770,7 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
 		goto out;
 
 	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
@@ -791,6 +794,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int ret = -ENOLCK;
+	int is_local = 0;
 
 	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
@@ -804,6 +808,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		goto out_err;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
+		is_local = 1;
+
 	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
 		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
 		if (ret < 0)
@@ -811,11 +818,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	}
 
 	if (IS_GETLK(cmd))
-		ret = do_getlk(filp, cmd, fl);
+		ret = do_getlk(filp, cmd, fl, is_local);
 	else if (fl->fl_type == F_UNLCK)
-		ret = do_unlk(filp, cmd, fl);
+		ret = do_unlk(filp, cmd, fl, is_local);
 	else
-		ret = do_setlk(filp, cmd, fl);
+		ret = do_setlk(filp, cmd, fl, is_local);
 out_err:
 	return ret;
 }
@@ -825,6 +832,9 @@ out_err:
  */
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 {
+	struct inode *inode = filp->f_mapping->host;
+	int is_local = 0;
+
 	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
 			filp->f_path.dentry->d_name.name,
@@ -833,14 +843,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
+		is_local = 1;
+
 	/* We're simulating flock() locks using posix locks on the server */
 	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_start = 0;
 	fl->fl_end = OFFSET_MAX;
 
 	if (fl->fl_type == F_UNLCK)
-		return do_unlk(filp, cmd, fl);
-	return do_setlk(filp, cmd, fl);
+		return do_unlk(filp, cmd, fl, is_local);
+	return do_setlk(filp, cmd, fl, is_local);
 }
 
 /*
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ec3966e..c0d51dc 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -100,6 +100,7 @@ enum {
 	Opt_addr, Opt_mountaddr, Opt_clientaddr,
 	Opt_lookupcache,
 	Opt_fscache_uniq,
+	Opt_local_lock,
 
 	/* Special mount options */
 	Opt_userspace, Opt_deprecated, Opt_sloppy,
@@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
 
 	{ Opt_lookupcache, "lookupcache=%s" },
 	{ Opt_fscache_uniq, "fsc=%s" },
+	{ Opt_local_lock, "local_lock=%s" },
 
 	{ Opt_err, NULL }
 };
@@ -236,6 +238,23 @@ static match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_err, NULL }
 };
 
+enum {
+	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_fcntl,
+	Opt_local_lock_none,
+
+	Opt_local_lock_err
+};
+
+static match_table_t nfs_local_lock_tokens = {
+	{ Opt_local_lock_all, "all" },
+	{ Opt_local_lock_flock, "flock" },
+	{ Opt_local_lock_fcntl, "fcntl" },
+	{ Opt_local_lock_fcntl, "posix" },
+	{ Opt_local_lock_none, "none" },
+
+	{ Opt_local_lock_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);
@@ -1412,6 +1431,34 @@ static int nfs_parse_mount_options(char *raw,
 			mnt->fscache_uniq = string;
 			mnt->options |= NFS_OPTION_FSCACHE;
 			break;
+		case Opt_local_lock:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string, nfs_local_lock_tokens,
+					args);
+			kfree(string);
+			switch (token) {
+			case Opt_local_lock_all:
+				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+					       NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			case Opt_local_lock_flock:
+				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
+				break;
+			case Opt_local_lock_fcntl:
+				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
+				break;
+			case Opt_local_lock_none:
+				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+						NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			default:
+				dfprintk(MOUNT, "NFS:	invalid	"
+						"local_lock argument\n");
+				return 0;
+			};
+			break;
 
 		/*
 		 * Special options
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 5d59ae8..652d7e9 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -56,7 +56,6 @@ struct nfs_mount_data {
 #define NFS_MOUNT_TCP		0x0040	/* 2 */
 #define NFS_MOUNT_VER3		0x0080	/* 3 */
 #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
-#define NFS_MOUNT_NONLM		0x0200	/* 3 */
 #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
 #define NFS_MOUNT_NOACL		0x0800	/* 4 */
 #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
@@ -71,4 +70,8 @@ struct nfs_mount_data {
 #define NFS_MOUNT_NORESVPORT		0x40000
 #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
 
+#define NFS_MOUNT_LOCAL_FLOCK	0x100000
+#define NFS_MOUNT_LOCAL_FCNTL	0x200000
+#define NFS_MOUNT_NONLM	 	(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
+
 #endif

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-09 19:36 [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local Suresh Jayaraman
@ 2010-09-09 20:20 ` Jeff Layton
  2010-09-09 23:14   ` Suresh Jayaraman
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2010-09-09 20:20 UTC (permalink / raw)
  To: Suresh Jayaraman; +Cc: Trond Myklebust, Linux NFS mailing list, Neil Brown

On Fri, 10 Sep 2010 01:06:38 +0530
Suresh Jayaraman <sjayaraman@suse.de> wrote:

> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> locks. Due to this, some windows applications which seem to use both flock
> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> the same file, can't lock as they falsely assume the file is already locked.
> The problem was reported on a setup with windows clients accessing excel files
> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> 
> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> considered local. To support legacy flock behavior, this patch adds a mount
> option "-olocal_lock=" which can take the following values:
> 
>    'none'  		- Neither flock locks or fcntl/posix locks are local
>    'flock'/'posix' 	- flock locks are local
	^^^^^^^
"posix" ought to be synonymous with "fcntl". "flock" was a BSD-ism.

It may be better to keep it simple though and drop either "posix" or
"fcntl". No need to add unneeded synonyms on a brand new mount option.

>    'fcntl' 		- fcntl locks are local
>    'all'		- Both flock locks and fcntl/posix locks are local
> 
> Testing:
> 
> This patch was tested by using -olocal_lock option with different values and
> the NLM calls were noted from the network packet capture.
> 
>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>    	     was granted, fcntl was denied
>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>    	     granted
>    'fcntl' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>    'all'   - no NLM calls were seen during both flock() and fcntl()
> 
> Also, with these patches ensured that the exisiting behavior of '-olock' and
> '-onolock' didn't change.
> 
> Cc: Neil Brown <neilb@suse.de>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
>  fs/nfs/client.c           |    6 +++-
>  fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
>  fs/nfs/super.c            |   47 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/nfs_mount.h |    5 +++-
>  4 files changed, 79 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 4e7df2a..5f01f42 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>   */
>  static void nfs_destroy_server(struct nfs_server *server)
>  {
> -	if (!(server->flags & NFS_MOUNT_NONLM))
> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		nlmclnt_done(server->nlm_host);
>  }
>  
> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>  
>  	if (nlm_init.nfs_version > 3)
>  		return 0;
> -	if (server->flags & NFS_MOUNT_NONLM)
> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		return 0;
>  
>  	switch (clp->cl_proto) {
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index eb51bd6..e338d37 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>  	return ret;
>  }
>  
> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status = 0;
> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (nfs_have_delegation(inode, FMODE_READ))
>  		goto out_noconflict;
>  
> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
> +	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
>  		goto out_noconflict;
>  
>  	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> @@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
>  	return res;
>  }
>  
> -static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -746,14 +748,15 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
>  	 * 	still need to complete the unlock.
>  	 */
>  	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
>  	return status;
>  }
>  
> -static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -767,7 +770,7 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
>  		goto out;
>  
>  	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
> @@ -791,6 +794,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int ret = -ENOLCK;
> +	int is_local = 0;
>  
>  	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
> @@ -804,6 +808,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
>  		goto out_err;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
> +		is_local = 1;
> +
>  	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
>  		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
>  		if (ret < 0)
> @@ -811,11 +818,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	}
>  
>  	if (IS_GETLK(cmd))
> -		ret = do_getlk(filp, cmd, fl);
> +		ret = do_getlk(filp, cmd, fl, is_local);
>  	else if (fl->fl_type == F_UNLCK)
> -		ret = do_unlk(filp, cmd, fl);
> +		ret = do_unlk(filp, cmd, fl, is_local);
>  	else
> -		ret = do_setlk(filp, cmd, fl);
> +		ret = do_setlk(filp, cmd, fl, is_local);
>  out_err:
>  	return ret;
>  }
> @@ -825,6 +832,9 @@ out_err:
>   */
>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  {
> +	struct inode *inode = filp->f_mapping->host;
> +	int is_local = 0;
> +
>  	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
>  			filp->f_path.dentry->d_name.name,
> @@ -833,14 +843,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
> +		is_local = 1;
> +
>  	/* We're simulating flock() locks using posix locks on the server */
>  	fl->fl_owner = (fl_owner_t)filp;
>  	fl->fl_start = 0;
>  	fl->fl_end = OFFSET_MAX;
>  
>  	if (fl->fl_type == F_UNLCK)
> -		return do_unlk(filp, cmd, fl);
> -	return do_setlk(filp, cmd, fl);
> +		return do_unlk(filp, cmd, fl, is_local);
> +	return do_setlk(filp, cmd, fl, is_local);
>  }
>  
>  /*
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index ec3966e..c0d51dc 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -100,6 +100,7 @@ enum {
>  	Opt_addr, Opt_mountaddr, Opt_clientaddr,
>  	Opt_lookupcache,
>  	Opt_fscache_uniq,
> +	Opt_local_lock,
>  
>  	/* Special mount options */
>  	Opt_userspace, Opt_deprecated, Opt_sloppy,
> @@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
>  
>  	{ Opt_lookupcache, "lookupcache=%s" },
>  	{ Opt_fscache_uniq, "fsc=%s" },
> +	{ Opt_local_lock, "local_lock=%s" },
>  
>  	{ Opt_err, NULL }
>  };
> @@ -236,6 +238,23 @@ static match_table_t nfs_lookupcache_tokens = {
>  	{ Opt_lookupcache_err, NULL }
>  };
>  
> +enum {
> +	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_fcntl,
> +	Opt_local_lock_none,
> +
> +	Opt_local_lock_err
> +};
> +
> +static match_table_t nfs_local_lock_tokens = {
> +	{ Opt_local_lock_all, "all" },
> +	{ Opt_local_lock_flock, "flock" },
> +	{ Opt_local_lock_fcntl, "fcntl" },
> +	{ Opt_local_lock_fcntl, "posix" },
> +	{ Opt_local_lock_none, "none" },
> +
> +	{ Opt_local_lock_err, NULL }
> +};
> +
>  
>  static void nfs_umount_begin(struct super_block *);
>  static int  nfs_statfs(struct dentry *, struct kstatfs *);
> @@ -1412,6 +1431,34 @@ static int nfs_parse_mount_options(char *raw,
>  			mnt->fscache_uniq = string;
>  			mnt->options |= NFS_OPTION_FSCACHE;
>  			break;
> +		case Opt_local_lock:
> +			string = match_strdup(args);
> +			if (string == NULL)
> +				goto out_nomem;
> +			token = match_token(string, nfs_local_lock_tokens,
> +					args);
> +			kfree(string);
> +			switch (token) {
> +			case Opt_local_lock_all:
> +				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
> +					       NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			case Opt_local_lock_flock:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
> +				break;
> +			case Opt_local_lock_fcntl:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
> +				break;
> +			case Opt_local_lock_none:
> +				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
> +						NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			default:
> +				dfprintk(MOUNT, "NFS:	invalid	"
> +						"local_lock argument\n");
> +				return 0;
> +			};
> +			break;
>  
>  		/*
>  		 * Special options
> diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
> index 5d59ae8..652d7e9 100644
> --- a/include/linux/nfs_mount.h
> +++ b/include/linux/nfs_mount.h
> @@ -56,7 +56,6 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_TCP		0x0040	/* 2 */
>  #define NFS_MOUNT_VER3		0x0080	/* 3 */
>  #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
> -#define NFS_MOUNT_NONLM		0x0200	/* 3 */
>  #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>  #define NFS_MOUNT_NOACL		0x0800	/* 4 */
>  #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
> @@ -71,4 +70,8 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_NORESVPORT		0x40000
>  #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
>  
> +#define NFS_MOUNT_LOCAL_FLOCK	0x100000
> +#define NFS_MOUNT_LOCAL_FCNTL	0x200000
> +#define NFS_MOUNT_NONLM	 	(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
> +
>  #endif


-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-09 20:20 ` Jeff Layton
@ 2010-09-09 23:14   ` Suresh Jayaraman
  2010-09-10  1:44     ` Trond Myklebust
  0 siblings, 1 reply; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-09 23:14 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Trond Myklebust, Linux NFS mailing list, Neil Brown

On 09/10/2010 01:50 AM, Jeff Layton wrote:
> On Fri, 10 Sep 2010 01:06:38 +0530
> Suresh Jayaraman <sjayaraman@suse.de> wrote:
> 
>> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
>> locks. Due to this, some windows applications which seem to use both flock
>> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
>> the same file, can't lock as they falsely assume the file is already locked.
>> The problem was reported on a setup with windows clients accessing excel files
>> on a Samba exported share which is originally a NFS mount from a NetApp filer.
>>
>> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
>> considered local. To support legacy flock behavior, this patch adds a mount
>> option "-olocal_lock=" which can take the following values:
>>
>>    'none'  		- Neither flock locks or fcntl/posix locks are local
>>    'flock'/'posix' 	- flock locks are local
> 	^^^^^^^
> "posix" ought to be synonymous with "fcntl". "flock" was a BSD-ism.

Oops, that should have read 'fcntl/posix' (though the code gets it right)..

> It may be better to keep it simple though and drop either "posix" or
> "fcntl". No need to add unneeded synonyms on a brand new mount option.

Makes sense.. Trond: which one is more consistent?


>>    'fcntl' 		- fcntl locks are local
>>    'all'		- Both flock locks and fcntl/posix locks are local
>>
>> Testing:
>>
>> This patch was tested by using -olocal_lock option with different values and
>> the NLM calls were noted from the network packet capture.
>>
>>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>>    	     was granted, fcntl was denied
>>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>>    	     granted
>>    'fcntl' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>>    'all'   - no NLM calls were seen during both flock() and fcntl()
>>
>> Also, with these patches ensured that the exisiting behavior of '-olock' and
>> '-onolock' didn't change.
>>
>> Cc: Neil Brown <neilb@suse.de>
>> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
>> ---
>>  fs/nfs/client.c           |    6 +++-
>>  fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
>>  fs/nfs/super.c            |   47 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/nfs_mount.h |    5 +++-
>>  4 files changed, 79 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index 4e7df2a..5f01f42 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>>   */
>>  static void nfs_destroy_server(struct nfs_server *server)
>>  {
>> -	if (!(server->flags & NFS_MOUNT_NONLM))
>> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
>> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>>  		nlmclnt_done(server->nlm_host);
>>  }
>>  
>> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>>  
>>  	if (nlm_init.nfs_version > 3)
>>  		return 0;
>> -	if (server->flags & NFS_MOUNT_NONLM)
>> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
>> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>>  		return 0;
>>  
>>  	switch (clp->cl_proto) {
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index eb51bd6..e338d37 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>  	return ret;
>>  }
>>  
>> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>> +static int
>> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>  {
>>  	struct inode *inode = filp->f_mapping->host;
>>  	int status = 0;
>> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>  	if (nfs_have_delegation(inode, FMODE_READ))
>>  		goto out_noconflict;
>>  
>> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
>> +	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
>>  		goto out_noconflict;
>>  
>>  	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>> @@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
>>  	return res;
>>  }
>>  
>> -static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
>> +static int
>> +do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>  {
>>  	struct inode *inode = filp->f_mapping->host;
>>  	int status;
>> @@ -746,14 +748,15 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
>>  	 * 	still need to complete the unlock.
>>  	 */
>>  	/* Use local locking if mounted with "-onolock" */
>> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
>> +	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
>>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>>  	else
>>  		status = do_vfs_lock(filp, fl);
>>  	return status;
>>  }
>>  
>> -static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
>> +static int
>> +do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>  {
>>  	struct inode *inode = filp->f_mapping->host;
>>  	int status;
>> @@ -767,7 +770,7 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
>>  		goto out;
>>  
>>  	/* Use local locking if mounted with "-onolock" */
>> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
>> +	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
>>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>>  	else
>>  		status = do_vfs_lock(filp, fl);
>> @@ -791,6 +794,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>>  {
>>  	struct inode *inode = filp->f_mapping->host;
>>  	int ret = -ENOLCK;
>> +	int is_local = 0;
>>  
>>  	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
>>  			filp->f_path.dentry->d_parent->d_name.name,
>> @@ -804,6 +808,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>>  	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
>>  		goto out_err;
>>  
>> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
>> +		is_local = 1;
>> +
>>  	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
>>  		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
>>  		if (ret < 0)
>> @@ -811,11 +818,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>>  	}
>>  
>>  	if (IS_GETLK(cmd))
>> -		ret = do_getlk(filp, cmd, fl);
>> +		ret = do_getlk(filp, cmd, fl, is_local);
>>  	else if (fl->fl_type == F_UNLCK)
>> -		ret = do_unlk(filp, cmd, fl);
>> +		ret = do_unlk(filp, cmd, fl, is_local);
>>  	else
>> -		ret = do_setlk(filp, cmd, fl);
>> +		ret = do_setlk(filp, cmd, fl, is_local);
>>  out_err:
>>  	return ret;
>>  }
>> @@ -825,6 +832,9 @@ out_err:
>>   */
>>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>>  {
>> +	struct inode *inode = filp->f_mapping->host;
>> +	int is_local = 0;
>> +
>>  	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
>>  			filp->f_path.dentry->d_parent->d_name.name,
>>  			filp->f_path.dentry->d_name.name,
>> @@ -833,14 +843,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>>  	if (!(fl->fl_flags & FL_FLOCK))
>>  		return -ENOLCK;
>>  
>> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
>> +		is_local = 1;
>> +
>>  	/* We're simulating flock() locks using posix locks on the server */
>>  	fl->fl_owner = (fl_owner_t)filp;
>>  	fl->fl_start = 0;
>>  	fl->fl_end = OFFSET_MAX;
>>  
>>  	if (fl->fl_type == F_UNLCK)
>> -		return do_unlk(filp, cmd, fl);
>> -	return do_setlk(filp, cmd, fl);
>> +		return do_unlk(filp, cmd, fl, is_local);
>> +	return do_setlk(filp, cmd, fl, is_local);
>>  }
>>  
>>  /*
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index ec3966e..c0d51dc 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -100,6 +100,7 @@ enum {
>>  	Opt_addr, Opt_mountaddr, Opt_clientaddr,
>>  	Opt_lookupcache,
>>  	Opt_fscache_uniq,
>> +	Opt_local_lock,
>>  
>>  	/* Special mount options */
>>  	Opt_userspace, Opt_deprecated, Opt_sloppy,
>> @@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>  
>>  	{ Opt_lookupcache, "lookupcache=%s" },
>>  	{ Opt_fscache_uniq, "fsc=%s" },
>> +	{ Opt_local_lock, "local_lock=%s" },
>>  
>>  	{ Opt_err, NULL }
>>  };
>> @@ -236,6 +238,23 @@ static match_table_t nfs_lookupcache_tokens = {
>>  	{ Opt_lookupcache_err, NULL }
>>  };
>>  
>> +enum {
>> +	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_fcntl,
>> +	Opt_local_lock_none,
>> +
>> +	Opt_local_lock_err
>> +};
>> +
>> +static match_table_t nfs_local_lock_tokens = {
>> +	{ Opt_local_lock_all, "all" },
>> +	{ Opt_local_lock_flock, "flock" },
>> +	{ Opt_local_lock_fcntl, "fcntl" },
>> +	{ Opt_local_lock_fcntl, "posix" },
>> +	{ Opt_local_lock_none, "none" },
>> +
>> +	{ Opt_local_lock_err, NULL }
>> +};
>> +
>>  
>>  static void nfs_umount_begin(struct super_block *);
>>  static int  nfs_statfs(struct dentry *, struct kstatfs *);
>> @@ -1412,6 +1431,34 @@ static int nfs_parse_mount_options(char *raw,
>>  			mnt->fscache_uniq = string;
>>  			mnt->options |= NFS_OPTION_FSCACHE;
>>  			break;
>> +		case Opt_local_lock:
>> +			string = match_strdup(args);
>> +			if (string == NULL)
>> +				goto out_nomem;
>> +			token = match_token(string, nfs_local_lock_tokens,
>> +					args);
>> +			kfree(string);
>> +			switch (token) {
>> +			case Opt_local_lock_all:
>> +				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
>> +					       NFS_MOUNT_LOCAL_FCNTL);
>> +				break;
>> +			case Opt_local_lock_flock:
>> +				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
>> +				break;
>> +			case Opt_local_lock_fcntl:
>> +				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
>> +				break;
>> +			case Opt_local_lock_none:
>> +				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
>> +						NFS_MOUNT_LOCAL_FCNTL);
>> +				break;
>> +			default:
>> +				dfprintk(MOUNT, "NFS:	invalid	"
>> +						"local_lock argument\n");
>> +				return 0;
>> +			};
>> +			break;
>>  
>>  		/*
>>  		 * Special options
>> diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
>> index 5d59ae8..652d7e9 100644
>> --- a/include/linux/nfs_mount.h
>> +++ b/include/linux/nfs_mount.h
>> @@ -56,7 +56,6 @@ struct nfs_mount_data {
>>  #define NFS_MOUNT_TCP		0x0040	/* 2 */
>>  #define NFS_MOUNT_VER3		0x0080	/* 3 */
>>  #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
>> -#define NFS_MOUNT_NONLM		0x0200	/* 3 */
>>  #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>>  #define NFS_MOUNT_NOACL		0x0800	/* 4 */
>>  #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
>> @@ -71,4 +70,8 @@ struct nfs_mount_data {
>>  #define NFS_MOUNT_NORESVPORT		0x40000
>>  #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
>>  
>> +#define NFS_MOUNT_LOCAL_FLOCK	0x100000
>> +#define NFS_MOUNT_LOCAL_FCNTL	0x200000
>> +#define NFS_MOUNT_NONLM	 	(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
>> +
>>  #endif
> 
> 


-- 
Suresh Jayaraman

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-09 23:14   ` Suresh Jayaraman
@ 2010-09-10  1:44     ` Trond Myklebust
  2010-09-10  4:07       ` Suresh Jayaraman
  0 siblings, 1 reply; 11+ messages in thread
From: Trond Myklebust @ 2010-09-10  1:44 UTC (permalink / raw)
  To: Suresh Jayaraman; +Cc: Jeff Layton, Linux NFS mailing list, Neil Brown

On Fri, 2010-09-10 at 04:44 +0530, Suresh Jayaraman wrote:
> On 09/10/2010 01:50 AM, Jeff Layton wrote:
> > On Fri, 10 Sep 2010 01:06:38 +0530
> > Suresh Jayaraman <sjayaraman@suse.de> wrote:
> > 
> >> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> >> locks. Due to this, some windows applications which seem to use both flock
> >> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> >> the same file, can't lock as they falsely assume the file is already locked.
> >> The problem was reported on a setup with windows clients accessing excel files
> >> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> >>
> >> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> >> considered local. To support legacy flock behavior, this patch adds a mount
> >> option "-olocal_lock=" which can take the following values:
> >>
> >>    'none'  		- Neither flock locks or fcntl/posix locks are local
> >>    'flock'/'posix' 	- flock locks are local
> > 	^^^^^^^
> > "posix" ought to be synonymous with "fcntl". "flock" was a BSD-ism.
> 
> Oops, that should have read 'fcntl/posix' (though the code gets it right)..

Yup. It appears to be a changelog bug...

> > It may be better to keep it simple though and drop either "posix" or
> > "fcntl". No need to add unneeded synonyms on a brand new mount option.
> 
> Makes sense.. Trond: which one is more consistent?

I have a slight preference for 'posix'. fcntl is an extensible
interface, which currently supports at least two different types of lock
('posix' and 'lease').

> >>    'fcntl' 		- fcntl locks are local
> >>    'all'		- Both flock locks and fcntl/posix locks are local
> >>

Cheers
  Trond

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-10  1:44     ` Trond Myklebust
@ 2010-09-10  4:07       ` Suresh Jayaraman
  2010-09-10  4:24         ` Neil Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-10  4:07 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Jeff Layton, Linux NFS mailing list, Neil Brown

On 09/10/2010 07:14 AM, Trond Myklebust wrote:
> On Fri, 2010-09-10 at 04:44 +0530, Suresh Jayaraman wrote:
>> On 09/10/2010 01:50 AM, Jeff Layton wrote:
>>> On Fri, 10 Sep 2010 01:06:38 +0530
>>> Suresh Jayaraman <sjayaraman@suse.de> wrote:
>>>
>>>> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
>>>> locks. Due to this, some windows applications which seem to use both flock
>>>> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
>>>> the same file, can't lock as they falsely assume the file is already locked.
>>>> The problem was reported on a setup with windows clients accessing excel files
>>>> on a Samba exported share which is originally a NFS mount from a NetApp filer.
>>>>
>>>> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
>>>> considered local. To support legacy flock behavior, this patch adds a mount
>>>> option "-olocal_lock=" which can take the following values:
>>>>
>>>>    'none'  		- Neither flock locks or fcntl/posix locks are local
>>>>    'flock'/'posix' 	- flock locks are local
>>> 	^^^^^^^
>>> "posix" ought to be synonymous with "fcntl". "flock" was a BSD-ism.
>>
>> Oops, that should have read 'fcntl/posix' (though the code gets it right)..
> 
> Yup. It appears to be a changelog bug...
> 
>>> It may be better to keep it simple though and drop either "posix" or
>>> "fcntl". No need to add unneeded synonyms on a brand new mount option.
>>
>> Makes sense.. Trond: which one is more consistent?
> 
> I have a slight preference for 'posix'. fcntl is an extensible

Ok, how about this one?

---
From: Suresh Jayaraman <sjayaraman@suse.de>
Subject: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local

NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
locks. Due to this, some windows applications which seem to use both flock
(share mode lock mapped as flock by Samba) and fcntl locks sequentially on
the same file, can't lock as they falsely assume the file is already locked.
The problem was reported on a setup with windows clients accessing excel files
on a Samba exported share which is originally a NFS mount from a NetApp filer.

Older NFS clients (< 2.6.12) did not see this problem as flock locks were
considered local. To support legacy flock behavior, this patch adds a mount
option "-olocal_lock=" which can take the following values:

   'none'  		- Neither flock locks nor POSIX locks are local
   'flock' 		- flock locks are local
   'posix' 		- fcntl/POSIX locks are local
   'all'		- Both flock locks and POSIX locks are local

Testing:

This patch was tested by using -olocal_lock option with different values and
the NLM calls were noted from the network packet captured.

   'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
   	     was granted, fcntl was denied
   'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
   	     granted
   'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
   'all'   - no NLM calls were seen during both flock() and fcntl()

Cc: Neil Brown <neilb@suse.de>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---
 fs/nfs/client.c           |    6 +++-
 fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
 fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_mount.h |    5 +++-
 4 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4e7df2a..5f01f42 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
  */
 static void nfs_destroy_server(struct nfs_server *server)
 {
-	if (!(server->flags & NFS_MOUNT_NONLM))
+	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
+			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		nlmclnt_done(server->nlm_host);
 }
 
@@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
 
 	if (nlm_init.nfs_version > 3)
 		return 0;
-	if (server->flags & NFS_MOUNT_NONLM)
+	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
+			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		return 0;
 
 	switch (clp->cl_proto) {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index eb51bd6..e338d37 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
 	return ret;
 }
 
-static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status = 0;
@@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (nfs_have_delegation(inode, FMODE_READ))
 		goto out_noconflict;
 
-	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
+	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
 		goto out_noconflict;
 
 	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
@@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
 	return res;
 }
 
-static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -746,14 +748,15 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
 	 * 	still need to complete the unlock.
 	 */
 	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
 	return status;
 }
 
-static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -767,7 +770,7 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
 		goto out;
 
 	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || !is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
@@ -791,6 +794,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int ret = -ENOLCK;
+	int is_local = 0;
 
 	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
@@ -804,6 +808,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		goto out_err;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
+		is_local = 1;
+
 	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
 		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
 		if (ret < 0)
@@ -811,11 +818,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	}
 
 	if (IS_GETLK(cmd))
-		ret = do_getlk(filp, cmd, fl);
+		ret = do_getlk(filp, cmd, fl, is_local);
 	else if (fl->fl_type == F_UNLCK)
-		ret = do_unlk(filp, cmd, fl);
+		ret = do_unlk(filp, cmd, fl, is_local);
 	else
-		ret = do_setlk(filp, cmd, fl);
+		ret = do_setlk(filp, cmd, fl, is_local);
 out_err:
 	return ret;
 }
@@ -825,6 +832,9 @@ out_err:
  */
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 {
+	struct inode *inode = filp->f_mapping->host;
+	int is_local = 0;
+
 	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
 			filp->f_path.dentry->d_name.name,
@@ -833,14 +843,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
+		is_local = 1;
+
 	/* We're simulating flock() locks using posix locks on the server */
 	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_start = 0;
 	fl->fl_end = OFFSET_MAX;
 
 	if (fl->fl_type == F_UNLCK)
-		return do_unlk(filp, cmd, fl);
-	return do_setlk(filp, cmd, fl);
+		return do_unlk(filp, cmd, fl, is_local);
+	return do_setlk(filp, cmd, fl, is_local);
 }
 
 /*
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ec3966e..78c08e9 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -100,6 +100,7 @@ enum {
 	Opt_addr, Opt_mountaddr, Opt_clientaddr,
 	Opt_lookupcache,
 	Opt_fscache_uniq,
+	Opt_local_lock,
 
 	/* Special mount options */
 	Opt_userspace, Opt_deprecated, Opt_sloppy,
@@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
 
 	{ Opt_lookupcache, "lookupcache=%s" },
 	{ Opt_fscache_uniq, "fsc=%s" },
+	{ Opt_local_lock, "local_lock=%s" },
 
 	{ Opt_err, NULL }
 };
@@ -236,6 +238,22 @@ static match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_err, NULL }
 };
 
+enum {
+	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_posix,
+	Opt_local_lock_none,
+
+	Opt_local_lock_err
+};
+
+static match_table_t nfs_local_lock_tokens = {
+	{ Opt_local_lock_all, "all" },
+	{ Opt_local_lock_flock, "flock" },
+	{ Opt_local_lock_posix, "posix" },
+	{ Opt_local_lock_none, "none" },
+
+	{ Opt_local_lock_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);
@@ -1412,6 +1430,34 @@ static int nfs_parse_mount_options(char *raw,
 			mnt->fscache_uniq = string;
 			mnt->options |= NFS_OPTION_FSCACHE;
 			break;
+		case Opt_local_lock:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string, nfs_local_lock_tokens,
+					args);
+			kfree(string);
+			switch (token) {
+			case Opt_local_lock_all:
+				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+					       NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			case Opt_local_lock_flock:
+				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
+				break;
+			case Opt_local_lock_posix:
+				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
+				break;
+			case Opt_local_lock_none:
+				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+						NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			default:
+				dfprintk(MOUNT, "NFS:	invalid	"
+						"local_lock argument\n");
+				return 0;
+			};
+			break;
 
 		/*
 		 * Special options
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 5d59ae8..d6a16cc 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -56,7 +56,6 @@ struct nfs_mount_data {
 #define NFS_MOUNT_TCP		0x0040	/* 2 */
 #define NFS_MOUNT_VER3		0x0080	/* 3 */
 #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
-#define NFS_MOUNT_NONLM		0x0200	/* 3 */
 #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
 #define NFS_MOUNT_NOACL		0x0800	/* 4 */
 #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
@@ -71,4 +70,8 @@ struct nfs_mount_data {
 #define NFS_MOUNT_NORESVPORT		0x40000
 #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
 
+#define NFS_MOUNT_LOCAL_FLOCK	0x100000
+#define NFS_MOUNT_LOCAL_FCNTL	0x200000
+#define NFS_MOUNT_NONLM		(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
+
 #endif

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-10  4:07       ` Suresh Jayaraman
@ 2010-09-10  4:24         ` Neil Brown
  2010-09-10  6:09           ` Suresh Jayaraman
  0 siblings, 1 reply; 11+ messages in thread
From: Neil Brown @ 2010-09-10  4:24 UTC (permalink / raw)
  To: Suresh Jayaraman; +Cc: Trond Myklebust, Jeff Layton, Linux NFS mailing list

On Fri, 10 Sep 2010 09:37:33 +0530
Suresh Jayaraman <sjayaraman@suse.de> wrote:

> On 09/10/2010 07:14 AM, Trond Myklebust wrote:
> > On Fri, 2010-09-10 at 04:44 +0530, Suresh Jayaraman wrote:
> >> On 09/10/2010 01:50 AM, Jeff Layton wrote:
> >>> On Fri, 10 Sep 2010 01:06:38 +0530
> >>> Suresh Jayaraman <sjayaraman@suse.de> wrote:
> >>>
> >>>> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> >>>> locks. Due to this, some windows applications which seem to use both flock
> >>>> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> >>>> the same file, can't lock as they falsely assume the file is already locked.
> >>>> The problem was reported on a setup with windows clients accessing excel files
> >>>> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> >>>>
> >>>> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> >>>> considered local. To support legacy flock behavior, this patch adds a mount
> >>>> option "-olocal_lock=" which can take the following values:
> >>>>
> >>>>    'none'  		- Neither flock locks or fcntl/posix locks are local
> >>>>    'flock'/'posix' 	- flock locks are local
> >>> 	^^^^^^^
> >>> "posix" ought to be synonymous with "fcntl". "flock" was a BSD-ism.
> >>
> >> Oops, that should have read 'fcntl/posix' (though the code gets it right)..
> > 
> > Yup. It appears to be a changelog bug...
> > 
> >>> It may be better to keep it simple though and drop either "posix" or
> >>> "fcntl". No need to add unneeded synonyms on a brand new mount option.
> >>
> >> Makes sense.. Trond: which one is more consistent?
> > 
> > I have a slight preference for 'posix'. fcntl is an extensible
> 
> Ok, how about this one?
> 
> ---
> From: Suresh Jayaraman <sjayaraman@suse.de>
> Subject: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
> 
> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> locks. Due to this, some windows applications which seem to use both flock
> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> the same file, can't lock as they falsely assume the file is already locked.
> The problem was reported on a setup with windows clients accessing excel files
> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> 
> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> considered local. To support legacy flock behavior, this patch adds a mount
> option "-olocal_lock=" which can take the following values:
> 
>    'none'  		- Neither flock locks nor POSIX locks are local
>    'flock' 		- flock locks are local
>    'posix' 		- fcntl/POSIX locks are local
>    'all'		- Both flock locks and POSIX locks are local
> 
> Testing:
> 
> This patch was tested by using -olocal_lock option with different values and
> the NLM calls were noted from the network packet captured.
> 
>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>    	     was granted, fcntl was denied
>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>    	     granted
>    'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>    'all'   - no NLM calls were seen during both flock() and fcntl()
> 
> Cc: Neil Brown <neilb@suse.de>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
>  fs/nfs/client.c           |    6 +++-
>  fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
>  fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/nfs_mount.h |    5 +++-
>  4 files changed, 78 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 4e7df2a..5f01f42 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>   */
>  static void nfs_destroy_server(struct nfs_server *server)
>  {
> -	if (!(server->flags & NFS_MOUNT_NONLM))
> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		nlmclnt_done(server->nlm_host);
>  }
>  
> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>  
>  	if (nlm_init.nfs_version > 3)
>  		return 0;
> -	if (server->flags & NFS_MOUNT_NONLM)
> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		return 0;
>  
>  	switch (clp->cl_proto) {
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index eb51bd6..e338d37 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>  	return ret;
>  }
>  
> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status = 0;
> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (nfs_have_delegation(inode, FMODE_READ))
>  		goto out_noconflict;
>  
> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
> +	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
>  		goto out_noconflict;

I cannot figure out why this isn't simply
        if (is_local)
		goto out_noconflict;

what am I missing?

Thanks,
NeilBrown

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-10  4:24         ` Neil Brown
@ 2010-09-10  6:09           ` Suresh Jayaraman
  2010-09-14  5:06             ` Suresh Jayaraman
  0 siblings, 1 reply; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-10  6:09 UTC (permalink / raw)
  To: Neil Brown, Trond Myklebust; +Cc: Jeff Layton, Linux NFS mailing list

On 09/10/2010 09:54 AM, Neil Brown wrote:
> On Fri, 10 Sep 2010 09:37:33 +0530
> Suresh Jayaraman <sjayaraman@suse.de> wrote:
>> ---
>> From: Suresh Jayaraman <sjayaraman@suse.de>
>> Subject: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
>>
>> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
>> locks. Due to this, some windows applications which seem to use both flock
>> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
>> the same file, can't lock as they falsely assume the file is already locked.
>> The problem was reported on a setup with windows clients accessing excel files
>> on a Samba exported share which is originally a NFS mount from a NetApp filer.
>>
>> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
>> considered local. To support legacy flock behavior, this patch adds a mount
>> option "-olocal_lock=" which can take the following values:
>>
>>    'none'  		- Neither flock locks nor POSIX locks are local
>>    'flock' 		- flock locks are local
>>    'posix' 		- fcntl/POSIX locks are local
>>    'all'		- Both flock locks and POSIX locks are local
>>
>> Testing:
>>
>> This patch was tested by using -olocal_lock option with different values and
>> the NLM calls were noted from the network packet captured.
>>
>>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>>    	     was granted, fcntl was denied
>>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>>    	     granted
>>    'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>>    'all'   - no NLM calls were seen during both flock() and fcntl()
>>
>> Cc: Neil Brown <neilb@suse.de>
>> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
>> ---
>>  fs/nfs/client.c           |    6 +++-
>>  fs/nfs/file.c             |   35 +++++++++++++++++++++++----------
>>  fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/nfs_mount.h |    5 +++-
>>  4 files changed, 78 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index 4e7df2a..5f01f42 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>>   */
>>  static void nfs_destroy_server(struct nfs_server *server)
>>  {
>> -	if (!(server->flags & NFS_MOUNT_NONLM))
>> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
>> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>>  		nlmclnt_done(server->nlm_host);
>>  }
>>  
>> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>>  
>>  	if (nlm_init.nfs_version > 3)
>>  		return 0;
>> -	if (server->flags & NFS_MOUNT_NONLM)
>> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
>> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>>  		return 0;
>>  
>>  	switch (clp->cl_proto) {
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index eb51bd6..e338d37 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>  	return ret;
>>  }
>>  
>> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>> +static int
>> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>  {
>>  	struct inode *inode = filp->f_mapping->host;
>>  	int status = 0;
>> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>  	if (nfs_have_delegation(inode, FMODE_READ))
>>  		goto out_noconflict;
>>  
>> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
>> +	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
>>  		goto out_noconflict;
> 
> I cannot figure out why this isn't simply
>         if (is_local)
> 		goto out_noconflict;
> 

Yes, just checking is_local is sufficient since we set NFS_MOUNT_NONLM to NFS_MOUNT_LOCAL_FLOCK|NFS_MOUNT_LOCAL_FCNTL. Here's the fixed version:

Changes since last post:
  - remove unneeded NFS_MOUNT_NONLM flag checks from do_getlk()/do_setlk()/do_unlck
  - update comments to include the new mount option


From: Suresh Jayaraman <sjayaraman@suse.de>
Subject: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local

NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
locks. Due to this, some windows applications which seem to use both flock
(share mode lock mapped as flock by Samba) and fcntl locks sequentially on
the same file, can't lock as they falsely assume the file is already locked.
The problem was reported on a setup with windows clients accessing excel files
on a Samba exported share which is originally a NFS mount from a NetApp filer.

Older NFS clients (< 2.6.12) did not see this problem as flock locks were
considered local. To support legacy flock behavior, this patch adds a mount
option "-olocal_lock=" which can take the following values:

   'none'  		- Neither flock locks nor POSIX locks are local
   'flock' 		- flock locks are local
   'posix' 		- fcntl/POSIX locks are local
   'all'		- Both flock locks and POSIX locks are local

Testing:

This patch was tested by using -olocal_lock option with different values and
the NLM calls were noted from the network packet captured.

   'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
   	     was granted, fcntl was denied
   'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
   	     granted
   'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
   'all'   - no NLM calls were seen during both flock() and fcntl()

Cc: Neil Brown <neilb@suse.de>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---
 fs/nfs/client.c           |    6 +++-
 fs/nfs/file.c             |   45 +++++++++++++++++++++++++++++++------------
 fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_mount.h |    5 +++-
 4 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4e7df2a..5f01f42 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
  */
 static void nfs_destroy_server(struct nfs_server *server)
 {
-	if (!(server->flags & NFS_MOUNT_NONLM))
+	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
+			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		nlmclnt_done(server->nlm_host);
 }
 
@@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
 
 	if (nlm_init.nfs_version > 3)
 		return 0;
-	if (server->flags & NFS_MOUNT_NONLM)
+	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
+			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		return 0;
 
 	switch (clp->cl_proto) {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index eb51bd6..59cbe1b 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
 	return ret;
 }
 
-static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status = 0;
@@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (nfs_have_delegation(inode, FMODE_READ))
 		goto out_noconflict;
 
-	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
+	if (is_local)
 		goto out_noconflict;
 
 	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
@@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
 	return res;
 }
 
-static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -745,15 +747,19 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
 	 * 	If we're signalled while cleaning up locks on process exit, we
 	 * 	still need to complete the unlock.
 	 */
-	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	/*
+	 * Use local locking if mounted with "-onolock" or with appropriate
+	 * "-olocal_lock="
+	 */
+	if (!is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
 	return status;
 }
 
-static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -766,8 +772,11 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (status != 0)
 		goto out;
 
-	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	/*
+	 * Use local locking if mounted with "-onolock" or with appropriate
+	 * "-olocal_lock="
+	 */
+	if (!is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
@@ -791,6 +800,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int ret = -ENOLCK;
+	int is_local = 0;
 
 	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
@@ -804,6 +814,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		goto out_err;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
+		is_local = 1;
+
 	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
 		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
 		if (ret < 0)
@@ -811,11 +824,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	}
 
 	if (IS_GETLK(cmd))
-		ret = do_getlk(filp, cmd, fl);
+		ret = do_getlk(filp, cmd, fl, is_local);
 	else if (fl->fl_type == F_UNLCK)
-		ret = do_unlk(filp, cmd, fl);
+		ret = do_unlk(filp, cmd, fl, is_local);
 	else
-		ret = do_setlk(filp, cmd, fl);
+		ret = do_setlk(filp, cmd, fl, is_local);
 out_err:
 	return ret;
 }
@@ -825,6 +838,9 @@ out_err:
  */
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 {
+	struct inode *inode = filp->f_mapping->host;
+	int is_local = 0;
+
 	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
 			filp->f_path.dentry->d_name.name,
@@ -833,14 +849,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
+		is_local = 1;
+
 	/* We're simulating flock() locks using posix locks on the server */
 	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_start = 0;
 	fl->fl_end = OFFSET_MAX;
 
 	if (fl->fl_type == F_UNLCK)
-		return do_unlk(filp, cmd, fl);
-	return do_setlk(filp, cmd, fl);
+		return do_unlk(filp, cmd, fl, is_local);
+	return do_setlk(filp, cmd, fl, is_local);
 }
 
 /*
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ec3966e..78c08e9 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -100,6 +100,7 @@ enum {
 	Opt_addr, Opt_mountaddr, Opt_clientaddr,
 	Opt_lookupcache,
 	Opt_fscache_uniq,
+	Opt_local_lock,
 
 	/* Special mount options */
 	Opt_userspace, Opt_deprecated, Opt_sloppy,
@@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
 
 	{ Opt_lookupcache, "lookupcache=%s" },
 	{ Opt_fscache_uniq, "fsc=%s" },
+	{ Opt_local_lock, "local_lock=%s" },
 
 	{ Opt_err, NULL }
 };
@@ -236,6 +238,22 @@ static match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_err, NULL }
 };
 
+enum {
+	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_posix,
+	Opt_local_lock_none,
+
+	Opt_local_lock_err
+};
+
+static match_table_t nfs_local_lock_tokens = {
+	{ Opt_local_lock_all, "all" },
+	{ Opt_local_lock_flock, "flock" },
+	{ Opt_local_lock_posix, "posix" },
+	{ Opt_local_lock_none, "none" },
+
+	{ Opt_local_lock_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);
@@ -1412,6 +1430,34 @@ static int nfs_parse_mount_options(char *raw,
 			mnt->fscache_uniq = string;
 			mnt->options |= NFS_OPTION_FSCACHE;
 			break;
+		case Opt_local_lock:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string, nfs_local_lock_tokens,
+					args);
+			kfree(string);
+			switch (token) {
+			case Opt_local_lock_all:
+				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+					       NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			case Opt_local_lock_flock:
+				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
+				break;
+			case Opt_local_lock_posix:
+				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
+				break;
+			case Opt_local_lock_none:
+				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+						NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			default:
+				dfprintk(MOUNT, "NFS:	invalid	"
+						"local_lock argument\n");
+				return 0;
+			};
+			break;
 
 		/*
 		 * Special options
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 5d59ae8..d6a16cc 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -56,7 +56,6 @@ struct nfs_mount_data {
 #define NFS_MOUNT_TCP		0x0040	/* 2 */
 #define NFS_MOUNT_VER3		0x0080	/* 3 */
 #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
-#define NFS_MOUNT_NONLM		0x0200	/* 3 */
 #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
 #define NFS_MOUNT_NOACL		0x0800	/* 4 */
 #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
@@ -71,4 +70,8 @@ struct nfs_mount_data {
 #define NFS_MOUNT_NORESVPORT		0x40000
 #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
 
+#define NFS_MOUNT_LOCAL_FLOCK	0x100000
+#define NFS_MOUNT_LOCAL_FCNTL	0x200000
+#define NFS_MOUNT_NONLM		(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
+
 #endif

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-10  6:09           ` Suresh Jayaraman
@ 2010-09-14  5:06             ` Suresh Jayaraman
  0 siblings, 0 replies; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-14  5:06 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Neil Brown, Jeff Layton, Linux NFS mailing list

On 09/10/2010 11:39 AM, Suresh Jayaraman wrote:
> On 09/10/2010 09:54 AM, Neil Brown wrote:
>>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>>> index eb51bd6..e338d37 100644
>>> --- a/fs/nfs/file.c
>>> +++ b/fs/nfs/file.c
>>> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>>  	return ret;
>>>  }
>>>  
>>> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>> +static int
>>> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>>  {
>>>  	struct inode *inode = filp->f_mapping->host;
>>>  	int status = 0;
>>> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>>  	if (nfs_have_delegation(inode, FMODE_READ))
>>>  		goto out_noconflict;
>>>  
>>> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
>>> +	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM) || is_local)
>>>  		goto out_noconflict;
>>
>> I cannot figure out why this isn't simply
>>         if (is_local)
>> 		goto out_noconflict;
>>
> 
> Yes, just checking is_local is sufficient since we set NFS_MOUNT_NONLM to NFS_MOUNT_LOCAL_FLOCK|NFS_MOUNT_LOCAL_FCNTL. Here's the fixed version:
> 
> Changes since last post:
>   - remove unneeded NFS_MOUNT_NONLM flag checks from do_getlk()/do_setlk()/do_unlck
>   - update comments to include the new mount option
> 

Trond: Does this patch look Ok or you want me to send in a separate
email? Just making sure that this doesn't get lost in huge pile of patches.


> From: Suresh Jayaraman <sjayaraman@suse.de>
> Subject: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
> 
> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> locks. Due to this, some windows applications which seem to use both flock
> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> the same file, can't lock as they falsely assume the file is already locked.
> The problem was reported on a setup with windows clients accessing excel files
> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> 
> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> considered local. To support legacy flock behavior, this patch adds a mount
> option "-olocal_lock=" which can take the following values:
> 
>    'none'  		- Neither flock locks nor POSIX locks are local
>    'flock' 		- flock locks are local
>    'posix' 		- fcntl/POSIX locks are local
>    'all'		- Both flock locks and POSIX locks are local
> 
> Testing:
> 
> This patch was tested by using -olocal_lock option with different values and
> the NLM calls were noted from the network packet captured.
> 
>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>    	     was granted, fcntl was denied
>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>    	     granted
>    'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>    'all'   - no NLM calls were seen during both flock() and fcntl()
> 
> Cc: Neil Brown <neilb@suse.de>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
>  fs/nfs/client.c           |    6 +++-
>  fs/nfs/file.c             |   45 +++++++++++++++++++++++++++++++------------
>  fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/nfs_mount.h |    5 +++-
>  4 files changed, 86 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 4e7df2a..5f01f42 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>   */
>  static void nfs_destroy_server(struct nfs_server *server)
>  {
> -	if (!(server->flags & NFS_MOUNT_NONLM))
> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		nlmclnt_done(server->nlm_host);
>  }
>  
> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>  
>  	if (nlm_init.nfs_version > 3)
>  		return 0;
> -	if (server->flags & NFS_MOUNT_NONLM)
> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		return 0;
>  
>  	switch (clp->cl_proto) {
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index eb51bd6..59cbe1b 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>  	return ret;
>  }
>  
> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status = 0;
> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (nfs_have_delegation(inode, FMODE_READ))
>  		goto out_noconflict;
>  
> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
> +	if (is_local)
>  		goto out_noconflict;
>  
>  	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> @@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
>  	return res;
>  }
>  
> -static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -745,15 +747,19 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
>  	 * 	If we're signalled while cleaning up locks on process exit, we
>  	 * 	still need to complete the unlock.
>  	 */
> -	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	/*
> +	 * Use local locking if mounted with "-onolock" or with appropriate
> +	 * "-olocal_lock="
> +	 */
> +	if (!is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
>  	return status;
>  }
>  
> -static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -766,8 +772,11 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (status != 0)
>  		goto out;
>  
> -	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	/*
> +	 * Use local locking if mounted with "-onolock" or with appropriate
> +	 * "-olocal_lock="
> +	 */
> +	if (!is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
> @@ -791,6 +800,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int ret = -ENOLCK;
> +	int is_local = 0;
>  
>  	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
> @@ -804,6 +814,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
>  		goto out_err;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
> +		is_local = 1;
> +
>  	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
>  		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
>  		if (ret < 0)
> @@ -811,11 +824,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	}
>  
>  	if (IS_GETLK(cmd))
> -		ret = do_getlk(filp, cmd, fl);
> +		ret = do_getlk(filp, cmd, fl, is_local);
>  	else if (fl->fl_type == F_UNLCK)
> -		ret = do_unlk(filp, cmd, fl);
> +		ret = do_unlk(filp, cmd, fl, is_local);
>  	else
> -		ret = do_setlk(filp, cmd, fl);
> +		ret = do_setlk(filp, cmd, fl, is_local);
>  out_err:
>  	return ret;
>  }
> @@ -825,6 +838,9 @@ out_err:
>   */
>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  {
> +	struct inode *inode = filp->f_mapping->host;
> +	int is_local = 0;
> +
>  	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
>  			filp->f_path.dentry->d_name.name,
> @@ -833,14 +849,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
> +		is_local = 1;
> +
>  	/* We're simulating flock() locks using posix locks on the server */
>  	fl->fl_owner = (fl_owner_t)filp;
>  	fl->fl_start = 0;
>  	fl->fl_end = OFFSET_MAX;
>  
>  	if (fl->fl_type == F_UNLCK)
> -		return do_unlk(filp, cmd, fl);
> -	return do_setlk(filp, cmd, fl);
> +		return do_unlk(filp, cmd, fl, is_local);
> +	return do_setlk(filp, cmd, fl, is_local);
>  }
>  
>  /*
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index ec3966e..78c08e9 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -100,6 +100,7 @@ enum {
>  	Opt_addr, Opt_mountaddr, Opt_clientaddr,
>  	Opt_lookupcache,
>  	Opt_fscache_uniq,
> +	Opt_local_lock,
>  
>  	/* Special mount options */
>  	Opt_userspace, Opt_deprecated, Opt_sloppy,
> @@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
>  
>  	{ Opt_lookupcache, "lookupcache=%s" },
>  	{ Opt_fscache_uniq, "fsc=%s" },
> +	{ Opt_local_lock, "local_lock=%s" },
>  
>  	{ Opt_err, NULL }
>  };
> @@ -236,6 +238,22 @@ static match_table_t nfs_lookupcache_tokens = {
>  	{ Opt_lookupcache_err, NULL }
>  };
>  
> +enum {
> +	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_posix,
> +	Opt_local_lock_none,
> +
> +	Opt_local_lock_err
> +};
> +
> +static match_table_t nfs_local_lock_tokens = {
> +	{ Opt_local_lock_all, "all" },
> +	{ Opt_local_lock_flock, "flock" },
> +	{ Opt_local_lock_posix, "posix" },
> +	{ Opt_local_lock_none, "none" },
> +
> +	{ Opt_local_lock_err, NULL }
> +};
> +
>  
>  static void nfs_umount_begin(struct super_block *);
>  static int  nfs_statfs(struct dentry *, struct kstatfs *);
> @@ -1412,6 +1430,34 @@ static int nfs_parse_mount_options(char *raw,
>  			mnt->fscache_uniq = string;
>  			mnt->options |= NFS_OPTION_FSCACHE;
>  			break;
> +		case Opt_local_lock:
> +			string = match_strdup(args);
> +			if (string == NULL)
> +				goto out_nomem;
> +			token = match_token(string, nfs_local_lock_tokens,
> +					args);
> +			kfree(string);
> +			switch (token) {
> +			case Opt_local_lock_all:
> +				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
> +					       NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			case Opt_local_lock_flock:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
> +				break;
> +			case Opt_local_lock_posix:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
> +				break;
> +			case Opt_local_lock_none:
> +				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
> +						NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			default:
> +				dfprintk(MOUNT, "NFS:	invalid	"
> +						"local_lock argument\n");
> +				return 0;
> +			};
> +			break;
>  
>  		/*
>  		 * Special options
> diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
> index 5d59ae8..d6a16cc 100644
> --- a/include/linux/nfs_mount.h
> +++ b/include/linux/nfs_mount.h
> @@ -56,7 +56,6 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_TCP		0x0040	/* 2 */
>  #define NFS_MOUNT_VER3		0x0080	/* 3 */
>  #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
> -#define NFS_MOUNT_NONLM		0x0200	/* 3 */
>  #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>  #define NFS_MOUNT_NOACL		0x0800	/* 4 */
>  #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
> @@ -71,4 +70,8 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_NORESVPORT		0x40000
>  #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
>  
> +#define NFS_MOUNT_LOCAL_FLOCK	0x100000
> +#define NFS_MOUNT_LOCAL_FCNTL	0x200000
> +#define NFS_MOUNT_NONLM		(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
> +
>  #endif
> --
> 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


-- 
Suresh Jayaraman

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-17 22:15 ` Trond Myklebust
@ 2010-09-20  9:27   ` Suresh Jayaraman
  0 siblings, 0 replies; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-20  9:27 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Neil Brown, Linux NFS mailing list

On 09/18/2010 03:45 AM, Trond Myklebust wrote:
> On Thu, 2010-09-16 at 11:44 +0530, Suresh Jayaraman wrote:
>> Hi Trond,
>>
>> I'm resending this patch as a separate email as I have not heard from you.
>>
>> Changes since last post:
>>   - remove unneeded NFS_MOUNT_NONLM flag checks from do_getlk()/do_setlk()/do_unlck
>>   - update comments to include the new mount option
>>
> 
> Hi Suresh,
> 
> Just a couple of comments:
> 
>       * Firstly the legacy binary mount interface uses
>         NFS_MOUNT_NONLM=0x200, so we cannot change that value. What I
>         was suggesting was rather that we just parse it to mean
>         NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL.

Ah, ok. Will fix it in the next spin.

>       * Have you tested this with NFSv4? I think I asked you this
>         previously, but I haven't seen an answer. I ask because -onolock
>         used to cause NFSv4 reboot recovery to Oops since the struct
>         file_lock didn't have any associated nfsv4 lock state. I just
>         want to make sure we're not re-enabling that bug.
> 

I had earlier tested the patch with NFSv4 but I didn't test the reboot
recovery. And, you guessed it right - with "-olocal_lock=all", the patch
causes an oops in _nfs4_do_setlk() when the server recovers after a
reboot. Clearing the new flags in nfs4_validate_mount_flags() fixes the
problem. Also, I have updated the patch to take into account the nfsroot
case.

I also noticed Chuck's recent patchset will move nfsroot option
handling. I'll rebase by patch against nfs-2.6.git and resend it.

Thanks,

-- 
Suresh Jayaraman

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
  2010-09-16  6:14 Suresh Jayaraman
@ 2010-09-17 22:15 ` Trond Myklebust
  2010-09-20  9:27   ` Suresh Jayaraman
  0 siblings, 1 reply; 11+ messages in thread
From: Trond Myklebust @ 2010-09-17 22:15 UTC (permalink / raw)
  To: Suresh Jayaraman; +Cc: Neil Brown, Linux NFS mailing list

On Thu, 2010-09-16 at 11:44 +0530, Suresh Jayaraman wrote:
> Hi Trond,
> 
> I'm resending this patch as a separate email as I have not heard from you.
> 
> Changes since last post:
>   - remove unneeded NFS_MOUNT_NONLM flag checks from do_getlk()/do_setlk()/do_unlck
>   - update comments to include the new mount option
> 

Hi Suresh,

Just a couple of comments:

      * Firstly the legacy binary mount interface uses
        NFS_MOUNT_NONLM=0x200, so we cannot change that value. What I
        was suggesting was rather that we just parse it to mean
        NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL.
      * Have you tested this with NFSv4? I think I asked you this
        previously, but I haven't seen an answer. I ask because -onolock
        used to cause NFSv4 reboot recovery to Oops since the struct
        file_lock didn't have any associated nfsv4 lock state. I just
        want to make sure we're not re-enabling that bug.

Cheers
  Trond

> Thanks,
> Suresh Jayaraman
> ---
> 
> NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
> locks. Due to this, some windows applications which seem to use both flock
> (share mode lock mapped as flock by Samba) and fcntl locks sequentially on
> the same file, can't lock as they falsely assume the file is already locked.
> The problem was reported on a setup with windows clients accessing excel files
> on a Samba exported share which is originally a NFS mount from a NetApp filer.
> 
> Older NFS clients (< 2.6.12) did not see this problem as flock locks were
> considered local. To support legacy flock behavior, this patch adds a mount
> option "-olocal_lock=" which can take the following values:
> 
>    'none'  		- Neither flock locks nor POSIX locks are local
>    'flock' 		- flock locks are local
>    'posix' 		- fcntl/POSIX locks are local
>    'all'		- Both flock locks and POSIX locks are local
> 
> Testing:
> 
> This patch was tested by using -olocal_lock option with different values and
> the NLM calls were noted from the network packet captured.
> 
>    'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
>    	     was granted, fcntl was denied
>    'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
>    	     granted
>    'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
>    'all'   - no NLM calls were seen during both flock() and fcntl()
> 
> Cc: Neil Brown <neilb@suse.de>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
>  fs/nfs/client.c           |    6 +++-
>  fs/nfs/file.c             |   45 +++++++++++++++++++++++++++++++------------
>  fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/nfs_mount.h |    5 +++-
>  4 files changed, 86 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 4e7df2a..5f01f42 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
>   */
>  static void nfs_destroy_server(struct nfs_server *server)
>  {
> -	if (!(server->flags & NFS_MOUNT_NONLM))
> +	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
> +			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		nlmclnt_done(server->nlm_host);
>  }
>  
> @@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
>  
>  	if (nlm_init.nfs_version > 3)
>  		return 0;
> -	if (server->flags & NFS_MOUNT_NONLM)
> +	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
> +			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
>  		return 0;
>  
>  	switch (clp->cl_proto) {
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index eb51bd6..59cbe1b 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>  	return ret;
>  }
>  
> -static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status = 0;
> @@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (nfs_have_delegation(inode, FMODE_READ))
>  		goto out_noconflict;
>  
> -	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
> +	if (is_local)
>  		goto out_noconflict;
>  
>  	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> @@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
>  	return res;
>  }
>  
> -static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -745,15 +747,19 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
>  	 * 	If we're signalled while cleaning up locks on process exit, we
>  	 * 	still need to complete the unlock.
>  	 */
> -	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	/*
> +	 * Use local locking if mounted with "-onolock" or with appropriate
> +	 * "-olocal_lock="
> +	 */
> +	if (!is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
>  	return status;
>  }
>  
> -static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
> +static int
> +do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int status;
> @@ -766,8 +772,11 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
>  	if (status != 0)
>  		goto out;
>  
> -	/* Use local locking if mounted with "-onolock" */
> -	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
> +	/*
> +	 * Use local locking if mounted with "-onolock" or with appropriate
> +	 * "-olocal_lock="
> +	 */
> +	if (!is_local)
>  		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
>  	else
>  		status = do_vfs_lock(filp, fl);
> @@ -791,6 +800,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  {
>  	struct inode *inode = filp->f_mapping->host;
>  	int ret = -ENOLCK;
> +	int is_local = 0;
>  
>  	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
> @@ -804,6 +814,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
>  		goto out_err;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
> +		is_local = 1;
> +
>  	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
>  		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
>  		if (ret < 0)
> @@ -811,11 +824,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	}
>  
>  	if (IS_GETLK(cmd))
> -		ret = do_getlk(filp, cmd, fl);
> +		ret = do_getlk(filp, cmd, fl, is_local);
>  	else if (fl->fl_type == F_UNLCK)
> -		ret = do_unlk(filp, cmd, fl);
> +		ret = do_unlk(filp, cmd, fl, is_local);
>  	else
> -		ret = do_setlk(filp, cmd, fl);
> +		ret = do_setlk(filp, cmd, fl, is_local);
>  out_err:
>  	return ret;
>  }
> @@ -825,6 +838,9 @@ out_err:
>   */
>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  {
> +	struct inode *inode = filp->f_mapping->host;
> +	int is_local = 0;
> +
>  	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
>  			filp->f_path.dentry->d_parent->d_name.name,
>  			filp->f_path.dentry->d_name.name,
> @@ -833,14 +849,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (!(fl->fl_flags & FL_FLOCK))
>  		return -ENOLCK;
>  
> +	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
> +		is_local = 1;
> +
>  	/* We're simulating flock() locks using posix locks on the server */
>  	fl->fl_owner = (fl_owner_t)filp;
>  	fl->fl_start = 0;
>  	fl->fl_end = OFFSET_MAX;
>  
>  	if (fl->fl_type == F_UNLCK)
> -		return do_unlk(filp, cmd, fl);
> -	return do_setlk(filp, cmd, fl);
> +		return do_unlk(filp, cmd, fl, is_local);
> +	return do_setlk(filp, cmd, fl, is_local);
>  }
>  
>  /*
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index ec3966e..78c08e9 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -100,6 +100,7 @@ enum {
>  	Opt_addr, Opt_mountaddr, Opt_clientaddr,
>  	Opt_lookupcache,
>  	Opt_fscache_uniq,
> +	Opt_local_lock,
>  
>  	/* Special mount options */
>  	Opt_userspace, Opt_deprecated, Opt_sloppy,
> @@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
>  
>  	{ Opt_lookupcache, "lookupcache=%s" },
>  	{ Opt_fscache_uniq, "fsc=%s" },
> +	{ Opt_local_lock, "local_lock=%s" },
>  
>  	{ Opt_err, NULL }
>  };
> @@ -236,6 +238,22 @@ static match_table_t nfs_lookupcache_tokens = {
>  	{ Opt_lookupcache_err, NULL }
>  };
>  
> +enum {
> +	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_posix,
> +	Opt_local_lock_none,
> +
> +	Opt_local_lock_err
> +};
> +
> +static match_table_t nfs_local_lock_tokens = {
> +	{ Opt_local_lock_all, "all" },
> +	{ Opt_local_lock_flock, "flock" },
> +	{ Opt_local_lock_posix, "posix" },
> +	{ Opt_local_lock_none, "none" },
> +
> +	{ Opt_local_lock_err, NULL }
> +};
> +
>  
>  static void nfs_umount_begin(struct super_block *);
>  static int  nfs_statfs(struct dentry *, struct kstatfs *);
> @@ -1412,6 +1430,34 @@ static int nfs_parse_mount_options(char *raw,
>  			mnt->fscache_uniq = string;
>  			mnt->options |= NFS_OPTION_FSCACHE;
>  			break;
> +		case Opt_local_lock:
> +			string = match_strdup(args);
> +			if (string == NULL)
> +				goto out_nomem;
> +			token = match_token(string, nfs_local_lock_tokens,
> +					args);
> +			kfree(string);
> +			switch (token) {
> +			case Opt_local_lock_all:
> +				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
> +					       NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			case Opt_local_lock_flock:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
> +				break;
> +			case Opt_local_lock_posix:
> +				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
> +				break;
> +			case Opt_local_lock_none:
> +				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
> +						NFS_MOUNT_LOCAL_FCNTL);
> +				break;
> +			default:
> +				dfprintk(MOUNT, "NFS:	invalid	"
> +						"local_lock argument\n");
> +				return 0;
> +			};
> +			break;
>  
>  		/*
>  		 * Special options
> diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
> index 5d59ae8..d6a16cc 100644
> --- a/include/linux/nfs_mount.h
> +++ b/include/linux/nfs_mount.h
> @@ -56,7 +56,6 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_TCP		0x0040	/* 2 */
>  #define NFS_MOUNT_VER3		0x0080	/* 3 */
>  #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
> -#define NFS_MOUNT_NONLM		0x0200	/* 3 */
>  #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>  #define NFS_MOUNT_NOACL		0x0800	/* 4 */
>  #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
> @@ -71,4 +70,8 @@ struct nfs_mount_data {
>  #define NFS_MOUNT_NORESVPORT		0x40000
>  #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
>  
> +#define NFS_MOUNT_LOCAL_FLOCK	0x100000
> +#define NFS_MOUNT_LOCAL_FCNTL	0x200000
> +#define NFS_MOUNT_NONLM		(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
> +
>  #endif
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local
@ 2010-09-16  6:14 Suresh Jayaraman
  2010-09-17 22:15 ` Trond Myklebust
  0 siblings, 1 reply; 11+ messages in thread
From: Suresh Jayaraman @ 2010-09-16  6:14 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Neil Brown, Linux NFS mailing list

Hi Trond,

I'm resending this patch as a separate email as I have not heard from you.

Changes since last post:
  - remove unneeded NFS_MOUNT_NONLM flag checks from do_getlk()/do_setlk()/do_unlck
  - update comments to include the new mount option


Thanks,
Suresh Jayaraman
---

NFS clients since 2.6.12 support flock locks by emulating fcntl byte-range
locks. Due to this, some windows applications which seem to use both flock
(share mode lock mapped as flock by Samba) and fcntl locks sequentially on
the same file, can't lock as they falsely assume the file is already locked.
The problem was reported on a setup with windows clients accessing excel files
on a Samba exported share which is originally a NFS mount from a NetApp filer.

Older NFS clients (< 2.6.12) did not see this problem as flock locks were
considered local. To support legacy flock behavior, this patch adds a mount
option "-olocal_lock=" which can take the following values:

   'none'  		- Neither flock locks nor POSIX locks are local
   'flock' 		- flock locks are local
   'posix' 		- fcntl/POSIX locks are local
   'all'		- Both flock locks and POSIX locks are local

Testing:

This patch was tested by using -olocal_lock option with different values and
the NLM calls were noted from the network packet captured.

   'none'  - NLM calls were seen during both flock() and fcntl(), flock lock
   	     was granted, fcntl was denied
   'flock' - no NLM calls for flock(), NLM call was seen for fcntl(),
   	     granted
   'posix' - NLM call was seen for flock() - granted, no NLM call for fcntl()
   'all'   - no NLM calls were seen during both flock() and fcntl()

Cc: Neil Brown <neilb@suse.de>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---
 fs/nfs/client.c           |    6 +++-
 fs/nfs/file.c             |   45 +++++++++++++++++++++++++++++++------------
 fs/nfs/super.c            |   46 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_mount.h |    5 +++-
 4 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4e7df2a..5f01f42 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -635,7 +635,8 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
  */
 static void nfs_destroy_server(struct nfs_server *server)
 {
-	if (!(server->flags & NFS_MOUNT_NONLM))
+	if (!(server->flags & NFS_MOUNT_LOCAL_FLOCK) ||
+			!(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		nlmclnt_done(server->nlm_host);
 }
 
@@ -657,7 +658,8 @@ static int nfs_start_lockd(struct nfs_server *server)
 
 	if (nlm_init.nfs_version > 3)
 		return 0;
-	if (server->flags & NFS_MOUNT_NONLM)
+	if ((server->flags & NFS_MOUNT_LOCAL_FLOCK) &&
+			(server->flags & NFS_MOUNT_LOCAL_FCNTL))
 		return 0;
 
 	switch (clp->cl_proto) {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index eb51bd6..59cbe1b 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -684,7 +684,8 @@ static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
 	return ret;
 }
 
-static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_getlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status = 0;
@@ -699,7 +700,7 @@ static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (nfs_have_delegation(inode, FMODE_READ))
 		goto out_noconflict;
 
-	if (NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM)
+	if (is_local)
 		goto out_noconflict;
 
 	status = NFS_PROTO(inode)->lock(filp, cmd, fl);
@@ -730,7 +731,8 @@ static int do_vfs_lock(struct file *file, struct file_lock *fl)
 	return res;
 }
 
-static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -745,15 +747,19 @@ static int do_unlk(struct file *filp, int cmd, struct file_lock *fl)
 	 * 	If we're signalled while cleaning up locks on process exit, we
 	 * 	still need to complete the unlock.
 	 */
-	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	/*
+	 * Use local locking if mounted with "-onolock" or with appropriate
+	 * "-olocal_lock="
+	 */
+	if (!is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
 	return status;
 }
 
-static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
+static int
+do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int status;
@@ -766,8 +772,11 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
 	if (status != 0)
 		goto out;
 
-	/* Use local locking if mounted with "-onolock" */
-	if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NONLM))
+	/*
+	 * Use local locking if mounted with "-onolock" or with appropriate
+	 * "-olocal_lock="
+	 */
+	if (!is_local)
 		status = NFS_PROTO(inode)->lock(filp, cmd, fl);
 	else
 		status = do_vfs_lock(filp, fl);
@@ -791,6 +800,7 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
 	struct inode *inode = filp->f_mapping->host;
 	int ret = -ENOLCK;
+	int is_local = 0;
 
 	dprintk("NFS: lock(%s/%s, t=%x, fl=%x, r=%lld:%lld)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
@@ -804,6 +814,9 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		goto out_err;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
+		is_local = 1;
+
 	if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
 		ret = NFS_PROTO(inode)->lock_check_bounds(fl);
 		if (ret < 0)
@@ -811,11 +824,11 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
 	}
 
 	if (IS_GETLK(cmd))
-		ret = do_getlk(filp, cmd, fl);
+		ret = do_getlk(filp, cmd, fl, is_local);
 	else if (fl->fl_type == F_UNLCK)
-		ret = do_unlk(filp, cmd, fl);
+		ret = do_unlk(filp, cmd, fl, is_local);
 	else
-		ret = do_setlk(filp, cmd, fl);
+		ret = do_setlk(filp, cmd, fl, is_local);
 out_err:
 	return ret;
 }
@@ -825,6 +838,9 @@ out_err:
  */
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 {
+	struct inode *inode = filp->f_mapping->host;
+	int is_local = 0;
+
 	dprintk("NFS: flock(%s/%s, t=%x, fl=%x)\n",
 			filp->f_path.dentry->d_parent->d_name.name,
 			filp->f_path.dentry->d_name.name,
@@ -833,14 +849,17 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;
 
+	if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK)
+		is_local = 1;
+
 	/* We're simulating flock() locks using posix locks on the server */
 	fl->fl_owner = (fl_owner_t)filp;
 	fl->fl_start = 0;
 	fl->fl_end = OFFSET_MAX;
 
 	if (fl->fl_type == F_UNLCK)
-		return do_unlk(filp, cmd, fl);
-	return do_setlk(filp, cmd, fl);
+		return do_unlk(filp, cmd, fl, is_local);
+	return do_setlk(filp, cmd, fl, is_local);
 }
 
 /*
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ec3966e..78c08e9 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -100,6 +100,7 @@ enum {
 	Opt_addr, Opt_mountaddr, Opt_clientaddr,
 	Opt_lookupcache,
 	Opt_fscache_uniq,
+	Opt_local_lock,
 
 	/* Special mount options */
 	Opt_userspace, Opt_deprecated, Opt_sloppy,
@@ -171,6 +172,7 @@ static const match_table_t nfs_mount_option_tokens = {
 
 	{ Opt_lookupcache, "lookupcache=%s" },
 	{ Opt_fscache_uniq, "fsc=%s" },
+	{ Opt_local_lock, "local_lock=%s" },
 
 	{ Opt_err, NULL }
 };
@@ -236,6 +238,22 @@ static match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_err, NULL }
 };
 
+enum {
+	Opt_local_lock_all, Opt_local_lock_flock, Opt_local_lock_posix,
+	Opt_local_lock_none,
+
+	Opt_local_lock_err
+};
+
+static match_table_t nfs_local_lock_tokens = {
+	{ Opt_local_lock_all, "all" },
+	{ Opt_local_lock_flock, "flock" },
+	{ Opt_local_lock_posix, "posix" },
+	{ Opt_local_lock_none, "none" },
+
+	{ Opt_local_lock_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);
@@ -1412,6 +1430,34 @@ static int nfs_parse_mount_options(char *raw,
 			mnt->fscache_uniq = string;
 			mnt->options |= NFS_OPTION_FSCACHE;
 			break;
+		case Opt_local_lock:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string, nfs_local_lock_tokens,
+					args);
+			kfree(string);
+			switch (token) {
+			case Opt_local_lock_all:
+				mnt->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+					       NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			case Opt_local_lock_flock:
+				mnt->flags |= NFS_MOUNT_LOCAL_FLOCK;
+				break;
+			case Opt_local_lock_posix:
+				mnt->flags |= NFS_MOUNT_LOCAL_FCNTL;
+				break;
+			case Opt_local_lock_none:
+				mnt->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+						NFS_MOUNT_LOCAL_FCNTL);
+				break;
+			default:
+				dfprintk(MOUNT, "NFS:	invalid	"
+						"local_lock argument\n");
+				return 0;
+			};
+			break;
 
 		/*
 		 * Special options
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 5d59ae8..d6a16cc 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -56,7 +56,6 @@ struct nfs_mount_data {
 #define NFS_MOUNT_TCP		0x0040	/* 2 */
 #define NFS_MOUNT_VER3		0x0080	/* 3 */
 #define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
-#define NFS_MOUNT_NONLM		0x0200	/* 3 */
 #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
 #define NFS_MOUNT_NOACL		0x0800	/* 4 */
 #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
@@ -71,4 +70,8 @@ struct nfs_mount_data {
 #define NFS_MOUNT_NORESVPORT		0x40000
 #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
 
+#define NFS_MOUNT_LOCAL_FLOCK	0x100000
+#define NFS_MOUNT_LOCAL_FCNTL	0x200000
+#define NFS_MOUNT_NONLM		(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL)
+
 #endif


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-09-20  9:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-09 19:36 [PATCH] nfs: introduce mount option '-olocal_lock' to make locks local Suresh Jayaraman
2010-09-09 20:20 ` Jeff Layton
2010-09-09 23:14   ` Suresh Jayaraman
2010-09-10  1:44     ` Trond Myklebust
2010-09-10  4:07       ` Suresh Jayaraman
2010-09-10  4:24         ` Neil Brown
2010-09-10  6:09           ` Suresh Jayaraman
2010-09-14  5:06             ` Suresh Jayaraman
2010-09-16  6:14 Suresh Jayaraman
2010-09-17 22:15 ` Trond Myklebust
2010-09-20  9:27   ` Suresh Jayaraman

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.