Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with i_mutex held

[Ian Kent <raven@themaw.net>]

Sage Weil wrote:
> On Thu, 26 Mar 2009, Ian Kent wrote:
>>> Would it be possible to avoid the upcall in revalidate, and instead fail 
>>> and let the subsequent lookup path do it?  (I'm guessing the upcall 
>>> doesn't happen for _every_ revalidate?)
>> Yes, that's right, just every revalidate from processes that aren't the
>> automount process itself. The normal case is the mount succeeds and
>> further walks follow the mount from then on until it expires.
>>
>> It was more than three years ago when I tried to make everything go
>> through lookup so my memory is pretty unclear now. In the end I think
>> there was one case in real_lookup() where the lookup was skipped,
>> revalidate was called and failed but lookup wasn't then called again and
>> I got an incorrect failure.
> 
> That is _exactly_ the bug this patch is fixing.  :)  A (racing) process 
> ends up in real_lookup(), takes the mutex and finds the dentry has already 
> been added to the cache by someone else.  The mutex is dropped, revalidate 
> is called, and if it fails, real_lookup() returns ENOENT (!!) without ever 
> trying lookup.  The basic problem is that the fs revalidate might fail, 
> expecting lookup to get called, but real_lookup() returns ENOENT 
> instead... which is just wrong.
> 
>> AFAICR all other code paths that hold the
>> mutex over lookup and revalidate perform the revalidate first and then
>> the lookup if that fails, which avoids this case.
> 
> If you mean the paths autofs manages to avoid (unlinkat, rmdir, etc.), 
> yeah: the mutex is taken, then cached_lookup() (and revalidate), then 
> lookup if necessary.  Holding the mutex over revalidate avoids dealing 
> with various races.
> 
> So, it sounds like this fix would need to go in along with an autofs patch 
> that moves the upcall back into lookup exclusively, now that a revalidate 
> failure does the right thing (always calls lookup).  Hopefully that would 
> mean a net simplification on the autofs side as well?

Maybe ... doesn't look like it so far ... but it's to soon to tell.

autofs4 - always use lookup for mount

From: Ian Kent <raven@themaw.net>


---

 fs/autofs4/autofs_i.h |   15 ------
 fs/autofs4/root.c     |  117
++++++++++++++++++++++++++++++++++---------------
 2 files changed, 81 insertions(+), 51 deletions(-)

Ian