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? 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
