rename locking

rename locking

[J. Bruce Fields]

This is a resend of the rename locking patches.  With the quota locking
cleaned up, it looks safe to move ext4's locking into common code and
then use that.

Steven Whitehouse has another plan for gfs2, but last I talked to him
was OK with Jan Kara's fix for now.

--b.

[PATCH 1/6] gfs2: Get rid of I_MUTEX_QUOTA usage

[J. Bruce Fields]

From: Jan Kara <jack@suse.cz>

GFS2 uses i_mutex on its system quota inode to synchronize writes to
quota file. Since this is an internal inode to GFS2 (not part of directory
hiearchy or visible by user) we are safe to define locking rules for it. So
let's just get it its own locking class to make it clear.

CC: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/gfs2/ops_fstype.c |    8 ++++++++
 fs/gfs2/quota.c      |    2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index b8c250f..c714ef7 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -19,6 +19,7 @@
 #include <linux/mount.h>
 #include <linux/gfs2_ondisk.h>
 #include <linux/quotaops.h>
+#include <linux/lockdep.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -766,6 +767,7 @@ fail:
 	return error;
 }
 
+static struct lock_class_key gfs2_quota_imutex_key;
 
 static int init_inodes(struct gfs2_sbd *sdp, int undo)
 {
@@ -803,6 +805,12 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
 		fs_err(sdp, "can't get quota file inode: %d\n", error);
 		goto fail_rindex;
 	}
+	/*
+	 * i_mutex on quota files is special. Since this inode is hidden system
+	 * file, we are safe to define locking ourselves.
+	 */
+	lockdep_set_class(&sdp->sd_quota_inode->i_mutex,
+			  &gfs2_quota_imutex_key);
 
 	error = gfs2_rindex_update(sdp);
 	if (error)
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index b97178e..8ed70e7 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -772,7 +772,7 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
 		return -ENOMEM;
 
 	sort(qda, num_qd, sizeof(struct gfs2_quota_data *), sort_qd, NULL);
