On Tue, Feb 02, 2010 at 05:36:31PM +1100, Nick Piggin wrote: > I ended up having to use a seqlock to do name comparison without locks > (and without holding references for that matter, just RCU). However > name comparison is obviously a lot more critical because you can't > ignore races, so you might be able to do something simpler. Umm... Your ->d_seq, you mean? IIRC, that stuff landed too close to do_filp_open() queue back then *and* had potential headache from hell with vfsmount side of that business. I've reread it now. Comments: * mntput() is blocking. When refcount goes to 0, we get dput() on root, possibly followed by actual fs shutdown. Very much not an RCU fodder, even though most of the calls will be OK. We can do a variant that would do atomic_dec_and_bail() instead ;-) I.e. decrement atomically if greater than 1, bail out otherwise. * why do you bail out on LOOKUP_PARENT? For that matter, why do you do that so late? * how does that interact with d_materialise_unique()? Sure, you bail out on NFS anyway, but it's still not nice to leave relying just on that. * why can we access dentry->d_inode->i_op contents? Or dentry->d_op one, for that matter... BTW, I'd love to take the entire "last component" part of link_path_walk() out into a separate function and away from link_path_walk(), leaving basically just the LOOKUP_PARENT case in there. Price: trailing symlinks need to be handled by an iterative loop in do_follow_link(). And that actually ends up an improvement both in stack depth and in overall code cleanup. Nothing like the horrors in do_filp_open(), TYVM (said horrors had mostly gone away in #untested, BTW, and I'm going to move that series to for-next shortly). However, we are still several prereqs away from link_path_walk() split, so that'll have to wait a bit. In any case, LOOKUP_PARENT is very much worth the first-class treatment... -- 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
