From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: ->d_lock FUBAR (was Re: Linux 3.0-rc6) Date: Wed, 13 Jul 2011 02:39:36 +0100 Message-ID: <20110713013936.GL11013@ZenIV.linux.org.uk> References: <20110711060315.GI11013@ZenIV.linux.org.uk> <20110712234806.GJ11013@ZenIV.linux.org.uk> <20110713005634.GK11013@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, npiggin@kernel.dk To: Linus Torvalds Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:39502 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755008Ab1GMBjh (ORCPT ); Tue, 12 Jul 2011 21:39:37 -0400 Content-Disposition: inline In-Reply-To: <20110713005634.GK11013@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Jul 13, 2011 at 01:56:34AM +0100, Al Viro wrote: > Nick, could you please describe the locking rules you had in mind for > ->d_lock? unlazy_walk() (aka nameidata_dentry_drop_rcu()) can probably > be dealt with by checking d_seq twice, once before locking the child. > Then we could be sure that it's still a child of parent and will stay > so as long as parent's ->d_lock is held, and thus the ordering would > stay stable... As the matter of fact, can we ever get there with IS_ROOT(dentry)? AFAICS, that should be impossible - dentry->d_seq would have to be changed by whatever had torn it off the tree and we would have buggered off on __d_rcu_to_refcount() failing... AFAICS, the only way to get there would be with mountpoint crossing returning a symlink with symlink already killed by rename() somehow (call in walk_component()). The first part should be impossible - symlinks can't be mounted/bound on anything (and if it would be possible, we'd trigger that BUG_ON() if symlink was still alive, anyway). So here's what I want to do to unlazy_walk(); it'll almost certainly leave other problems with ->d_lock, but at least it'll take care of that one: Make sure that child is still a child of parent before nested locking of child->d_lock in unlazy_walk(); otherwise we are risking a violation of locking order and deadlocks. Signed-off-by: Al Viro --- diff --git a/fs/namei.c b/fs/namei.c index 0223c41..5c867dd 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -433,6 +433,8 @@ static int unlazy_walk(struct nameidata *nd, struct dentry *dentry) goto err_parent; BUG_ON(nd->inode != parent->d_inode); } else { + if (dentry->d_parent != parent) + goto err_parent; spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED); if (!__d_rcu_to_refcount(dentry, nd->seq)) goto err_child;