vfs lease api

vfs lease api

[J. Bruce Fields]

Eventually we want to be able to support NFSv4 delegations for cluster
filesystem exports.  We implement NFSv4 delegations using leases.  So to
make this work, we need leases to be passed down to the filesystem, so
that a cluster filesystem can enforce leases correctly across all nodes.

The following patches do some minor cleanup of the lease code, and then
add a setlease() file method.

For now the only implementations of setlease() we include (for NFS and
GFS2) are used only to turn off leases selectively.  To complete this
work we'll need to implement proper lease support for GFS2 or OCFS2, and
probably do some more work on lease breaking.

But we're requesting this be included in -mm now in the theory that the
lease-disabling behavior is useful now anyway, and in hopes that this
will the required further work.

I'm also keeping this in the "for-mm" branch at:

	git://linux-nfs.org/~bfields/linux.git for-mm 

(And maybe you could also remove the server-cluster-locking-api branch
from your list of git fetches, if it's still there?--I don't intend to
use it any more.)

--b.

[PATCH 1/6] locks: share more common lease code

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

Share more code between setlease (used by nfsd) and fcntl.

Also some minor cleanup.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 fs/locks.c |   30 ++++++++++--------------------
 1 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 431a8b8..6ad3c7b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1346,6 +1346,14 @@ static int __setlease(struct file *filp, long arg, struct file_lock **flp)
 	struct inode *inode = dentry->d_inode;
 	int error, rdlease_count = 0, wrlease_count = 0;
 
+	if ((current->fsuid != inode->i_uid) && !capable(CAP_LEASE))
+		return -EACCES;
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+	error = security_file_lock(filp, arg);
+	if (error)
+		return error;
+
 	time_out_leases(inode);
 
 	error = -EINVAL;
@@ -1431,18 +1439,8 @@ out:
 
 int setlease(struct file *filp, long arg, struct file_lock **lease)
 {
-	struct dentry *dentry = filp->f_path.dentry;
-	struct inode *inode = dentry->d_inode;
 	int error;
 
-	if ((current->fsuid != inode->i_uid) && !capable(CAP_LEASE))
-		return -EACCES;
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
-	error = security_file_lock(filp, arg);
-	if (error)
-		return error;
-
 	lock_kernel();
 	error = __setlease(filp, arg, lease);
 	unlock_kernel();
@@ -1469,14 +1467,6 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 	struct inode *inode = dentry->d_inode;
 	int error;
 
-	if ((current->fsuid != inode->i_uid) && !capable(CAP_LEASE))
-		return -EACCES;
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
-	error = security_file_lock(filp, arg);
-	if (error)
-		return error;
-
 	locks_init_lock(&fl);
 	error = lease_init(filp, arg, &fl);
 	if (error)
@@ -1490,9 +1480,9 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 
 	error = fasync_helper(fd, filp, 1, &flp->fl_fasync);
 	if (error < 0) {
-		/* remove lease just inserted by __setlease */
+		/* remove lease just inserted by setlease */
 		flp->fl_type = F_UNLCK | F_INPROGRESS;
-		flp->fl_break_time = jiffies- 10;
+		flp->fl_break_time = jiffies - 10;
 		time_out_leases(inode);
 		goto out_unlock;
 	}
-- 
1.5.2.58.g98ee

[PATCH 2/6] locks: provide a file lease method enabling cluster-coherent leases

[J. Bruce Fields]

From: J. Bruce Fields <bfields@fieldses.org>

Currently leases are only kept locally, so there's no way for a distributed
filesystem to enforce them against multiple clients.  We're particularly
interested in the case of nfsd exporting a cluster filesystem, in which
case nfsd needs cluster-coherent leases in order to implement delegations
correctly.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/locks.c         |    7 +++++--
 include/linux/fs.h |    1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 6ad3c7b..8fa4420 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1442,7 +1442,10 @@ int setlease(struct file *filp, long arg, struct file_lock **lease)
 	int error;
 
 	lock_kernel();
-	error = __setlease(filp, arg, lease);
+	if (filp->f_op && filp->f_op->setlease)
+		error = filp->f_op->setlease(filp, arg, lease);
+        else
+		error = __setlease(filp, arg, lease);
 	unlock_kernel();
 
 	return error;
@@ -1474,7 +1477,7 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 
 	lock_kernel();
 
-	error = __setlease(filp, arg, &flp);
+	error = setlease(filp, arg, &flp);
 	if (error || arg == F_UNLCK)
 		goto out_unlock;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6a41f4c..4fbbc7f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1113,6 +1113,7 @@ struct file_operations {
 	int (*flock) (struct file *, int, struct file_lock *);
 	ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int);
 	ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
+	int (*setlease)(struct file *, long, struct file_lock **);
 };
 
 struct inode_operations {
-- 
1.5.2.58.g98ee

[PATCH 3/6] locks: rename lease functions to reflect locks.c conventions

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

We've been using the convention that vfs_foo is the function that calls
a filesystem-specific foo method if it exists, or falls back on a
generic method if it doesn't.

So rename setlease to vfs_setlease, and __setlease to setlease.  Keep
setlease exported to allow filesystems to use the generic method in
addition to doing their own bookkeeping.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 fs/locks.c          |   14 +++++++-------
 fs/nfsd/nfs4state.c |   10 +++++-----
 include/linux/fs.h  |    1 +
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 8fa4420..c06a002 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1329,7 +1329,7 @@ int fcntl_getlease(struct file *filp)
 }
 
 /**
- *	__setlease	-	sets a lease on an open file
+ *	setlease	-	sets a lease on an open file
  *	@filp: file pointer
  *	@arg: type of lease to obtain
  *	@flp: input - file_lock to use, output - file_lock inserted
@@ -1339,7 +1339,7 @@ int fcntl_getlease(struct file *filp)
  *
  *	Called with kernel lock held.
  */
-static int __setlease(struct file *filp, long arg, struct file_lock **flp)
+int setlease(struct file *filp, long arg, struct file_lock **flp)
 {
 	struct file_lock *fl, **before, **my_before = NULL, *lease;
 	struct dentry *dentry = filp->f_path.dentry;
@@ -1428,7 +1428,7 @@ out:
 }
 
  /**
- *	setlease        -       sets a lease on an open file
+ *	vfs_setlease        -       sets a lease on an open file
  *	@filp: file pointer
  *	@arg: type of lease to obtain
  *	@lease: file_lock to use
@@ -1437,7 +1437,7 @@ out:
  *	The fl_lmops fl_break function is required by break_lease
  */
 
