[PATCH] vfs: take i_mutex on renamed file

[PATCH] vfs: take i_mutex on renamed file

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

A read delegation is used by NFSv4 as a guarantee that a client can
perform local read opens without informing the server.

The open operation takes the last component of the pathname as an
argument, thus is also a lookup operation, and giving the client the
above guarantee means informing the client before we allow anything that
would change the set of names pointing to the inode.

Therefore, we need to break delegations on rename, link, and unlink.

We also need to prevent new delegations from being acquired while one of
these operations is in progress.

We could add some completely new locking for that purpose, but it's
simpler to use the i_mutex, since that's already taken by all the
operations we care about.

The single exception is rename.  So, modify rename to take the i_mutex
on the file that is being renamed.

Also fix up lockdep and Documentation/filesystems/directory-locking to
reflect the change.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 Documentation/filesystems/directory-locking |   11 ++++++-----
 fs/namei.c                                  |    3 +++
 include/linux/fs.h                          |    9 ++++++---
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/directory-locking b/Documentation/filesystems/directory-locking
index ff7b611..9edbcd2 100644
--- a/Documentation/filesystems/directory-locking
+++ b/Documentation/filesystems/directory-locking
@@ -12,8 +12,8 @@ kinds of locks - per-inode (->i_mutex) and per-filesystem
 locks victim and calls the method.
 
 4) rename() that is _not_ cross-directory.  Locking rules: caller locks
-the parent, finds source and target, if target already exists - locks it
-and then calls the method.
+the parent, finds source and target, locks source, also locks target if
+it already exists, and then calls the method.
 
 5) link creation.  Locking rules:
 	* lock parent
@@ -30,6 +30,7 @@ rules:
 		fail with -ENOTEMPTY
 	* if new parent is equal to or is a descendent of source
 		fail with -ELOOP
+	* lock source if it is not a directory.
 	* if target exists - lock it.
 	* call the method.
 
@@ -56,9 +57,9 @@ objects - A < B iff A is an ancestor of B.
     renames will be blocked on filesystem lock and we don't start changing
     the order until we had acquired all locks).
 
-(3) any operation holds at most one lock on non-directory object and
-    that lock is acquired after all other locks.  (Proof: see descriptions
-    of operations).
+(3) locks on non-directory objects are acquired only after taking locks
+    on their parents (which remain their parents until all locks are
+    acquired, by (1) and (2)).  (Proof: see descriptions of operations).
 
 	Now consider the minimal deadlock.  Each process is blocked on
 attempt to acquire some lock and already holds at least one lock.  Let's
diff --git a/fs/namei.c b/fs/namei.c
index 208c6aa..d29b3c4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3073,6 +3073,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 			    struct inode *new_dir, struct dentry *new_dentry)
 {
 	struct inode *target = new_dentry->d_inode;
+	struct inode *source = old_dentry->d_inode;
 	int error;
 
 	error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry);
@@ -3080,6 +3081,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 		return error;
 
 	dget(new_dentry);
+	mutex_lock_nested(&source->i_mutex, I_MUTEX_RENAME_SOURCE);
 	if (target)
 		mutex_lock(&target->i_mutex);
 
@@ -3098,6 +3100,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 out:
 	if (target)
 		mutex_unlock(&target->i_mutex);
+	mutex_unlock(&source->i_mutex);
 	dput(new_dentry);
 	return error;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 386da09..e537b67 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -855,10 +855,12 @@ static inline int inode_unhashed(struct inode *inode)
  * 0: the object of the current VFS operation
  * 1: parent
  * 2: child/target
- * 3: quota file
+ * 3: xattr
+ * 4: quota file
+ * 5: the file being renamed (used only in rename of a non-directory)
  *
  * The locking order between these classes is