-	mutex_lock_nested(&ip->i_inode.i_mutex, I_MUTEX_QUOTA);
+	mutex_lock(&ip->i_inode.i_mutex);
 	for (qx = 0; qx < num_qd; qx++) {
 		error = gfs2_glock_nq_init(qda[qx]->qd_gl, LM_ST_EXCLUSIVE,
 					   GL_NOCACHE, &ghs[qx]);
-- 
1.7.9.5

Re: [PATCH 1/6] gfs2: Get rid of I_MUTEX_QUOTA usage

[Steven Whitehouse]

Hi,

Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Eventually we should be able to drop i_mutex from here entirely, but
that proved more problematic that I'd expected, so this is a good
interim fix,

Steve.

On Wed, 2012-05-30 at 21:50 -0400, J. Bruce Fields wrote:
> From: Jan Kara <jack@suse.cz>
> 
> GFS2 uses i_mutex on its system quota inode to synchronize writes to
> quota file. Since this is an internal inode to GFS2 (not part of directory
> hiearchy or visible by user) we are safe to define locking rules for it. So
> let's just get it its own locking class to make it clear.
> 
> CC: Steven Whitehouse <swhiteho@redhat.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/gfs2/ops_fstype.c |    8 ++++++++
>  fs/gfs2/quota.c      |    2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index b8c250f..c714ef7 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -19,6 +19,7 @@
>  #include <linux/mount.h>
>  #include <linux/gfs2_ondisk.h>
>  #include <linux/quotaops.h>
> +#include <linux/lockdep.h>
>  
>  #include "gfs2.h"
>  #include "incore.h"
> @@ -766,6 +767,7 @@ fail:
>  	return error;
>  }
>  
> +static struct lock_class_key gfs2_quota_imutex_key;
>  
>  static int init_inodes(struct gfs2_sbd *sdp, int undo)
>  {
> @@ -803,6 +805,12 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
>  		fs_err(sdp, "can't get quota file inode: %d\n", error);
>  		goto fail_rindex;
>  	}
> +	/*
> +	 * i_mutex on quota files is special. Since this inode is hidden system
> +	 * file, we are safe to define locking ourselves.
> +	 */
> +	lockdep_set_class(&sdp->sd_quota_inode->i_mutex,
> +			  &gfs2_quota_imutex_key);
>  
>  	error = gfs2_rindex_update(sdp);
>  	if (error)
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index b97178e..8ed70e7 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -772,7 +772,7 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
>  		return -ENOMEM;
>  
>  	sort(qda, num_qd, sizeof(struct gfs2_quota_data *), sort_qd, NULL);
> -	mutex_lock_nested(&ip->i_inode.i_mutex, I_MUTEX_QUOTA);
> +	mutex_lock(&ip->i_inode.i_mutex);
>  	for (qx = 0; qx < num_qd; qx++) {
>  		error = gfs2_glock_nq_init(qda[qx]->qd_gl, LM_ST_EXCLUSIVE,
>  					   GL_NOCACHE, &ghs[qx]);

Re: [PATCH 1/6] gfs2: Get rid of I_MUTEX_QUOTA usage

[Bob Peterson]

----- Original Message -----
| From: Jan Kara <jack@suse.cz>
| 
| GFS2 uses i_mutex on its system quota inode to synchronize writes to
| quota file. Since this is an internal inode to GFS2 (not part of
| directory
| hiearchy or visible by user) we are safe to define locking rules for
| it. So
| let's just get it its own locking class to make it clear.

Hi,

Acked-by: Bob Peterson <rpeterso@redhat.com>

Regards,

Bob Peterson
Red Hat File Systems

[PATCH 2/6] vfs: fix outdated i_mutex_lock_class documentation

[J. Bruce Fields]

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

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 include/linux/fs.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 038076b..5584a29 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -861,7 +861,8 @@ 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
  *
  * The locking order between these classes is
  * parent -> child -> normal -> xattr -> quota
-- 
1.7.9.5

Re: [PATCH 2/6] vfs: fix outdated i_mutex_lock_class documentation

[Jan Kara]

On Wed 30-05-12 21:50:12, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  include/linux/fs.h |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 038076b..5584a29 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -861,7 +861,8 @@ 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
  This isn't correct anymore, is it? Just remove references to quota in this
comment...

>   *
>   * The locking order between these classes is
>   * parent -> child -> normal -> xattr -> quota

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 2/6] vfs: fix outdated i_mutex_lock_class documentation

[Jan Kara]

On Thu 31-05-12 20:51:11, Jan Kara wrote:
> On Wed 30-05-12 21:50:12, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  include/linux/fs.h |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 038076b..5584a29 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -861,7 +861,8 @@ 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
>   This isn't correct anymore, is it? Just remove references to quota in this
> comment...
  Oh, now I've noticed patch 6/6. Maybe this patch and 6/6 should be just
joined into one?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 2/6] vfs: fix outdated i_mutex_lock_class documentation

[J. Bruce Fields]

On Thu, May 31, 2012 at 08:54:39PM +0200, Jan Kara wrote:
> On Thu 31-05-12 20:51:11, Jan Kara wrote:
> > On Wed 30-05-12 21:50:12, J. Bruce Fields wrote:
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > 
> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > ---
> > >  include/linux/fs.h |    3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > index 038076b..5584a29 100644
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -861,7 +861,8 @@ 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
> >   This isn't correct anymore, is it? Just remove references to quota in this
> > comment...
>   Oh, now I've noticed patch 6/6. Maybe this patch and 6/6 should be just
> joined into one?

Sure.--b.

[PATCH 6/6 (v2)] vfs: rename I_MUTEX_QUOTA now that it's not used for quotas

[J. Bruce Fields]

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

I_MUTEX_QUOTA is now just being used whenever we want to lock two
non-directories.  So the name isn't right.  I_MUTEX_NONDIR2 isn't
especially elegant but it's the best I could think of.