-int setlease(struct file *filp, long arg, struct file_lock **lease)
+int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 {
 	int error;
 
@@ -1445,13 +1445,13 @@ int setlease(struct file *filp, long arg, struct file_lock **lease)
 	if (filp->f_op && filp->f_op->setlease)
 		error = filp->f_op->setlease(filp, arg, lease);
         else
-		error = __setlease(filp, arg, lease);
+		error = setlease(filp, arg, lease);
 	unlock_kernel();
 
 	return error;
 }
 
-EXPORT_SYMBOL(setlease);
+EXPORT_SYMBOL(vfs_setlease);
 
 /**
  *	fcntl_setlease	-	sets a lease on an open file
@@ -1477,7 +1477,7 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 
 	lock_kernel();
 
-	error = setlease(filp, arg, &flp);
+	error = vfs_setlease(filp, arg, &flp);
 	if (error || arg == F_UNLCK)
 		goto out_unlock;
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3cc8ce4..ed01a77 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -251,7 +251,7 @@ nfs4_close_delegation(struct nfs4_delegation *dp)
 	/* The following nfsd_close may not actually close the file,
 	 * but we want to remove the lease in any case. */
 	if (dp->dl_flock)
-		setlease(filp, F_UNLCK, &dp->dl_flock);
+		vfs_setlease(filp, F_UNLCK, &dp->dl_flock);
 	nfsd_close(filp);
 }
 
@@ -1396,7 +1396,7 @@ void nfsd_release_deleg_cb(struct file_lock *fl)
 /*
  * Set the delegation file_lock back pointer.
  *
- * Called from __setlease() with lock_kernel() held.
+ * Called from setlease() with lock_kernel() held.
  */
 static
 void nfsd_copy_lock_deleg_cb(struct file_lock *new, struct file_lock *fl)
@@ -1410,7 +1410,7 @@ void nfsd_copy_lock_deleg_cb(struct file_lock *new, struct file_lock *fl)
 }
 
 /*
- * Called from __setlease() with lock_kernel() held
+ * Called from setlease() with lock_kernel() held
  */
 static
 int nfsd_same_client_deleg_cb(struct file_lock *onlist, struct file_lock *try)
@@ -1710,10 +1710,10 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 	fl.fl_file = stp->st_vfs_file;
 	fl.fl_pid = current->tgid;
 
-	/* setlease checks to see if delegation should be handed out.
+	/* vfs_setlease checks to see if delegation should be handed out.
 	 * the lock_manager callbacks fl_mylease and fl_change are used
 	 */