- * parent -> child -> normal -> xattr -> quota
+ * parent -> child -> rename_source -> normal -> xattr -> quota
  */
 enum inode_i_mutex_lock_class
 {
@@ -866,7 +868,8 @@ enum inode_i_mutex_lock_class
 	I_MUTEX_PARENT,
 	I_MUTEX_CHILD,
 	I_MUTEX_XATTR,
-	I_MUTEX_QUOTA
+	I_MUTEX_QUOTA,
+	I_MUTEX_RENAME_SOURCE
 };
 
 /*
-- 
1.7.5.4

Re: [PATCH] vfs: take i_mutex on renamed file

[J. Bruce Fields]

Al, do you see any reason this won't work?

After this, all I need to make nfsd happy is a patch to locks.c to
implement delegations, and then a few one-liners to add break_deleg
calls in the right operations.  So this is the one non-trivial change I
have to core vfs code.

All of those patches are also ready, and I'd like to merge them for 3.4.

--b.

On Mon, Mar 05, 2012 at 05:38:47PM -0500, bfields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> A read delegation is used by NFSv4 as a guarantee that a client can
> perform local read opens without informing the server.
> 
> The open operation takes the last component of the pathname as an
> argument, thus is also a lookup operation, and giving the client the
> above guarantee means informing the client before we allow anything that
> would change the set of names pointing to the inode.
> 
> Therefore, we need to break delegations on rename, link, and unlink.
> 
> We also need to prevent new delegations from being acquired while one of
> these operations is in progress.
> 
> We could add some completely new locking for that purpose, but it's
> simpler to use the i_mutex, since that's already taken by all the
> operations we care about.
> 
> The single exception is rename.  So, modify rename to take the i_mutex
> on the file that is being renamed.
> 
> Also fix up lockdep and Documentation/filesystems/directory-locking to
> reflect the change.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  Documentation/filesystems/directory-locking |   11 ++++++-----
>  fs/namei.c                                  |    3 +++
>  include/linux/fs.h                          |    9 ++++++---
>  3 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/filesystems/directory-locking b/Documentation/filesystems/directory-locking
> index ff7b611..9edbcd2 100644
> --- a/Documentation/filesystems/directory-locking
> +++ b/Documentation/filesystems/directory-locking
> @@ -12,8 +12,8 @@ kinds of locks - per-inode (->i_mutex) and per-filesystem
>  locks victim and calls the method.
>  
>  4) rename() that is _not_ cross-directory.  Locking rules: caller locks
> -the parent, finds source and target, if target already exists - locks it
> -and then calls the method.
> +the parent, finds source and target, locks source, also locks target if
> +it already exists, and then calls the method.
>  
>  5) link creation.  Locking rules:
>  	* lock parent
> @@ -30,6 +30,7 @@ rules:
>  		fail with -ENOTEMPTY
>  	* if new parent is equal to or is a descendent of source
>  		fail with -ELOOP
> +	* lock source if it is not a directory.
>  	* if target exists - lock it.
>  	* call the method.
>  
> @@ -56,9 +57,9 @@ objects - A < B iff A is an ancestor of B.
>      renames will be blocked on filesystem lock and we don't start changing
>      the order until we had acquired all locks).
>  
> -(3) any operation holds at most one lock on non-directory object and
> -    that lock is acquired after all other locks.  (Proof: see descriptions
> -    of operations).
> +(3) locks on non-directory objects are acquired only after taking locks
> +    on their parents (which remain their parents until all locks are
> +    acquired, by (1) and (2)).  (Proof: see descriptions of operations).
>  
>  	Now consider the minimal deadlock.  Each process is blocked on
>  attempt to acquire some lock and already holds at least one lock.  Let's
> diff --git a/fs/namei.c b/fs/namei.c
> index 208c6aa..d29b3c4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3073,6 +3073,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
>  			    struct inode *new_dir, struct dentry *new_dentry)
>  {
>  	struct inode *target = new_dentry->d_inode;
> +	struct inode *source = old_dentry->d_inode;
>  	int error;
>  
>  	error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry);
> @@ -3080,6 +3081,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
>  		return error;
>  
>  	dget(new_dentry);
> +	mutex_lock_nested(&source->i_mutex, I_MUTEX_RENAME_SOURCE);
>  	if (target)
>  		mutex_lock(&target->i_mutex);
>  
> @@ -3098,6 +3100,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
>  out:
>  	if (target)
>  		mutex_unlock(&target->i_mutex);
> +	mutex_unlock(&source->i_mutex);
>  	dput(new_dentry);
>  	return error;
>  }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 386da09..e537b67 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -855,10 +855,12 @@ static inline int inode_unhashed(struct inode *inode)
>   * 0: the object of the current VFS operation
>   * 1: parent
>   * 2: child/target
> - * 3: quota file
> + * 3: xattr
> + * 4: quota file
> + * 5: the file being renamed (used only in rename of a non-directory)
>   *
>   * The locking order between these classes is
> - * parent -> child -> normal -> xattr -> quota
> + * parent -> child -> rename_source -> normal -> xattr -> quota
>   */
>  enum inode_i_mutex_lock_class
>  {
> @@ -866,7 +868,8 @@ enum inode_i_mutex_lock_class
>  	I_MUTEX_PARENT,
>  	I_MUTEX_CHILD,
>  	I_MUTEX_XATTR,
> -	I_MUTEX_QUOTA
> +	I_MUTEX_QUOTA,
> +	I_MUTEX_RENAME_SOURCE
>  };
>  
>  /*
> -- 
> 1.7.5.4
> 

Re: [PATCH] vfs: take i_mutex on renamed file

[J. Bruce Fields]

On Mon, Mar 05, 2012 at 05:43:34PM -0500, J. Bruce Fields wrote:
> Al, do you see any reason this won't work?
> 
> After this, all I need to make nfsd happy is a patch to locks.c to
> implement delegations, and then a few one-liners to add break_deleg
> calls in the right operations.  So this is the one non-trivial change I
> have to core vfs code.
> 
> All of those patches are also ready, and I'd like to merge them for 3.4.

By the way, the following is the rest of the series.

(Compared to the last posting the deleg-breaking interface is
simplified.)

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] vfs: take i_mutex on renamed file

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

A read delegation is used by NFSv4 as a guarantee that a client can
perform local read opens without informing the server.

The open operation takes the last component of the pathname as an
argument, thus is also a lookup operation, and giving the client the
above guarantee means informing the client before we allow anything that
would change the set of names pointing to the inode.

Therefore, we need to break delegations on rename, link, and unlink.

We also need to prevent new delegations from being acquired while one of
these operations is in progress.

We could add some completely new locking for that purpose, but it's
simpler to use the i_mutex, since that's already taken by all the
operations we care about.

The single exception is rename.  So, modify rename to take the i_mutex
on the file that is being renamed.

Also fix up lockdep and Documentation/filesystems/directory-locking to
reflect the change.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 Documentation/filesystems/directory-locking |   11 ++++++-----
 fs/namei.c                                  |    3 +++
 include/linux/fs.h                          |    9 ++++++---
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/directory-locking b/Documentation/filesystems/directory-locking
index ff7b611..9edbcd2 100644
--- a/Documentation/filesystems/directory-locking
+++ b/Documentation/filesystems/directory-locking
@@ -12,8 +12,8 @@ kinds of locks - per-inode (->i_mutex) and per-filesystem
 locks victim and calls the method.
 
 4) rename() that is _not_ cross-directory.  Locking rules: caller locks
-the parent, finds source and target, if target already exists - locks it
-and then calls the method.
+the parent, finds source and target, locks source, also locks target if
+it already exists, and then calls the method.
 
 5) link creation.  Locking rules:
 	* lock parent
@@ -30,6 +30,7 @@ rules:
 		fail with -ENOTEMPTY
 	* if new parent is equal to or is a descendent of source
 		fail with -ELOOP
+	* lock source if it is not a directory.
 	* if target exists - lock it.
 	* call the method.
 
@@ -56,9 +57,9 @@ objects - A < B iff A is an ancestor of B.
     renames will be blocked on filesystem lock and we don't start changing
     the order until we had acquired all locks).
 
-(3) any operation holds at most one lock on non-directory object and
-    that lock is acquired after all other locks.  (Proof: see descriptions
-    of operations).
+(3) locks on non-directory objects are acquired only after taking locks
+    on their parents (which remain their parents until all locks are
+    acquired, by (1) and (2)).  (Proof: see descriptions of operations).
 
 	Now consider the minimal deadlock.  Each process is blocked on
 attempt to acquire some lock and already holds at least one lock.  Let's
diff --git a/fs/namei.c b/fs/namei.c
index 208c6aa..d29b3c4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3073,6 +3073,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 			    struct inode *new_dir, struct dentry *new_dentry)
 {
 	struct inode *target = new_dentry->d_inode;
+	struct inode *source = old_dentry->d_inode;
 	int error;
 
 	error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry);
@@ -3080,6 +3081,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 		return error;
 
 	dget(new_dentry);
+	mutex_lock_nested(&source->i_mutex, I_MUTEX_RENAME_SOURCE);
 	if (target)
 		mutex_lock(&target->i_mutex);
 
@@ -3098,6 +3100,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 out:
 	if (target)
 		mutex_unlock(&target->i_mutex);
+	mutex_unlock(&source->i_mutex);
 	dput(new_dentry);
 	return error;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 386da09..e537b67 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -855,10 +855,12 @@ static inline int inode_unhashed(struct inode *inode)
  * 0: the object of the current VFS operation
  * 1: parent
  * 2: child/target
- * 3: quota file
+ * 3: xattr
+ * 4: quota file
+ * 5: the file being renamed (used only in rename of a non-directory)
  *
  * The locking order between these classes is
- * parent -> child -> normal -> xattr -> quota
+ * parent -> child -> rename_source -> normal -> xattr -> quota
  */
 enum inode_i_mutex_lock_class
 {
@@ -866,7 +868,8 @@ enum inode_i_mutex_lock_class
 	I_MUTEX_PARENT,
 	I_MUTEX_CHILD,
 	I_MUTEX_XATTR,
-	I_MUTEX_QUOTA
+	I_MUTEX_QUOTA,
+	I_MUTEX_RENAME_SOURCE
 };
 
 /*
-- 
1.7.5.4

[PATCH 2/7] locks: introduce new FL_DELEG lock flag

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

For now FL_DELEG is just a synonym for FL_LEASE.  So this patch doesn't
change behavior.

Next we'll modify break_lease to treat FL_DELEG leases differently, to
account for the fact that NFSv4 delegations should be broken in more
situations than Windows oplocks.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/locks.c          |    2 +-
 fs/nfsd/nfs4state.c |    2 +-
 include/linux/fs.h  |    1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 637694b..ea07a98 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -131,7 +131,7 @@
 
 #define IS_POSIX(fl)	(fl->fl_flags & FL_POSIX)
 #define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
-#define IS_LEASE(fl)	(fl->fl_flags & FL_LEASE)
+#define IS_LEASE(fl)	(fl->fl_flags & (FL_LEASE|FL_DELEG))
 
 static bool lease_breaking(struct file_lock *fl)
 {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8670863..38562ee 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2838,7 +2838,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp, int f
 		return NULL;
 	locks_init_lock(fl);
 	fl->fl_lmops = &nfsd_lease_mng_ops;
-	fl->fl_flags = FL_LEASE;
+	fl->fl_flags = FL_DELEG;
 	fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
 	fl->fl_end = OFFSET_MAX;
 	fl->fl_owner = (fl_owner_t)(dp->dl_file);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e537b67..9d7446e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1085,6 +1085,7 @@ static inline int file_check_writeable(struct file *filp)
 
 #define FL_POSIX	1
 #define FL_FLOCK	2
+#define FL_DELEG	4	/* NFSv4 delegation */
 #define FL_ACCESS	8	/* not trying to lock, just looking */
 #define FL_EXISTS	16	/* when unlocking, test for existence */
 #define FL_LEASE	32	/* lease held on this file */