Also fix some outdated documentation.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/inode.c         |    4 ++--
 include/linux/fs.h |    9 ++++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

On Thu, May 31, 2012 at 04:11:50PM -0400, bfields wrote:
> On Thu, May 31, 2012 at 08:54:39PM +0200, Jan Kara wrote:
> >   Oh, now I've noticed patch 6/6. Maybe this patch and 6/6 should be just
> > joined into one?
> 
> Sure.--b.

So, we'd omit patch 2 and replace the previous patch 6 by this.  The
other patches are unchanged.

diff --git a/fs/inode.c b/fs/inode.c
index 0272ac3..9c828ab 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -979,11 +979,11 @@ void lock_two_nondirectories(struct inode *inode1, struct inode *inode2)
 		mutex_lock(&inode1->i_mutex);
 	else if (inode1 < inode2) {
 		mutex_lock(&inode1->i_mutex);
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_QUOTA);
+		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_NONDIR2);
 
 	} else {
 		mutex_lock(&inode2->i_mutex);
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_QUOTA);
+		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_NONDIR2);
 	}
 }
 EXPORT_SYMBOL(lock_two_nondirectories);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 09434db..f0b5a5d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -861,10 +861,13 @@ 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: second non-directory
+ * The last is for certain operations (such as rename) which lock two
+ * non-directories at once.
  *
  * The locking order between these classes is
- * parent -> child -> normal -> xattr -> quota
+ * parent -> child -> normal -> xattr -> second non-directory
  */
 enum inode_i_mutex_lock_class
 {
@@ -872,7 +875,7 @@ enum inode_i_mutex_lock_class
 	I_MUTEX_PARENT,
 	I_MUTEX_CHILD,
 	I_MUTEX_XATTR,
-	I_MUTEX_QUOTA
+	I_MUTEX_NONDIR2
 };
 
 void lock_two_nondirectories(struct inode *, struct inode*);
-- 
1.7.9.5

[PATCH 3/6] vfs: pull ext4's double-i_mutex-locking into common code

[J. Bruce Fields]

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

We want to do this elsewhere as well.

Also, compare pointers instead of inode numbers.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/ext4/move_extent.c |   21 ++-------------------
 fs/inode.c            |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/fs.h    |    3 +++
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index c5826c6..b87d94a 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -1086,19 +1086,7 @@ mext_inode_double_lock(struct inode *inode1, struct inode *inode2)
 	if (ret < 0)
 		goto out;
 
-	if (inode1 == inode2) {
-		mutex_lock(&inode1->i_mutex);
-		goto out;
-	}
-
-	if (inode1->i_ino < inode2->i_ino) {
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT);
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD);
-	} else {
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT);
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD);
-	}
-
+	lock_two_nondirectories(inode1, inode2);
 out:
 	return ret;
 }
@@ -1123,12 +1111,7 @@ mext_inode_double_unlock(struct inode *inode1, struct inode *inode2)
 	if (ret < 0)
 		goto out;
 
-	if (inode1)
-		mutex_unlock(&inode1->i_mutex);
-
-	if (inode2 && inode2 != inode1)
-		mutex_unlock(&inode2->i_mutex);
-
+	unlock_two_nondirectories(inode1, inode2);
 out:
 	return ret;
 }