-	if ((status = setlease(stp->st_vfs_file,
+	if ((status = vfs_setlease(stp->st_vfs_file,
 		flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK, &flp))) {
 		dprintk("NFSD: setlease failed [%d], no delegation\n", status);
 		unhash_delegation(dp);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4fbbc7f..a26b52d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -866,6 +866,7 @@ extern int flock_lock_file_wait(struct file *filp, struct file_lock *fl);
 extern int __break_lease(struct inode *inode, unsigned int flags);
 extern void lease_get_mtime(struct inode *, struct timespec *time);
 extern int setlease(struct file *, long, struct file_lock **);
+extern int vfs_setlease(struct file *, long, struct file_lock **);
 extern int lease_modify(struct file_lock **, int);
 extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
 extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
-- 
1.5.2.58.g98ee

[PATCH 4/6] locks: fix locks.c lease symbol exports

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

Bring lease exports into line with conventions for posix locks:
	setlease() should be exported so filesystems can use it to implement
		their lease methods.
	vfs_setlease() need only be GPL-exported since only nfsd
		needs it.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 fs/locks.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index c06a002..ac267af 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1427,6 +1427,8 @@ out:
 	return error;
 }
 
+EXPORT_SYMBOL(setlease);
+
  /**
  *	vfs_setlease        -       sets a lease on an open file
  *	@filp: file pointer
@@ -1451,7 +1453,7 @@ int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 	return error;
 }
 
-EXPORT_SYMBOL(vfs_setlease);
+EXPORT_SYMBOL_GPL(vfs_setlease);
 
 /**
  *	fcntl_setlease	-	sets a lease on an open file
-- 
1.5.2.58.g98ee

[PATCH 5/6] gfs2: stop giving out non-cluster-coherent leases

[J. Bruce Fields]

From: Marc Eshel <eshel@almaden.ibm.com>

Since gfs2 can't prevent conflicting opens or leases on other nodes, we
probably shouldn't allow it to give out leases at all.

Put the newly defined lease operation into use in gfs2 by turning off
lease, unless we're using the "nolock' locking module (in which case all
locking is local anyway).

Signed-off-by: Marc Eshel <eshel@almaden.ibm.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Cc: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/gfs2/ops_file.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/fs/gfs2/ops_file.c b/fs/gfs2/ops_file.c
index 064df88..e34d9bd 100644
--- a/fs/gfs2/ops_file.c
+++ b/fs/gfs2/ops_file.c
@@ -489,6 +489,31 @@ static int gfs2_fsync(struct file *file, struct dentry *dentry, int datasync)
 }
 
 /**
+ * gfs2_setlease - acquire/release a file lease
+ * @file: the file pointer
+ * @arg: lease type
+ * @fl: file lock
+ *
+ * Returns: errno
+ */
+
+static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
+{
+	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
+	int ret = -EOPNOTSUPP;
+
+	if (sdp->sd_args.ar_localflocks) {
+		return setlease(file, arg, fl);
+	}
+
+	/* For now fail the delegation request. Cluster file system can not
+	   allow any node in the cluster to get a local lease until it can
+	   be managed centrally by the cluster file system.
+	*/
+	return ret;
+}
+
+/**
  * gfs2_lock - acquire/release a posix lock on a file
  * @file: the file pointer
  * @cmd: either modify or retrieve lock state, possibly wait
@@ -639,6 +664,7 @@ const struct file_operations gfs2_file_fops = {
 	.flock		= gfs2_flock,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= generic_file_splice_write,
+	.setlease	= gfs2_setlease,
 };
 
 const struct file_operations gfs2_dir_fops = {
-- 
1.5.2.58.g98ee

[PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

As Peter Staubach says elsewhere
(http://marc.info/?l=linux-kernel&m=118113649526444&w=2):

> The problem is that some file system such as NFSv2 and NFSv3 do
> not have sufficient support to be able to support leases correctly.
> In particular for these two file systems, there is no over the wire
> protocol support.
>
> Currently, these two file systems fail the fcntl(F_SETLEASE) call
> accidentally, due to a reference counting difference.  These file
> systems should fail more consciously, with a proper error to
> indicate that the call is invalid for them.

Define an nfs setlease method that just returns -EOPNOTSUPP.

If someone can demonstrate a real need, perhaps we could reenable
them in the presence of the "nolock" mount option.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Cc: Peter Staubach <staubach@redhat.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/file.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 9eb8eb4..97c1a3d 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -51,6 +51,7 @@ static int  nfs_fsync(struct file *, struct dentry *dentry, int datasync);
 static int nfs_check_flags(int flags);
 static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl);
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl);
+static int nfs_setlease(struct file *file, long arg, struct file_lock **fl);
 
 const struct file_operations nfs_file_operations = {
 	.llseek		= nfs_file_llseek,
@@ -67,6 +68,7 @@ const struct file_operations nfs_file_operations = {
 	.flock		= nfs_flock,
 	.sendfile	= nfs_file_sendfile,
 	.check_flags	= nfs_check_flags,
+	.setlease	= nfs_setlease,
 };
 
 const struct inode_operations nfs_file_inode_operations = {
@@ -555,3 +557,8 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 		return do_unlk(filp, cmd, fl);
 	return do_setlk(filp, cmd, fl);
 }
+
+static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
+{
+	return -EOPNOTSUPP;
+}
-- 
1.5.2.58.g98ee

Re: [PATCH 6/6] nfs: disable leases over NFS

[Peter Staubach]

J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
>
> As Peter Staubach says elsewhere
> (http://marc.info/?l=linux-kernel&m=118113649526444&w=2):
>
>   
>> The problem is that some file system such as NFSv2 and NFSv3 do
>> not have sufficient support to be able to support leases correctly.
>> In particular for these two file systems, there is no over the wire
>> protocol support.
>>
>> Currently, these two file systems fail the fcntl(F_SETLEASE) call
>> accidentally, due to a reference counting difference.  These file
>> systems should fail more consciously, with a proper error to
>> indicate that the call is invalid for them.
>>     
>
> Define an nfs setlease method that just returns -EOPNOTSUPP.
>
> If someone can demonstrate a real need, perhaps we could reenable
> them in the presence of the "nolock" mount option.
>
> Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
> Cc: Peter Staubach <staubach@redhat.com>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/file.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 9eb8eb4..97c1a3d 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -51,6 +51,7 @@ static int  nfs_fsync(struct file *, struct dentry *dentry, int datasync);
>  static int nfs_check_flags(int flags);
>  static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl);
>  static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl);
> +static int nfs_setlease(struct file *file, long arg, struct file_lock **fl);
>  
>  const struct file_operations nfs_file_operations = {
>  	.llseek		= nfs_file_llseek,
> @@ -67,6 +68,7 @@ const struct file_operations nfs_file_operations = {
>  	.flock		= nfs_flock,
>  	.sendfile	= nfs_file_sendfile,
>  	.check_flags	= nfs_check_flags,
> +	.setlease	= nfs_setlease,
>  };
>  
>  const struct inode_operations nfs_file_inode_operations = {
> @@ -555,3 +557,8 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
>  		return do_unlk(filp, cmd, fl);
>  	return do_setlk(filp, cmd, fl);
>  }
> +
> +static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
> +{
> +	return -EOPNOTSUPP;
> +}
>   

A couple of things --

First, there is already some support to disable leases for NFS mounted
file systems in -mm, I think.  Are you planning on removing it?

Second, it seems to me that EINVAL would be a better error to return
than EOPNOTSUPP.  This is an invalid operation to apply to this file
and might match POSIX style specs better.

    Thanx...

       ps

Re: [PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

On Fri, Jun 29, 2007 at 05:16:19PM -0400, Peter Staubach wrote:
> First, there is already some support to disable leases for NFS mounted
> file systems in -mm, I think.

Oops, sorry; my fault for not checking -mm before sending....

> Are you planning on removing it?

I'd rather do that, yes.  Any objection?

> Second, it seems to me that EINVAL would be a better error to return
> than EOPNOTSUPP.  This is an invalid operation to apply to this file
> and might match POSIX style specs better.

I'm not sure what you mean by "might match POSIX style specs better"?

>From a quick check, other reasons we'd get EINVAL in this case:

	- attempt to get a lease on something other than a regular file.
	- leases disabled with /proc/sys/fs/leases-enable

So if the application calling fcntl knows it was calling it on a regular
file, then with your proposal an EINVAL return would mean leases were
disabled for one reason or another, and it could take that as a sign to
fall back on some other behavior.  And I can't see any reason it would
need to distinguish between those two remaining cases (filesystem
doesn't support leases, or leases are disabled by the sysctl).  So,
OK, EINVAL sounds fine to me.

But I don't have a really strong opinion.  I think the suggestion of
EOPNOTSUPP was from Steven Whitehouse; Steven, do you care?

--b.

Re: [PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

On Fri, Jun 29, 2007 at 11:30:27PM +0100, Steven Whitehouse wrote:
> EINVAL is fine by me, just so long as its not EAGAIN then it gets my
> blessing :-)

OK.  I've changed the error return, in both the NFS and GFS2 cases, did
some minor cleanup and commenting while I was at it, and pushed the
results out to "for-mm":

	git://linux-nfs.org/~bfields/linux.git for-mm

--b.

diff --git a/fs/gfs2/ops_file.c b/fs/gfs2/ops_file.c
index e34d9bd..29a86fe 100644
--- a/fs/gfs2/ops_file.c
+++ b/fs/gfs2/ops_file.c
@@ -500,17 +500,15 @@ static int gfs2_fsync(struct file *file, struct dentry *dentry, int datasync)
 static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
-	int ret = -EOPNOTSUPP;
 
-	if (sdp->sd_args.ar_localflocks) {
+	/*
+	 * We don't currently have a way to enforce a lease across the whole
+	 * cluster; until we do, disable leases (by just returning -EINVAL),
+	 * unless the administrator has requested purely local locking.
+	 */
+	if (sdp->sd_args.ar_localflocks)
 		return setlease(file, arg, fl);
-	}
-
-	/* For now fail the delegation request. Cluster file system can not
-	   allow any node in the cluster to get a local lease until it can
-	   be managed centrally by the cluster file system.
-	*/
-	return ret;
+	return -EINVAL;
 }
 
 /**
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 97c1a3d..d92a383 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -560,5 +560,10 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 
 static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
 {
-	return -EOPNOTSUPP;
+	/*
+	 * There is no protocol support for leases, so we have no way
+	 * to implement them correctly in the face of opens by other
+	 * clients.
+	 */
+	return -EINVAL;
 }