-- 
1.7.5.4

[PATCH 3/7] locks: implement delegations

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Implement NFSv4 delegations at the vfs level using the new FL_DELEG lock
type.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/locks.c         |   49 +++++++++++++++++++++++++++++++++++++++----------
 include/linux/fs.h |   18 +++++++++++++++---
 2 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index ea07a98..29f911c 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1185,28 +1185,40 @@ static void time_out_leases(struct inode *inode)
 	}
 }
 
+static bool leases_conflict(struct file_lock *lease, struct file_lock *breaker)
+{
+	if ((breaker->fl_flags & FL_DELEG) && (lease->fl_flags & FL_LEASE))
+		return false;
+	return locks_conflict(breaker, lease);
+}
+
 /**
  *	__break_lease	-	revoke all outstanding leases on file
  *	@inode: the inode of the file to return
- *	@mode: the open mode (read or write)
+ *	@mode: O_RDONLY: break only write leases; O_WRONLY or O_RDWR:
+ *	    break all leases
+ *	@type: FL_LEASE: break leases and delegations; FL_DELEG: break
+ *	    only delegations
  *
  *	break_lease (inlined for speed) has checked there already is at least
  *	some kind of lock (maybe a lease) on this file.  Leases are broken on
  *	a call to open() or truncate().  This function can sleep unless you
  *	specified %O_NONBLOCK to your open().
  */
