From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [PATCH 4/6] leases: break read leases on rename Date: Fri, 23 Sep 2011 14:55:46 -0400 Message-ID: <20110923185546.GA7389@fieldses.org> References: <1316617097-21384-1-git-send-email-bfields@redhat.com> <1316617097-21384-5-git-send-email-bfields@redhat.com> <1316711869.3159.54.camel@localhost.localdomain> <20110923165510.GA807@pad.fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Mimi Zohar , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org, Christoph Hellwig , Al Viro To: "J. Bruce Fields" Return-path: Content-Disposition: inline In-Reply-To: <20110923165510.GA807-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> Sender: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.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 }; /* -- 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