Re: [PATCH 6/6] nfs: disable leases over NFS

[Steven Whitehouse]

Hi,

On Fri, 2007-06-29 at 17:39 -0400, J. Bruce Fields wrote:
> On Fri, Jun 29, 2007 at 05:16:19PM -0400, Peter Staubach wrote:
> > First, there is already some support to disable leases for NFS mounted
> > file systems in -mm, I think.
> 
> Oops, sorry; my fault for not checking -mm before sending....
> 
> > Are you planning on removing it?
> 
> I'd rather do that, yes.  Any objection?
> 
> > Second, it seems to me that EINVAL would be a better error to return
> > than EOPNOTSUPP.  This is an invalid operation to apply to this file
> > and might match POSIX style specs better.
> 
> I'm not sure what you mean by "might match POSIX style specs better"?
> 
> From a quick check, other reasons we'd get EINVAL in this case:
> 
> 	- attempt to get a lease on something other than a regular file.
> 	- leases disabled with /proc/sys/fs/leases-enable
> 
> So if the application calling fcntl knows it was calling it on a regular
> file, then with your proposal an EINVAL return would mean leases were
> disabled for one reason or another, and it could take that as a sign to
> fall back on some other behavior.  And I can't see any reason it would
> need to distinguish between those two remaining cases (filesystem
> doesn't support leases, or leases are disabled by the sysctl).  So,
> OK, EINVAL sounds fine to me.
> 
> But I don't have a really strong opinion.  I think the suggestion of
> EOPNOTSUPP was from Steven Whitehouse; Steven, do you care?
> 
> --b.

EINVAL is fine by me, just so long as its not EAGAIN then it gets my
blessing :-)

Steve.

Re: [PATCH 1/6] locks: share more common lease code

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:25PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> Share more code between setlease (used by nfsd) and fcntl.
> 
> Also some minor cleanup.

Looks good.  Fine for mainline just after 2.6.23 opens.

Re: [PATCH 3/6] locks: rename lease functions to reflect locks.c conventions

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:27PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> We've been using the convention that vfs_foo is the function that calls
> a filesystem-specific foo method if it exists, or falls back on a
> generic method if it doesn't.
> 
> So rename setlease to vfs_setlease, and __setlease to setlease.  Keep
> setlease exported to allow filesystems to use the generic method in
> addition to doing their own bookkeeping.

This should be moved before patch 2 and can probably go in once 2.6.23
opens aswell.

> @@ -1339,7 +1339,7 @@ int fcntl_getlease(struct file *filp)
>   *
>   *	Called with kernel lock held.
>   */
> -static int __setlease(struct file *filp, long arg, struct file_lock **flp)
> +int setlease(struct file *filp, long arg, struct file_lock **flp)

Except this shouldn't be made non-static in this patch.

>  extern int setlease(struct file *, long, struct file_lock **);

And this prototype shouldn't be kept.  If you want to use it somewhere
later in the patch series make it non-static and export it in a separate
patch.  If it's ever intended to be export it should probably get a
better name aswell, e.g. generic_setlease.

Re: [PATCH 4/6] locks: fix locks.c lease symbol exports

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:28PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> Bring lease exports into line with conventions for posix locks:
> 	setlease() should be exported so filesystems can use it to implement
> 		their lease methods.
> 	vfs_setlease() need only be GPL-exported since only nfsd
> 		needs it.

The latter should be merged in the previous patch, the first should
be a patch with making it non-static and exporting it.  (after the
rename of it is merged into the patch that introduces the vfs_setlease
name)