-int __break_lease(struct inode *inode, unsigned int mode)
+int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
 {
 	int error = 0;
 	struct file_lock *new_fl, *flock;
 	struct file_lock *fl;
 	unsigned long break_time;
 	int i_have_this_lease = 0;
+	bool lease_conflict = false;
 	int want_write = (mode & O_ACCMODE) != O_RDONLY;
 
 	new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK);
 	if (IS_ERR(new_fl))
 		return PTR_ERR(new_fl);
+	new_fl->fl_flags = type;
 
 	lock_flocks();
 
@@ -1216,13 +1228,16 @@ int __break_lease(struct inode *inode, unsigned int mode)
 	if ((flock == NULL) || !IS_LEASE(flock))
 		goto out;
 
-	if (!locks_conflict(flock, new_fl))
+	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
+		if (leases_conflict(fl, new_fl)) {
+			lease_conflict = true;
+			if (fl->fl_owner == current->files)
+				i_have_this_lease = 1;
+		}
+	}
+	if (!lease_conflict)
 		goto out;
 
-	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next)
-		if (fl->fl_owner == current->files)
-			i_have_this_lease = 1;
-
 	break_time = 0;
 	if (lease_break_time > 0) {
 		break_time = jiffies + lease_break_time * HZ;
@@ -1231,6 +1246,8 @@ int __break_lease(struct inode *inode, unsigned int mode)
 	}
 
 	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
