On Mon, Jul 04, 2011 at 04:09:29PM -0700, Linus Torvalds wrote: > Other than the isci driver, the rest really is just lots of random > small stuff. It's getting to the point where I'm thinking I should > just release 3.0, because it's been pretty quiet, and the fixes > haven't been earth-shakingly exciting. Some drm (radeon and intel) > fixes might be noticeable to more people, the rest would tend to be > pretty esoteric. Sigh... Looks like we have serious problems around ->d_parent handling. First of all, __d_unalias() is fscked - calling d_ancestor() is not going to do us any good before we made sure that tree topology won't change right under us. Used to be protected by dcache_lock, but not anymore. Moreover, there's a similar problem with __d_materialise_dentry() side of things; there we don't check for loop creation at all and with NFS we just might try to attach a root of disconnected subtree *inside* that subtree. No check and no locking either... Another piece of PITA - cifs_get_root() will cheerfully call d_materialise_unique() on dentry it got from d_lookup() if it happens to be negative to start with *and* directory had been created on server in the meanwhile. BUG_ON() triggered in d_materialise_unique()... The lack of i_mutex on parent also doesn't help. cifs_get_root() mess is from this cycle, BTW. btrfs get_default_root() doesn't grab i_mutex either. It should, since it calls d_splice_alias(). I'm really not fond of the code in dentry_lock_for_move(); we _might_ manage to avoid deadlocks since i_mutex serialization might prevent contention on d_lock, but I'm still not convinced, especially due to missing i_mutex in several callers... I mean, this * If there is an ancestor relationship: * dentry->d_parent->...->d_parent->d_lock * ... * dentry->d_parent->d_lock * dentry->d_lock * * If no ancestor relationship: * if (dentry1 < dentry2) * dentry1->d_lock * dentry2->d_lock is no good: suppose A is ancestor of B and C is unrelated to either. With B sitting at lower address than C and A at higher one. We have A before B, since it's an ancestor; C before A since they are unrelated and addresses compare that way; B before C (ditto). Loops in lock ordering are generally bad; we _might_ get away with that in this case since we serialize d_move() callers to hell and back, but... Al, going through ->d_parent code review and not happy about the state of that animal... -- 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
