On Fri, Jul 29, 2011 at 08:24:41AM +0100, Al Viro wrote: ... > > > > I'll defer to Al on that one - the intricacies of the rename locking > > are way over my head. > > I'm not sure that's safe. Note that one use of rename_lock is that > we allow hash lookup to race with d_move(). Which can move object > from one hash chain to another, so hash lookup may end up jumping > from one chain to another and getting a false negative. That's > why __d_lookup() is not safe without read_seqretry loop (or seq_writelock, > of course). > > But look what happens if we do per-sb locks - d_move() derailing the > hash lookup might happen on *any* filesystem. They all share the > same hash table. So just checking that we hadn't done renames on > our filesystem is not enough to make sure we hadn't hit a false > negative. > > Unless we go for making the hashtable itself per-superblock (and I really > doubt that it's a good idea), I don't see any obvious ways to avoid that > kind of race. IOW, how would you implement safe d_lookup()? Hi Al, (a bit late reply actually ;) but what about first two patches (without per-sb locks)? They simply introduce read_seqlock/unlock without modification of sl->sequence. The functions which were calling for read_seqretry -- still remains and do continue calling for read_seqretry. The following functions were converted to read_seqlock/unlock - have_submounts (still have the read_seqretry on unlocked path) - select_parent (still have the read_seqretry on unlocked path) - __d_path - d_path - d_path_with_unreachable - dentry_path_raw - dentry_path - getcwd - d_genocide (still have the read_seqretry on unlocked path) While both d_move() and d_materialise_unique() remains using write_seqlock. So it seems I'm a bit confused/screwed why can't we do so... Cyrill -- 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