+		if (!leases_conflict(fl, new_fl))
+			continue;
 		if (want_write) {
 			if (fl->fl_flags & FL_UNLOCK_PENDING)
 				continue;
@@ -1272,7 +1289,7 @@ restart:
 		 */
 		for (flock = inode->i_flock; flock && IS_LEASE(flock);
 				flock = flock->fl_next) {
-			if (locks_conflict(new_fl, flock))
+			if (leases_conflict(new_fl, flock))
 				goto restart;
 		}
 		error = 0;
@@ -1352,9 +1369,20 @@ int generic_add_lease(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;
 	struct inode *inode = dentry->d_inode;
+	bool is_deleg = (*flp)->fl_flags & FL_DELEG;
 	int error;
 
 	lease = *flp;
+	/*
+	 * In the delegation case we need mutual exclusion with
+	 * a number of operations that take the i_mutex.  We trylock
+	 * because delegations are an optional optimization, and if
+	 * there's some chance of a conflict--we'd rather not
+	 * bother, maybe that's a sign this just isn't a good file to
+	 * hand out a delegation on.
+	 */
+	if (is_deleg && !mutex_trylock(&inode->i_mutex))
+		return -EAGAIN;
 
 	error = -EAGAIN;
 	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
@@ -1406,9 +1434,10 @@ int generic_add_lease(struct file *filp, long arg, struct file_lock **flp)
 		goto out;
 
 	locks_insert_lock(before, lease);
-	return 0;
-
+	error = 0;
 out:
+	if (is_deleg)
+		mutex_unlock(&inode->i_mutex);
 	return error;
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9d7446e..2cc1adf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1207,7 +1207,7 @@ extern int vfs_test_lock(struct file *, struct file_lock *);
 extern int vfs_lock_file(struct file *, unsigned int, struct file_lock *, struct file_lock *);
 extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
 extern int flock_lock_file_wait(struct file *filp, struct file_lock *fl);
-extern int __break_lease(struct inode *inode, unsigned int flags);
+extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);
 extern void lease_get_mtime(struct inode *, struct timespec *time);
 extern int generic_setlease(struct file *, long, struct file_lock **);
 extern int vfs_setlease(struct file *, long, struct file_lock **);
@@ -1319,7 +1319,7 @@ static inline int flock_lock_file_wait(struct file *filp,
 	return -ENOLCK;
 }
 
-static inline int __break_lease(struct inode *inode, unsigned int mode)
+static inline int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
 {
 	return 0;
 }
@@ -1972,9 +1972,17 @@ static inline int locks_verify_truncate(struct inode *inode,
 static inline int break_lease(struct inode *inode, unsigned int mode)
 {
 	if (inode->i_flock)
-		return __break_lease(inode, mode);
+		return __break_lease(inode, mode, FL_LEASE);
 	return 0;
 }
+
+static inline int break_deleg(struct inode *inode, unsigned int mode)
+{
+	if (inode->i_flock)
+		return __break_lease(inode, mode, FL_DELEG);
+	return 0;
+}
+
 #else /* !CONFIG_FILE_LOCKING */
 static inline int locks_mandatory_locked(struct inode *inode)
 {
@@ -2014,6 +2022,10 @@ static inline int break_lease(struct inode *inode, unsigned int mode)
 	return 0;
 }
 
+static inline int break_deleg(struct inode *inode, unsigned int mode)
+{
+	return 0;
+}
 #endif /* CONFIG_FILE_LOCKING */
 
 /* fs/open.c */
-- 
1.7.5.4

[PATCH 4/7] locks: break delegations on unlink

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/namei.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d29b3c4..eb4166f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2788,6 +2788,9 @@ static long do_unlinkat(int dfd, const char __user *pathname)
 		error = security_path_unlink(&nd.path, dentry);
 		if (error)
 			goto exit3;
+		error = break_deleg(inode, O_WRONLY);
+		if (error)
+			goto exit3;
 		error = vfs_unlink(nd.path.dentry->d_inode, dentry);
 exit3:
 		mnt_drop_write(nd.path.mnt);
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] locks: break delegations on rename

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/namei.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index eb4166f..21a1d65 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3092,6 +3092,14 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 	if (d_mountpoint(old_dentry)||d_mountpoint(new_dentry))
 		goto out;
 
+	error = break_deleg(source, O_WRONLY);
+	if (error)
+		goto out;
+	if (target) {
+		error = break_deleg(target, O_WRONLY);
+		if (error)
+			goto out;
+	}
 	error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry);
 	if (error)
 		goto out;