Re: [PATCH 6/6] nfs: disable leases over NFS

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:30PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> As Peter Staubach says elsewhere
> (http://marc.info/?l=linux-kernel&m=118113649526444&w=2):
> 
> > The problem is that some file system such as NFSv2 and NFSv3 do
> > not have sufficient support to be able to support leases correctly.
> > In particular for these two file systems, there is no over the wire
> > protocol support.
> >
> > Currently, these two file systems fail the fcntl(F_SETLEASE) call
> > accidentally, due to a reference counting difference.  These file
> > systems should fail more consciously, with a proper error to
> > indicate that the call is invalid for them.
> 
> Define an nfs setlease method that just returns -EOPNOTSUPP.
> 
> If someone can demonstrate a real need, perhaps we could reenable
> them in the presence of the "nolock" mount option.

I'm not a big fan of default methods that do the wrong thing instead
of just missing functionality.  Would you mind just returning
-EOPNOTSUPP if ->setlease is not implemented and add it to all
the local filesystems while all the network/distributed filesystems
should not have it, not just nfs.

Re: [PATCH 2/6] locks: provide a file lease method enabling cluster-coherent leases

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:26PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@fieldses.org>
> 
> Currently leases are only kept locally, so there's no way for a distributed
> filesystem to enforce them against multiple clients.  We're particularly
> interested in the case of nfsd exporting a cluster filesystem, in which
> case nfsd needs cluster-coherent leases in order to implement delegations
> correctly.

Looks good but should be moved second to last in the patch series, with
the last one beeing implementing setleast for gfs.  Also see my comment
to the nfs patch about the default semantics for not implementing the
method.

Re: [PATCH 5/6] gfs2: stop giving out non-cluster-coherent leases

[Christoph Hellwig]

On Fri, Jun 29, 2007 at 03:21:29PM -0400, J. Bruce Fields wrote:
> +static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
> +{
> +	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
> +	int ret = -EOPNOTSUPP;
> +
> +	if (sdp->sd_args.ar_localflocks) {
> +		return setlease(file, arg, fl);
> +	}
> +
> +	/* For now fail the delegation request. Cluster file system can not
> +	   allow any node in the cluster to get a local lease until it can
> +	   be managed centrally by the cluster file system.
> +	*/
> +	return ret;
> +}

Very odd way to write this function.  It should look more like:

static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
{
	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
	int ret = -EOPNOTSUPP;

	/*
	 * For now fail the delegation request. Cluster file system can not
	 * allow any node in the cluster to get a local lease until it can
	 * be managed centrally by the cluster file system.
	 */
	if (!sdp->sd_args.ar_localflocks)
		return -EOPNOTSUPP;

	return setlease(file, arg, fl);
}

Re: [PATCH 5/6] gfs2: stop giving out non-cluster-coherent leases

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:27:42AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:29PM -0400, J. Bruce Fields wrote:
> > +static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
> > +{
> > +	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
> > +	int ret = -EOPNOTSUPP;
> > +
> > +	if (sdp->sd_args.ar_localflocks) {
> > +		return setlease(file, arg, fl);
> > +	}
> > +
> > +	/* For now fail the delegation request. Cluster file system can not
> > +	   allow any node in the cluster to get a local lease until it can
> > +	   be managed centrally by the cluster file system.
> > +	*/
> > +	return ret;
> > +}
> 
> Very odd way to write this function.  It should look more like:

Thanks for reading these through!  I'm travelling tomorrow, so I'll work
through your comments Monday afternoon or Tuesday....

--b.

Re: [PATCH 1/6] locks: share more common lease code

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:20:13AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:25PM -0400, J. Bruce Fields wrote:
> > From: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > Share more code between setlease (used by nfsd) and fcntl.
> > 
> > Also some minor cleanup.
> 
> Looks good.  Fine for mainline just after 2.6.23 opens.

Thanks.  (And, by the way, would it be helpful for me to translate this
kind of statement into an "acked-by: Christoph..." on the eventual
patch?)

--b.

Re: [PATCH 1/6] locks: share more common lease code

[Christoph Hellwig]

On Tue, Jul 03, 2007 at 06:17:35PM -0400, J. Bruce Fields wrote:
> On Sat, Jun 30, 2007 at 10:20:13AM +0100, Christoph Hellwig wrote:
> > On Fri, Jun 29, 2007 at 03:21:25PM -0400, J. Bruce Fields wrote:
> > > From: J. Bruce Fields <bfields@citi.umich.edu>
> > > 
> > > Share more code between setlease (used by nfsd) and fcntl.
> > > 
> > > Also some minor cleanup.
> > 
> > Looks good.  Fine for mainline just after 2.6.23 opens.
> 
> Thanks.  (And, by the way, would it be helpful for me to translate this
> kind of statement into an "acked-by: Christoph..." on the eventual
> patch?)

Fine with me.

Re: [PATCH 3/6] locks: rename lease functions to reflect locks.c conventions

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:22:43AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:27PM -0400, J. Bruce Fields wrote:
> > From: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > We've been using the convention that vfs_foo is the function that calls
> > a filesystem-specific foo method if it exists, or falls back on a
> > generic method if it doesn't.
> > 
> > So rename setlease to vfs_setlease, and __setlease to setlease.  Keep
> > setlease exported to allow filesystems to use the generic method in
> > addition to doing their own bookkeeping.
> 
> This should be moved before patch 2 and can probably go in once 2.6.23
> opens aswell.
> 
> > @@ -1339,7 +1339,7 @@ int fcntl_getlease(struct file *filp)
> >   *
> >   *	Called with kernel lock held.
> >   */
> > -static int __setlease(struct file *filp, long arg, struct file_lock **flp)
> > +int setlease(struct file *filp, long arg, struct file_lock **flp)
> 
> Except this shouldn't be made non-static in this patch.
> 
> >  extern int setlease(struct file *, long, struct file_lock **);
> 
> And this prototype shouldn't be kept.  If you want to use it somewhere
> later in the patch series make it non-static and export it in a separate
> patch.  If it's ever intended to be export it should probably get a
> better name aswell, e.g. generic_setlease.

OK, done.--b.

Re: [PATCH 4/6] locks: fix locks.c lease symbol exports

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:23:45AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:28PM -0400, J. Bruce Fields wrote:
> > From: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > Bring lease exports into line with conventions for posix locks:
> > 	setlease() should be exported so filesystems can use it to implement
> > 		their lease methods.
> > 	vfs_setlease() need only be GPL-exported since only nfsd
> > 		needs it.
> 
> The latter should be merged in the previous patch, the first should
> be a patch with making it non-static and exporting it.  (after the
> rename of it is merged into the patch that introduces the vfs_setlease
> name)

Done.--b.

Re: [PATCH 5/6] gfs2: stop giving out non-cluster-coherent leases

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:27:42AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:29PM -0400, J. Bruce Fields wrote:
> > +static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
> > +{
> > +	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
> > +	int ret = -EOPNOTSUPP;
> > +
> > +	if (sdp->sd_args.ar_localflocks) {
> > +		return setlease(file, arg, fl);
> > +	}
> > +
> > +	/* For now fail the delegation request. Cluster file system can not
> > +	   allow any node in the cluster to get a local lease until it can
> > +	   be managed centrally by the cluster file system.
> > +	*/
> > +	return ret;
> > +}
> 
> Very odd way to write this function.  It should look more like:

Agreed; I've reversed the sense of the test, as suggested, and got rid
of the superfluous local "ret".

--b.

> static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
> {
> 	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
> 	int ret = -EOPNOTSUPP;
> 
> 	/*
> 	 * For now fail the delegation request. Cluster file system can not
> 	 * allow any node in the cluster to get a local lease until it can
> 	 * be managed centrally by the cluster file system.
> 	 */
> 	if (!sdp->sd_args.ar_localflocks)
> 		return -EOPNOTSUPP;
> 
> 	return setlease(file, arg, fl);
> }
> 

Re: [PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:25:16AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:30PM -0400, J. Bruce Fields wrote:
> > From: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > As Peter Staubach says elsewhere
> > (http://marc.info/?l=linux-kernel&m=118113649526444&w=2):
> > 
> > > The problem is that some file system such as NFSv2 and NFSv3 do
> > > not have sufficient support to be able to support leases correctly.
> > > In particular for these two file systems, there is no over the wire
> > > protocol support.
> > >
> > > Currently, these two file systems fail the fcntl(F_SETLEASE) call
> > > accidentally, due to a reference counting difference.  These file
> > > systems should fail more consciously, with a proper error to
> > > indicate that the call is invalid for them.
> > 
> > Define an nfs setlease method that just returns -EOPNOTSUPP.
> > 
> > If someone can demonstrate a real need, perhaps we could reenable
> > them in the presence of the "nolock" mount option.
> 
> I'm not a big fan of default methods that do the wrong thing instead
> of just missing functionality.  Would you mind just returning
> -EOPNOTSUPP if ->setlease is not implemented and add it to all
> the local filesystems while all the network/distributed filesystems
> should not have it, not just nfs.

OK, I think that may make sense, but... wow does linux ever have a lot
of obscure filesystems.  This might take me a little longer.

--b.

Re: [PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

On Sat, Jun 30, 2007 at 10:25:16AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 29, 2007 at 03:21:30PM -0400, J. Bruce Fields wrote:
> > Define an nfs setlease method that just returns -EOPNOTSUPP.
> > 
> > If someone can demonstrate a real need, perhaps we could reenable
> > them in the presence of the "nolock" mount option.
> 
> I'm not a big fan of default methods that do the wrong thing instead
> of just missing functionality.  Would you mind just returning
> -EOPNOTSUPP if ->setlease is not implemented and add it to all
> the local filesystems while all the network/distributed filesystems
> should not have it, not just nfs.

OK, after looking at this a little more, I'm less happy about the idea
of erroring out by default:

	- There are a ton of filesystems that probably should allow
	  leases, and only a few (network filesystems) that shouldn't,
	  so leaving leases on by default seems simpler.
	- We already fall back on the local method by default in the case
	  of locks, and I don't see a reason to treat leases differently.
	- The patch to add
		.setlease = setlease,
	  to all the file_operations is going to be a big patch that
	  changes behavior in a way that might be easy to miss (because
	  it changes behavior exactly on those filesystems it *doesn't*
	  touch.)  I think it'll be easier to get better review on the
	  patch that adds a method just to those filesystems that we're
	  disabling leases for.

I agree about dealing with the other network filesystems, though, and
not just NFS and GFS2; I'll look into that.

--b.

Re: [PATCH 6/6] nfs: disable leases over NFS

[Christoph Hellwig]

On Thu, Jul 05, 2007 at 11:41:00AM -0400, J. Bruce Fields wrote:
> OK, after looking at this a little more, I'm less happy about the idea
> of erroring out by default:
> 
> 	- There are a ton of filesystems that probably should allow
> 	  leases, and only a few (network filesystems) that shouldn't,
> 	  so leaving leases on by default seems simpler.

But it gets you possible wrong behaviour by default.  I'm not a big
fan of non-trivial default methods as you see :)

> 	- We already fall back on the local method by default in the case
> 	  of locks, and I don't see a reason to treat leases differently.
> 	- The patch to add
> 		.setlease = setlease,
> 	  to all the file_operations is going to be a big patch that
> 	  changes behavior in a way that might be easy to miss (because
> 	  it changes behavior exactly on those filesystems it *doesn't*
> 	  touch.)  I think it'll be easier to get better review on the
> 	  patch that adds a method just to those filesystems that we're
> 	  disabling leases for.

Anyway, feel free to go ahead with the simpler version for now, I'll do
the switchover for locks and leases when I get some time.

Re: [PATCH 6/6] nfs: disable leases over NFS

[J. Bruce Fields]

On Wed, Jul 11, 2007 at 11:20:18AM +0100, Christoph Hellwig wrote:
> On Thu, Jul 05, 2007 at 11:41:00AM -0400, J. Bruce Fields wrote:
> > OK, after looking at this a little more, I'm less happy about the idea
> > of erroring out by default:
> > 
> > 	- There are a ton of filesystems that probably should allow
> > 	  leases, and only a few (network filesystems) that shouldn't,
> > 	  so leaving leases on by default seems simpler.
> 
> But it gets you possible wrong behaviour by default.  I'm not a big
> fan of non-trivial default methods as you see :)

Yeah, I do understand the attraction of doing it your way.  With some
quick grepping, what I found was:

	- about 28 on-disk filesystems, all of which I assume should
	  support leases.
	- about 12 filesystems that either are network filesystems (9p,
	  afs, cifs, coda, ncpfs, nfs, nfs4, ocfs2, smbfs) or that don't
	  have control over all opens/data modifications for whatever
	  reason (ecryptfs, fuse, hostfs), so shouldn't be giving out
	  leases by default.
	- A bunch of synthetic filesystems (proc, sysfs...).

The latter category being the strongest argument for your approach,
since it's sort of ludicrous to allow leases on those filesystems, but
currently we do just out of laziness.  (Or, in any case, I don't see any
reason the current code wouldn't allow them; I haven't actually tested).

> > 	- We already fall back on the local method by default in the case
> > 	  of locks, and I don't see a reason to treat leases differently.
> > 	- The patch to add
> > 		.setlease = setlease,
> > 	  to all the file_operations is going to be a big patch that
> > 	  changes behavior in a way that might be easy to miss (because
> > 	  it changes behavior exactly on those filesystems it *doesn't*
> > 	  touch.)  I think it'll be easier to get better review on the
> > 	  patch that adds a method just to those filesystems that we're
> > 	  disabling leases for.
> 
> Anyway, feel free to go ahead with the simpler version for now, I'll do
> the switchover for locks and leases when I get some time.

That would be great.  For now I think I'll also add another simple
EINVAL-returning setlease() at least to cifs (partly just as an attempt
to goad Steve French into following up on a promise at OLS to look into
proper lease support for cifs....)

But I've appended my first attempt at your suggestion for leases, in
case it's of use.  (Untested.)

--b.

From: J. Bruce Fields <bfields@citi.umich.edu>
Subject: [PATCH] Turn off support for fcntl leases by default

A lease enforces mutual exclusion with conflicting opens.  On
filesystems such as network filesystems where not all opens happen under
the control of this kernel, the default setlease() operation, which can
only check for local conflicts, is incorrect.  So in most cases the
correct thing is probably to disable leases for those filesystems until
they can implement something more sophisticated.

The only users of leases that I'm aware of (samba and nfsd) are actually
using them primarly to get synchronous notification of changes to files
so that they can update their caches.  So, more generally, we should
disable leases for any filesystem which might allow file contents to
change without a local open for write occuring.  That includes most of
the synthetic filesystems (like proc), which the file servers probably
don't want to export anyway.

Previously we explicitly disabled leases for some network filesystems,
but with this patch we disable by default and add an explicit setlease
method for those filesystems on which leases will be allowed.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 fs/adfs/file.c              |    1 +
 fs/affs/file.c              |    1 +
 fs/bfs/file.c               |    1 +
 fs/ext2/file.c              |    2 ++
 fs/ext3/file.c              |    1 +
 fs/ext4/file.c              |    1 +
 fs/fat/file.c               |    1 +
 fs/hfs/inode.c              |    1 +
 fs/hfsplus/inode.c          |    1 +
 fs/hppfs/hppfs_kern.c       |    1 +
 fs/jffs2/file.c             |    1 +
 fs/jfs/file.c               |    1 +
 fs/locks.c                  |    8 ++++----
 fs/minix/file.c             |    1 +
 fs/nfs/file.c               |   12 ------------
 fs/ntfs/file.c              |    1 +
 fs/qnx4/file.c              |    1 +
 fs/ramfs/file-mmu.c         |    1 +
 fs/ramfs/file-nommu.c       |    1 +
 fs/read_write.c             |    1 +
 fs/reiserfs/file.c          |    1 +
 fs/sysv/file.c              |    1 +
 fs/udf/file.c               |    1 +
 fs/ufs/file.c               |    1 +
 fs/xfs/linux-2.6/xfs_file.c |    2 ++
 25 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/fs/adfs/file.c b/fs/adfs/file.c
index f544a28..4e99861 100644
--- a/fs/adfs/file.c
+++ b/fs/adfs/file.c
@@ -34,6 +34,7 @@ const struct file_operations adfs_file_operations = {
 	.write		= do_sync_write,
 	.aio_write	= generic_file_aio_write,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 const struct inode_operations adfs_file_inode_operations = {
diff --git a/fs/affs/file.c b/fs/affs/file.c
index c879690..5beb510 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -36,6 +36,7 @@ const struct file_operations affs_file_operations = {
 	.release	= affs_file_release,
 	.fsync		= file_fsync,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 const struct inode_operations affs_file_inode_operations = {
diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index ef4d1fa..055e3c2 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -25,6 +25,7 @@ const struct file_operations bfs_file_operations = {
 	.aio_write	= generic_file_aio_write,
 	.mmap		= generic_file_mmap,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 static int bfs_move_block(unsigned long from, unsigned long to, struct super_block *sb)
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index 566d4e2..d0d9745 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -56,6 +56,7 @@ const struct file_operations ext2_file_operations = {
 	.sendfile	= generic_file_sendfile,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= generic_file_splice_write,
+	.setlease	= setlease,
 };
 
 #ifdef CONFIG_EXT2_FS_XIP
@@ -72,6 +73,7 @@ const struct file_operations ext2_xip_file_operations = {
 	.release	= ext2_release_file,
 	.fsync		= ext2_sync_file,
 	.sendfile	= xip_file_sendfile,
+	.setlease	= setlease,
 };
 #endif
 
diff --git a/fs/ext3/file.c b/fs/ext3/file.c
index 1e6f138..528e720 100644
--- a/fs/ext3/file.c
+++ b/fs/ext3/file.c
@@ -123,6 +123,7 @@ const struct file_operations ext3_file_operations = {
 	.sendfile	= generic_file_sendfile,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= generic_file_splice_write,
+	.setlease	= setlease,
 };
 
 const struct inode_operations ext3_file_inode_operations = {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 3c6c1fd..c4ceb21 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -123,6 +123,7 @@ const struct file_operations ext4_file_operations = {
 	.sendfile	= generic_file_sendfile,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= generic_file_splice_write,
+	.setlease	= setlease,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 55d3c74..9707be9 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -135,6 +135,7 @@ const struct file_operations fat_file_operations = {
 	.ioctl		= fat_generic_ioctl,
 	.fsync		= file_fsync,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 static int fat_cont_expand(struct inode *inode, loff_t size)
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 9a934db..6aaa02d 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -611,6 +611,7 @@ static const struct file_operations hfs_file_operations = {
 	.fsync		= file_fsync,
 	.open		= hfs_file_open,
 	.release	= hfs_file_release,
+	.setlease	= setlease,
 };
 
 static const struct inode_operations hfs_file_inode_operations = {
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 45dab5d..088e7c7 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -293,6 +293,7 @@ static const struct file_operations hfsplus_file_operations = {
 	.open		= hfsplus_file_open,
 	.release	= hfsplus_file_release,
 	.ioctl          = hfsplus_ioctl,
+	.setlease	= setlease,
 };
 
 struct inode *hfsplus_new_inode(struct super_block *sb, int mode)
diff --git a/fs/hppfs/hppfs_kern.c b/fs/hppfs/hppfs_kern.c
index affb741..edf7081 100644
--- a/fs/hppfs/hppfs_kern.c
+++ b/fs/hppfs/hppfs_kern.c
@@ -563,6 +563,7 @@ static const struct file_operations hppfs_file_fops = {
 	.read		= hppfs_read,
 	.write		= hppfs_write,
 	.open		= hppfs_open,
+	.setlease	= setlease,
 };
 
 struct hppfs_dirent {
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 9987127..6ccc03e 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -48,6 +48,7 @@ const struct file_operations jffs2_file_operations =
 	.mmap =		generic_file_readonly_mmap,
 	.fsync =	jffs2_fsync,
 	.sendfile =	generic_file_sendfile
+	.setlease = 	setlease,
 };
 
 /* jffs2_file_inode_operations */
diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index f7f8eff..3ef824b 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -114,4 +114,5 @@ const struct file_operations jfs_file_operations = {
 	.fsync		= jfs_fsync,
 	.release	= jfs_release,
 	.ioctl		= jfs_ioctl,
+	.setlease	= setlease,
 };
diff --git a/fs/locks.c b/fs/locks.c
index 428fb0a..2ef381e 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1455,11 +1455,11 @@ int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 {
 	int error;
 
+	if (!filp->f_op || !filp->f_op->setlease)
+		return -EINVAL;
+
 	lock_kernel();
-	if (filp->f_op && filp->f_op->setlease)
-		error = filp->f_op->setlease(filp, arg, lease);
-        else
-		error = setlease(filp, arg, lease);
+	error = filp->f_op->setlease(filp, arg, lease);
 	unlock_kernel();
 
 	return error;
diff --git a/fs/minix/file.c b/fs/minix/file.c
index f92baa1..352352b 100644
--- a/fs/minix/file.c
+++ b/fs/minix/file.c
@@ -24,6 +24,7 @@ const struct file_operations minix_file_operations = {
 	.mmap		= generic_file_mmap,
 	.fsync		= minix_sync_file,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 const struct inode_operations minix_file_inode_operations = {
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index d92a383..9eb8eb4 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -51,7 +51,6 @@ static int  nfs_fsync(struct file *, struct dentry *dentry, int datasync);
 static int nfs_check_flags(int flags);
 static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl);
 static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl);
-static int nfs_setlease(struct file *file, long arg, struct file_lock **fl);
 
 const struct file_operations nfs_file_operations = {
 	.llseek		= nfs_file_llseek,
@@ -68,7 +67,6 @@ const struct file_operations nfs_file_operations = {
 	.flock		= nfs_flock,
 	.sendfile	= nfs_file_sendfile,
 	.check_flags	= nfs_check_flags,
-	.setlease	= nfs_setlease,
 };
 
 const struct inode_operations nfs_file_inode_operations = {
@@ -557,13 +555,3 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 		return do_unlk(filp, cmd, fl);
 	return do_setlk(filp, cmd, fl);
 }
-
-static int nfs_setlease(struct file *file, long arg, struct file_lock **fl)
-{
-	/*
-	 * There is no protocol support for leases, so we have no way
-	 * to implement them correctly in the face of opens by other
-	 * clients.
-	 */
-	return -EINVAL;
-}
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 7ed5639..569b349 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2286,6 +2286,7 @@ const struct file_operations ntfs_file_ops = {
 						    on the ntfs partition.  We
 						    do not need to care about
 						    the data source. */
+	.setlease	= setlease,
 };
 
 const struct inode_operations ntfs_file_inode_ops = {
diff --git a/fs/qnx4/file.c b/fs/qnx4/file.c
index 4464998..225a45a 100644
--- a/fs/qnx4/file.c
+++ b/fs/qnx4/file.c
@@ -31,6 +31,7 @@ const struct file_operations qnx4_file_operations =
 	.aio_write	= generic_file_aio_write,
 	.fsync		= qnx4_sync_file,
 #endif
+	.setlease	= setlease,
 };
 
 const struct inode_operations qnx4_file_inode_operations =
diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
index 2f14774..cd1e5b4 100644
--- a/fs/ramfs/file-mmu.c
+++ b/fs/ramfs/file-mmu.c
@@ -43,6 +43,7 @@ const struct file_operations ramfs_file_operations = {
 	.fsync		= simple_sync_file,
 	.sendfile	= generic_file_sendfile,
 	.llseek		= generic_file_llseek,
+	.setlease	= setlease,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {
diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 5d258c4..faeb31f 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -44,6 +44,7 @@ const struct file_operations ramfs_file_operations = {
 	.fsync			= simple_sync_file,
 	.sendfile		= generic_file_sendfile,
 	.llseek			= generic_file_llseek,
+	.setlease		= setlease,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {
diff --git a/fs/read_write.c b/fs/read_write.c
index 4d03008..1e8ea4c 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -26,6 +26,7 @@ const struct file_operations generic_ro_fops = {
 	.aio_read	= generic_file_aio_read,
 	.mmap		= generic_file_readonly_mmap,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 EXPORT_SYMBOL(generic_ro_fops);
diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c
index 9e451a6..a95c43c 100644
--- a/fs/reiserfs/file.c
+++ b/fs/reiserfs/file.c
@@ -1536,6 +1536,7 @@ const struct file_operations reiserfs_file_operations = {
 	.aio_write = generic_file_aio_write,
 	.splice_read = generic_file_splice_read,
 	.splice_write = generic_file_splice_write,
+	.setlease = setlease,
 };
 
 const struct inode_operations reiserfs_file_inode_operations = {
diff --git a/fs/sysv/file.c b/fs/sysv/file.c
index 0732ddb..393487d 100644
--- a/fs/sysv/file.c
+++ b/fs/sysv/file.c
@@ -28,6 +28,7 @@ const struct file_operations sysv_file_operations = {
 	.mmap		= generic_file_mmap,
 	.fsync		= sysv_sync_file,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
 
 const struct inode_operations sysv_file_inode_operations = {
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 51b5764..1941fb9 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -262,6 +262,7 @@ const struct file_operations udf_file_operations = {
 	.release		= udf_release_file,
 	.fsync			= udf_fsync_file,
 	.sendfile		= generic_file_sendfile,
+	.setlease		= setlease,
 };
 
 const struct inode_operations udf_file_inode_operations = {
diff --git a/fs/ufs/file.c b/fs/ufs/file.c
index 1e09632..c9df8a5 100644
--- a/fs/ufs/file.c
+++ b/fs/ufs/file.c
@@ -61,4 +61,5 @@ const struct file_operations ufs_file_operations = {
 	.open           = generic_file_open,
 	.fsync		= ufs_sync_file,
 	.sendfile	= generic_file_sendfile,
+	.setlease	= setlease,
 };
diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c
index cb51dc9..f3a9409 100644
--- a/fs/xfs/linux-2.6/xfs_file.c
+++ b/fs/xfs/linux-2.6/xfs_file.c
@@ -467,6 +467,7 @@ const struct file_operations xfs_file_operations = {
 #ifdef HAVE_FOP_OPEN_EXEC
 	.open_exec	= xfs_file_open_exec,
 #endif
+	.setlease	= setlease,
 };
 
 const struct file_operations xfs_invis_file_operations = {
@@ -487,6 +488,7 @@ const struct file_operations xfs_invis_file_operations = {
 	.flush		= xfs_file_close,
 	.release	= xfs_file_release,
 	.fsync		= xfs_file_fsync,
+	.setlease	= setlease,
 };
 
 
-- 
1.5.3.rc0.63.gc956