diff --git a/fs/inode.c b/fs/inode.c
index 6bc8761..e48a96f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -969,6 +969,42 @@ void unlock_new_inode(struct inode *inode)
 EXPORT_SYMBOL(unlock_new_inode);
 
 /**
+ * lock_two_nondirectories - take two i_mutexes on non-directory objects
+ * @inode1: first inode to lock; must be non-NULL
+ * @inode2: second inode to lock; optional, may equal first or be NULL.
+ */
+void lock_two_nondirectories(struct inode *inode1, struct inode *inode2)
+{
+	if (inode1 == inode2 || inode2 == NULL)
+		mutex_lock(&inode1->i_mutex);
+	else if (inode1 < inode2) {
+		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT);
+		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD);
+
+	} else {
+		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT);
+		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD);
+	}
+}
+EXPORT_SYMBOL(lock_two_nondirectories);
+
+/**
+ * lock_two_nondirectories - release locks from lock_two_nondirectories()
+ * @inode1: first inode to unlock
+ * @inode2: second inode to lock
+ *
+ * Arguments must be same as those given to corresponding
+ * lock_two_nondirectories() call.
+ */
+void unlock_two_nondirectories(struct inode *inode1, struct inode *inode2)
+{
+	mutex_unlock(&inode1->i_mutex);
+	if (inode2 && inode2 != inode1)
+		mutex_unlock(&inode2->i_mutex);
+}
+EXPORT_SYMBOL(unlock_two_nondirectories);
+
+/**
  * iget5_locked - obtain an inode from a mounted file system
  * @sb:		super block of file system
  * @hashval:	hash value (usually inode number) to get
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5584a29..308c2a0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -876,6 +876,9 @@ enum inode_i_mutex_lock_class
 	I_MUTEX_QUOTA
 };
 
+void lock_two_nondirectories(struct inode *, struct inode*);
+void unlock_two_nondirectories(struct inode *, struct inode*);
+
 /*
  * NOTE: in a 32bit arch with a preemptable kernel and
  * an UP compile the i_size_read/write must be atomic
-- 
1.7.9.5

[PATCH 4/6] vfs: don't use PARENT/CHILD lock classes for non-directories

[J. Bruce Fields]

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

Reserve I_MUTEX_PARENT and I_MUTEX_CHILD for locking of actual
directories.

(Also I_MUTEX_QUOTA isn't really a meaningful name for this locking
class any more; fixed in a later patch.)

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

diff --git a/fs/inode.c b/fs/inode.c
index e48a96f..0272ac3 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -978,12 +978,12 @@ void lock_two_nondirectories(struct inode *inode1, struct inode *inode2)
 	if (inode1 == inode2 || inode2 == NULL)
 		mutex_lock(&inode1->i_mutex);
 	else if (inode1 < inode2) {
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT);
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD);
+		mutex_lock(&inode1->i_mutex);
+		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_QUOTA);
 
 	} else {
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT);
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD);
+		mutex_lock(&inode2->i_mutex);
+		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_QUOTA);
 	}
 }
 EXPORT_SYMBOL(lock_two_nondirectories);
-- 
1.7.9.5

[PATCH 5/6] 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 |   30 +++++++++++++++++++--------
 fs/namei.c                                  |    7 +++----
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/Documentation/filesystems/directory-locking b/Documentation/filesystems/directory-locking
index ff7b611..9e8a629 100644
--- a/Documentation/filesystems/directory-locking
+++ b/Documentation/filesystems/directory-locking
@@ -2,6 +2,10 @@
 kinds of locks - per-inode (->i_mutex) and per-filesystem
 (->s_vfs_rename_mutex).
 
+	When taking the i_mutex on multiple non-directory objects, we
+always acquire the locks in order by increasing address.  We'll call
+that "inode pointer" order in the following.
+
 	For our purposes all operations fall in 5 classes:
 
 1) read access.  Locking rules: caller locks directory we are accessing.
@@ -12,8 +16,9 @@ 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 and finds source and target.  If source and target both
+exist, they are locked in inode pointer order.  Otherwise lock just
+source.  Then call method.
 
 5) link creation.  Locking rules:
 	* lock parent
@@ -30,7 +35,8 @@ rules:
 		fail with -ENOTEMPTY
 	* if new parent is equal to or is a descendent of source
 		fail with -ELOOP
-	* if target exists - lock it.
+	* If target exists, lock both source and target, in inode
+	  pointer order.  Otherwise lock just source.
 	* call the method.
 
 
@@ -56,9 +62,11 @@ 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 locks on
+    directory objects, and are acquired in inode pointer order.
+    (Proof: all operations but renames take lock on at most one
+    non-directory object, except renames, which take locks on source and
+    target in inode pointer order.)
 
 	Now consider the minimal deadlock.  Each process is blocked on
 attempt to acquire some lock and already holds at least one lock.  Let's
@@ -66,9 +74,13 @@ consider the set of contended locks.  First of all, filesystem lock is
 not contended, since any process blocked on it is not holding any locks.
 Thus all processes are blocked on ->i_mutex.
 
-	Non-directory objects are not contended due to (3).  Thus link
-creation can't be a part of deadlock - it can't be blocked on source
-and it means that it doesn't hold any locks.
+	By (3), any process holding a non-directory lock can only be
+waiting on another non-directory lock with a larger address.  Therefore
+the process holding the "largest" such lock can always make progress, and
+non-directory objects are not included in the set of contended locks.
+
+	Thus link creation can't be a part of deadlock - it can't be
+blocked on source and it means that it doesn't hold any locks.
 
 	Any contended object is either held by cross-directory rename or
 has a child that is also contended.  Indeed, suppose that it is held by
diff --git a/fs/namei.c b/fs/namei.c
index c651f02..421810b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3162,6 +3162,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);
@@ -3169,8 +3170,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 		return error;
 
 	dget(new_dentry);
-	if (target)
-		mutex_lock(&target->i_mutex);
+	lock_two_nondirectories(source, target);
 
 	error = -EBUSY;
 	if (d_mountpoint(old_dentry)||d_mountpoint(new_dentry))
@@ -3185,8 +3185,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 	if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE))
 		d_move(old_dentry, new_dentry);
 out:
-	if (target)
-		mutex_unlock(&target->i_mutex);
+	unlock_two_nondirectories(source, target);
 	dput(new_dentry);
 	return error;
 }
-- 
1.7.9.5

[PATCH 6/6] vfs: rename I_MUTEX_QUOTA now that it's not used for quotas

[J. Bruce Fields]

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

I_MUTEX_QUOTA is now just being used whenever we want to lock two
non-directories.  So the name isn't right.  I_MUTEX_NONDIR2 isn't
especially elegant but it's the best I could think of.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/inode.c         |    4 ++--
 include/linux/fs.h |    8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 0272ac3..9c828ab 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -979,11 +979,11 @@ void lock_two_nondirectories(struct inode *inode1, struct inode *inode2)
 		mutex_lock(&inode1->i_mutex);
 	else if (inode1 < inode2) {
 		mutex_lock(&inode1->i_mutex);
-		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_QUOTA);
+		mutex_lock_nested(&inode2->i_mutex, I_MUTEX_NONDIR2);
 
 	} else {
 		mutex_lock(&inode2->i_mutex);
-		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_QUOTA);
+		mutex_lock_nested(&inode1->i_mutex, I_MUTEX_NONDIR2);
 	}
 }
 EXPORT_SYMBOL(lock_two_nondirectories);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 308c2a0..f0b5a5d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -862,10 +862,12 @@ static inline int inode_unhashed(struct inode *inode)
  * 1: parent
  * 2: child/target
  * 3: xattr
- * 4: quota file
+ * 4: second non-directory
+ * The last is for certain operations (such as rename) which lock two
+ * non-directories at once.
  *
  * The locking order between these classes is
- * parent -> child -> normal -> xattr -> quota
+ * parent -> child -> normal -> xattr -> second non-directory
  */
 enum inode_i_mutex_lock_class
 {
@@ -873,7 +875,7 @@ enum inode_i_mutex_lock_class
 	I_MUTEX_PARENT,
 	I_MUTEX_CHILD,
 	I_MUTEX_XATTR,
-	I_MUTEX_QUOTA
+	I_MUTEX_NONDIR2
 };
 
 void lock_two_nondirectories(struct inode *, struct inode*);
-- 
1.7.9.5