On Tue, 17 Nov 2009, akpm@xxxxxxxxxxxxxxxxxxx wrote: > From: Sage Weil <sage@xxxxxxxxxxxx> > > real_lookup() is called by do_lookup() if dentry revalidation fails. If > the cache is re-populated while waiting for i_mutex, it may find that a > d_lookup() subsequently succeeds (see the "Uhhuh! Nasty case" comment). > > Previously, real_lookup() would drop i_mutex and do_revalidate() again. > If revalidate failed _again_, however, it would give up with -ENOENT. The > problem here that network file systems may be invalidating dentries via > server callbacks, e.g. due to concurrent access from another client, and > -ENOENT is frequently the wrong answer. > > This problem has been seen with both Lustre and Ceph. It seems possible > to hit this case with NFS as well if the cache lifetime is very short. > > Instead, we should do_revalidate() while i_mutex is still held. If > revalidation fails, we can move on to a ->lookup() and ensure a correct > result without worrying about any subsequent races. > > Note that do_revalidate() is called with i_mutex held elsewhere. For > example, do_filp_open(), lookup_create(), do_unlinkat(), do_rmdir(), and > possibly others all take the directory i_mutex, and then > > -> lookup_hash > -> __lookup_hash > -> cached_lookup > -> do_revalidate > > so this does not introduce any new locking rules for d_revalidate > implementations. > > Yes, the goto is ugly. A cleanup patch follows. > > Cc: Ian Kent <raven@xxxxxxxxxx> > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > Cc: Andreas Dilger <adilger@xxxxxxx> > Signed-off-by: Yehuda Sadeh <yehuda@xxxxxxxxxxxx> > Signed-off-by: Sage Weil <sage@xxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Acked-by: Miklos Szeredi <mszeredi@xxxxxxx> -- 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