-- 
1.7.5.4

[PATCH 6/7] locks: break delegations on link

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/namei.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 21a1d65..6f1dbfa 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2920,8 +2920,11 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 	/* Make sure we don't allow creating hardlink to an unlinked file */
 	if (inode->i_nlink == 0)
 		error =  -ENOENT;
-	else
-		error = dir->i_op->link(old_dentry, dir, new_dentry);
+	else {
+		error = break_deleg(inode, O_WRONLY);
+		if (!error)
+			error = dir->i_op->link(old_dentry, dir, new_dentry);
+	}
 	mutex_unlock(&inode->i_mutex);
 	if (!error)
 		fsnotify_link(dir, inode, new_dentry);
-- 
1.7.5.4

[PATCH 7/7] locks: break delegations on any attribute modification

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

NFSv4 uses leases to guarantee that clients can cache metadata as well
as data.

This covers chmod, chown, etc.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/attr.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 95053ad..b8b5676 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -232,6 +232,9 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
 	error = security_inode_setattr(dentry, attr);
 	if (error)
 		return error;
+	error = break_deleg(inode, O_WRONLY);
+	if (error)
+		return error;
 
 	if (inode->i_op->setattr)
 		error = inode->i_op->setattr(dentry, attr);
-- 
1.7.5.4

