On Tue, Jul 12, 2011 at 07:40:01PM -0700, Linus Torvalds wrote: > It's dentry_lock_for_move() that makes me really nervous. Not only > does it lock up to four dentries, but it mixes the whole parenthood vs > pointer ordering. Or course, it does have those BUG_ON() checks, so > it should never cause any circular dependencies, but still.. Me too, obviously. > The actual main protection to get lookups correct in the presence of > concurrent moves largely depends on the sequence numbers (ie > d_lookup() retrying if it hits a rename), which is why I also find it > unlikely that we really should need to hold all those d_lock cases all > at the same time. > > So does d_move() really need to get all the locks at the same time and > then do all the operations inside that "super-locked" region? Or could > we get the locks in sequence and do individual parts of the rename > operations under individual locks? > > Are there any other d_lock cases that depend on the pointer ordering? > Most everything else seems to be about direct parenthood, no? It's not that easy. We want ->d_lock on parents - not only because there's code iterating through the list of children, but because ordering on direct parenthood bloody depends on children not moving out while we hold ->d_lock on their parent. Otherwise we are looking for nightmares in shrink_dcache_parent() et.al. I'm not sure how much do we care about stability of x->d_parent when x->d_lock is held. ->d_compare() is the most obvious potential area of trouble in that respect, but there might be more. I'm still not finished reviewing ->d_lock uses; about a couple of hundreds is left to wade through. I would really, *REALLY* appreciate explicitly defined locking rules from Nick (it's his changes, mostly). As in "this, this and that field is protected by ->d_lock on..." Note that ->d_parent is stable when i_mutex is held on parent, which makes most of the users of ->d_parent safe and fine (->lookup(), etc. are all called with directory locked). I've not finished reviewing ->d_parent users either, but IMO ->d_lock review is more important, so it got bumped in front of queue... Back to GrepTFS and stripping the paint off the walls... -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
