Re: autofs4 looks up wrong path element when ghosting is enabled

[Ian Kent <raven@themaw.net>]

On Wed, 19 Oct 2005, Jeff Moyer wrote:

> ==> Regarding Re: autofs4 looks up wrong path element when ghosting is enabled; Ian Kent <raven@themaw.net> adds:
> 
> raven> On Mon, 26 Sep 2005, Jeff Moyer wrote:
> >> ==> Regarding Re: autofs4 looks up wrong path element when ghosting is
> >> enabled; Ian Kent <raven@themaw.net> adds:
> >> 
> raven> On Sat, 24 Sep 2005, Jeff Moyer wrote:
> >> >> >> >> >> >> Ian, I'm not really sure how we can address this issue >>
> >> without VFS >> >> changes.  Any ideas?
> >> >> >> >> 
> >> >> >> 
> raven> I'm aware of this problem.  I'm not sure how to deal with it yet.
> raven> The case above is probably not that difficult to solve but if the
> raven> last component is a directory it's hard to work out it's a problem.
> >> >> >> Ugh.  If you're thinking what I think you're thinking, that's an
> >> ugly >> >> hack.
> >> >> 
> raven> Don't think so.
> >> >>
> raven> I've been seeing this for a while. I wasn't quite sure of the source
> raven> but, for some reason your report has cleared that up.
> >> >>
> raven> The problem is not so much the success returned on the failed mount
> raven> (revalidate). It's the return from the following lookup. This is a
> raven> lookup in a non-root directory. I replaced the non-root lookup with
> raven> the root lookup a while ago and I think this is an unexpected side
> raven> affect of that. Becuase of other changes that lead to that decision
> raven> I think that it should be now be OK to put back the null function
> raven> (always return a negative dentry) that was there before I started
> raven> working on the browable maps feature.
> >> >>
> 
> raven> I've had a look at this a bit more deeply.
> 
> raven> As we know we can't make the path walk lookup fail by
> raven> autofs4_revalidate simply returning 0 and to change that in the
> raven> kernel would be far to dangerous. So we need to deal with this
> raven> during the following lookup. This just means we get an unwanted
> raven> callback to the daemon which will fail and should not cause a
> raven> problem.
> 
> raven> I'm still not fully clear on the reasoning behind the logic in
> raven> try_to_fill_dentry when called with a negative dentry. One of the
> raven> things it attempts to do is cache a lookup failure (ENOENT return
> raven> from the wait). Unfortuneatly the subsequent test in
> raven> autofs4_revalidate is a tautology, always returning true. So
> raven> d_invalidate is never called to cleanup what might be a stale
> raven> dentry. While this is not causing the problem stale dentrys are the
> raven> problem.
> 
> raven> I still haven't decided whether it would be a good idea to return 0
> raven> instead of 1 from try_to_fill_dentry for these failed mount
> raven> attempts. All this would do is give the kernel more chances to clean
> raven> up the stale dentries. The dentry in question won't be released at
> raven> this point as it has a non zero reference count (I believe). But
> raven> sooner or later they will go anyway when d_invalidate is called.
> 
> raven> So to resolve this we need to ignore negative and unhashed dentries
> raven> when checking if directory dentry is empty.
> 
> raven> Please test this patch and let me know how you go.
> 
> OK, I've finally got 'round to testing your patch.  It does fix the test
> case I was using.  My only concern is the potential for regressions.  I'll
> try making sure all of my various maps still work as advertised.

Yep. That's always a danger.

Indeed I had a similar regression in the past. It was quickly obvious there 
was something wrong. oth this change is looking OK so far but more 
testing is definitely needed.

Ian

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/