Re: [PATCH] vfs: take i_mutex on renamed file

[J. Bruce Fields]

On Fri, Mar 09, 2012 at 05:21:14PM -0500, J. Bruce Fields wrote:
> On Mon, Mar 05, 2012 at 05:43:34PM -0500, J. Bruce Fields wrote:
> > Al, do you see any reason this won't work?
> > 
> > After this, all I need to make nfsd happy is a patch to locks.c to
> > implement delegations, and then a few one-liners to add break_deleg
> > calls in the right operations.  So this is the one non-trivial change I
> > have to core vfs code.
> > 
> > All of those patches are also ready, and I'd like to merge them for 3.4.
> 
> By the way, the following is the rest of the series.
> 
> (Compared to the last posting the deleg-breaking interface is
> simplified.)

What are the chances of getting this reviewed for this merge cycle?  Is
it on somebody's list?

If it would make it any simpler: the one that I think absolutely must get wider
review, and get merged through Al's tree, is "vfs: take i_mutex on renamed
file".

There aren't actually any dependencies between that and the rest; and
the rest is mainly in nfsd/ and locks.c (changes to which have mostly
been going through my tree anyway, I think); other than that:

 fs/attr.c           |    3 ++
 fs/namei.c          |   18 +++++++++++++-

straightforward changes that tend to look like:

	+	error = break_deleg(inode, O_WRONLY);
	+	if (error)
	+		return error;

 include/linux/fs.h  |   19 +++++++++++++--

and some definitions.

--b.

Re: [PATCH] vfs: take i_mutex on renamed file

[J. Bruce Fields]

On Tue, Mar 20, 2012 at 03:27:12PM -0400, J. Bruce Fields wrote:
> What are the chances of getting this reviewed for this merge cycle?  Is
> it on somebody's list?
> 
> If it would make it any simpler: the one that I think absolutely must get wider
> review, and get merged through Al's tree, is "vfs: take i_mutex on renamed
> file".
> 
> There aren't actually any dependencies between that and the rest;

Based on discussion at LSF, and absent any objections, I'm leaving "vfs:
take i_mutex on renamed file" for Al to take, and merging the rest
through the nfsd tree for 3.5.  (Al, assuming that's the correct thing
to do, should I add an acked-by/reviewed-by for you to the patches I
merge?)

--b.

> and
> the rest is mainly in nfsd/ and locks.c (changes to which have mostly
> been going through my tree anyway, I think); other than that:
> 
>  fs/attr.c           |    3 ++
>  fs/namei.c          |   18 +++++++++++++-
> 
> straightforward changes that tend to look like:
> 
> 	+	error = break_deleg(inode, O_WRONLY);
> 	+	if (error)
> 	+		return error;
> 
>  include/linux/fs.h  |   19 +++++++++++++--
> 
> and some definitions.
> 
> --b.