On Mon, Jul 18, 2011 at 01:35:14PM -0400, Trond Myklebust wrote: > On Mon, 2011-07-18 at 11:26 -0400, Jeff Layton wrote: > > If the task that initiated the sillyrename ends up being killed by a > > fatal signal, then it will eventually return back to userspace and end > > up releasing the i_mutex. d_move however needs to be done while holding > > the i_mutex. > > Umm... Where is this requirement documented? I thought the rename_lock > was there to protect against lookup races etc with d_move. It protects lookup against d_move(). It does *NOT* protect the i_mutex locking scheme from deadlocks a-sodding-plenty and it does not protect ->d_parent/->d_name accesses in directory methods (->i_mutex does). The latter is not a big deal, but the former is. > Besides, NFS already has > nfs_block_sillyrename()/nfs_unblock_sillyrename() to provide further > exclusion between dentry lookups and revalidations and the silly-unlink > code. It's broken. We are dealing with more than just NFS data structures. Don't change ->d_parent unless you hold ->i_mutex on parent(s) involved and if they are different you need ->s_vfs_rename_mutex as well. See lock_rename() in fs/namei.c and Documentation/filesystems/directory-locking. Moreover, I would be very sceptical about the code trying to grap ->i_mutex on ->d_parent of preexisting dentry, unless you have very good reasons to be sure that it couldn't be moved around in the meanwhile. d_move() in async rename is really broken... -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
