Sage Weil wrote:
> On Wed, 29 Jul 2009, Ian Kent wrote:
>>> How is it possible for a revalidate to fail and then to skip over the
>>> ->lookup() call? That sounds like a problem with the VFS? (Like the one
>>> I'm trying to fix... you're testing with the real_lookup patch applied, I
>>> hope?)
>> It's been hard to work out exactly what's happening but I suspect it is
>> due to multiple walks occurring while the the dentry hashed state
>> changes. Like, one process hits revalidate, drops the dentry, another
>> process comes along and goes straight to lookup and rehashes the dentry,
>> the original process, that dropped the dentry, then ends up not calling
>> lookup. The race only happens occasionally, and my test uses 10
>> processes that tend to hit the same dentrys at the same time at various
>> points in the mount tree over 150 iterations. Admittedly, it's a fairly
>> stressful test as autofs goes but it does tend to show up races quite well.
>
> If I'm reading namei.c correctly, every ->d_revalidate() that returns NULL
> will always result in a ->lookup() (unless IS_DEADDIR(inode)), without any
> regard for the dentry's hashed/unhashed status. The exception appears to
> be a call in __link_path_walk when FS_REVAL_DOT is set in the superblock
> (nfs only):
>
> if (nd->path.dentry && nd->path.dentry->d_sb &&
> (nd->path.dentry->d_sb->s_type->fs_flags & FS_REVAL_DOT)) {
> err = -ESTALE;
> /* Note: we do not d_invalidate() */
> if (!nd->path.dentry->d_op->d_revalidate(
> nd->path.dentry, nd))
> break;
>
> in which case userspace would see an ESTALE.
>
> It sounds like a few well placed printks that include the pid for unhash,
> rehash, revalidate, and lookup would narrow down what is going on?
Of course I've been doing that.
I think you need to re-read what I wrote above. Your analysis is fine if
there is only one process concurrently walking onto the dentry. If a
second process beats the revalidate process to real_lookup() and
incorrectly rehashes the dentry the following revalidate process will
not call ->lookup().
My most recent try at this seems to be working.
I need to clean up the printks and alter one of the comments and re-test
with a full test (I have been working with only half the test enabled as
it was most problematic). The testing takes a long time to run, 1 hour
10 minutes for each of two configurations and the criteria for success
in at least six consecutive successful runs, that's about thirteen
hours. But, hopefully, when that is done I'll have a series to post for
comment.
>
>> I always knew this was going to be difficult but I didn't expect it to
>> not be possible, given that I'm using your VFS locking change. If this
>> continues to be a problem I may need to start thinking about some small
>> VFS changes to help work around the difficulty.
>
> Thanks for continuing to work on this!
> sage
--
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
