Re: [PATCH 4/6] leases: break read leases on rename

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, Sep 23, 2011 at 12:55:13PM -0400, J. Bruce Fields wrote:
> On Thu, Sep 22, 2011 at 01:17:49PM -0400, Mimi Zohar wrote:
> > On Wed, 2011-09-21 at 10:58 -0400, J. Bruce Fields wrote: 
> > > To rely on the i_mutex for exclusion between setlease and rename, we
> > > need rename to take the i_mutex on the source as well as on any possible
> > > target.
> > > 
> > > I suspect this is deadlock-free, but I need to think this proof through
> > > again.  And I'm not sure what to do about lockdep.
> > 
> > Not sure that I will be of any help, but how about posting the lockdep
> > messages?
> 
> Sure, appended below, but it's not particularly surprising--we're taking
> i_mutex's on four different objects (both parents, source, and target if
> any) where before there were three.
> 
> I suppose the solution is another i_mutex lock class, used only on the
> lock of the source inode?

That'd be something like this.  (Works for me, anyway.)  I'll fold it
into the previous patch.

--b.

diff --git a/fs/namei.c b/fs/namei.c
index f6de42d..06a8d95 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3069,7 +3069,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 		return error;
 
 	dget(new_dentry);
-	mutex_lock(&source->i_mutex);
+	mutex_lock_nested(&source->i_mutex, I_MUTEX_RENAME_SOURCE);
 	error = break_lease(source, O_WRONLY);
 	if (error)
 		goto out_unlock_source;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 76460ed..74f4979 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -839,10 +839,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
 {
@@ -850,7 +852,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
 };
 
 /*