public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Suresh Jayaraman <sjayaraman@suse.de>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Neil Brown <neilb@suse.de>,
	Linux NFS mailing list <linux-nfs@vger.kernel.org>
Subject: [RFC][PATCH] nfs: support legacy NFS flock behavior via mount option
Date: Mon, 06 Sep 2010 18:03:43 +0530	[thread overview]
Message-ID: <4C84DFA7.7050507@suse.de> (raw)

NFS clients since 2.6.12 support flock()locks by emulating the
BSD-style locks in terms of POSIX byte range locks. So the NFS client
does not allow to lock the same file using both flock() and fcntl
byte-range locks.

For some Windows applications which seem to use both share mode locks
(flock()) and fcntl byte range locks sequentially on the same file,
the locking is failing as the lock has already been acquired. i.e. the
flock mapped as posix locks collide with actual byte range locks from
the same process. The problem was observed on a setup with Windows
clients accessing Excel files on a Samba exported share which is
originally a NFS mount from a NetApp filer. Since kernels < 2.6.12 does
not support flock, what was working (as flock locks were local) in
older kernels is not working with newer kernels.

This could be seen as a bug in the implementation of the windows
application or a NFS client regression, but that is debatable.
In the spirit of not breaking existing setups, this patch adds mount
options "flock=local" that enables older flock behavior and
"flock=fcntl" that allows the current flock behavior.

Here's the test program used to test the locking behavior:

/*
 * nfs-lock: Simple program to lock a file using flock and then using fcntl
 * Used to Test flock behavior on NFS (to be run on NFS mounts)
 *      Usage: ./nfs-lock <file> 
 */
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/file.h>

void usage()
{
        fprintf(stdout, "Usage: nfs-lock <file>\n");
        exit(1);
}

int main(int argc, char *argv[])
{
        char *file = argv[1];
        struct flock lock;
        int fd, rc;

        if (argc !=2)
                usage();

        fd = open(file, O_RDWR);
        if (fd < 0) {
                perror("open");
                return 1;
        }

        /* acquire flock */
        printf("nfs-lock: Trying to acquire flock lock\n");
        rc = flock(fd, LOCK_EX);
        if (rc)
                perror("flock");

        memset(&lock, 0, sizeof(lock));
        lock.l_type = F_WRLCK;
        printf("nfs-lock: Trying to acquire fcntl lock\n");
        rc = fcntl(fd, F_SETLK, &lock);
        if (rc) {
                perror("fcntl");
                return 1;
        }
        printf("nfs-lock: fcntl obtained successfully on the file\n");

        flock(fd, LOCK_UN);
        fcntl(fd, F_UNLCK, &lock);
        close(fd);

        return 0;
}

Cc: Neil Brown <neilb@suse.de>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
---
 fs/nfs/file.c             |    6 +++++-
 fs/nfs/super.c            |   33 +++++++++++++++++++++++++++++++++
 include/linux/nfs_mount.h |    2 ++
 3 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index eb51bd6..2384382 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -825,12 +825,16 @@ out_err:
  */
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 {
+	struct inode *inode = filp->f_mapping->host;
+
 	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,
 			fl->fl_type, fl->fl_flags);
 
-	if (!(fl->fl_flags & FL_FLOCK))
+	/* flock is considered local if mounted with "-o flock=local" */
+	if ((NFS_SERVER(inode)->flags & NFS_MOUNT_FLOCK_LOCAL) ||
+			(!(fl->fl_flags & FL_FLOCK)))
 		return -ENOLCK;
 
 	/* We're simulating flock() locks using posix locks on the server */
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ec3966e..65c780d 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_flock,
 
 	/* 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_flock, "flock=%s" },
 
 	{ Opt_err, NULL }
 };
@@ -236,6 +238,18 @@ static match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_err, NULL }
 };
 
+enum {
+	Opt_flock_local, Opt_flock_fcntl,
+	Opt_flock_err
+};
+
+static match_table_t nfs_flock_tokens = {
+	{ Opt_flock_local, "local" },
+	{ Opt_flock_fcntl, "fcntl" },
+
+	{ Opt_flock_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);
@@ -1412,6 +1426,25 @@ static int nfs_parse_mount_options(char *raw,
 			mnt->fscache_uniq = string;
 			mnt->options |= NFS_OPTION_FSCACHE;
 			break;
+		case Opt_flock:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string, nfs_flock_tokens, args);
+			kfree(string);
+			switch (token) {
+				case Opt_flock_local:
+					mnt->flags |= NFS_MOUNT_FLOCK_LOCAL;
+					break;
+				case Opt_flock_fcntl:
+					mnt->flags &= ~NFS_MOUNT_FLOCK_LOCAL;
+					break;
+				default:
+					dfprintk(MOUNT, "NFS:	invalid	"
+							"flock argument\n");
+					return 0;
+			};
+			break;
 
 		/*
 		 * Special options
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 5d59ae8..ee08dae 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -71,4 +71,6 @@ struct nfs_mount_data {
 #define NFS_MOUNT_NORESVPORT		0x40000
 #define NFS_MOUNT_LEGACY_INTERFACE	0x80000
 
+#define NFS_MOUNT_FLOCK_LOCAL	0x100000
+
 #endif

             reply	other threads:[~2010-09-06 12:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-06 12:33 Suresh Jayaraman [this message]
2010-09-07 13:40 ` [RFC][PATCH] nfs: support legacy NFS flock behavior via mount option Jeff Layton
2010-09-07 14:17 ` Trond Myklebust
2010-09-07 16:08   ` Jeff Layton
2010-09-07 17:06     ` Trond Myklebust
2010-09-07 20:13   ` Suresh Jayaraman
2010-09-07 20:49     ` Trond Myklebust
2010-09-08 14:36       ` Suresh Jayaraman
2010-09-08 16:50         ` Trond Myklebust
2010-09-07 22:23   ` Neil Brown
2010-09-07 22:42     ` Trond Myklebust
2010-09-08  0:04       ` Neil Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C84DFA7.7050507@suse.de \
    --to=sjayaraman@suse.de \
    --cc=Trond.Myklebust@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox