Re: [PATCH 2.4.19pre8][RFC] remove-NFS-close-to-open from VFS (was Re: [PATCHSET] 2.4.19-pre8-jp12)

[Jan Harkes <jaharkes@cs.cmu.edu>]

On Fri, Oct 18, 2002 at 09:23:59PM +0200, Trond Myklebust wrote:
>      > Ok, it will cost Coda a whole upcall/context switch, but as it
>      > is '.'  we're talking about it should already be locally
>      > cached.
> 
> But if all you want to do is read the cache, you'd be better off with
> d_revalidate(dentry,LOOKUP_DOT) / d_revalidate(dentry, LOOKUP_DOTDOT).
> Wouldn't that allow you to drop the upcall+context switch too?

Actually, as long as Coda hasn't received a callback from userspace
indicating that the object has changed, it doesn't even make the getattr
upcall. In fact I would probably end up with coda_dentry_revalidate
calling coda_inode_revalidate in either of these cases. So why not just
use iops->revalidate in the first place.

As far as I can see, d_revalidate should just be used to check whether a
cached directory path might have been changed (i.e. forces a real_lookup).
While iops->revalidate checks the validity of the object.

It also looks like 2.5 doesn't have this patch, so I'm assuming a change
like this will be made to that tree at some point. And I'd rather see a
variant that makes sense and doesn't change existing semantics, even
when it might be less efficient for some filesystems. As long as it
remains definitely correct.

Especially the changed semantics of d_revalidate is what troubles me,

- If I do the test on the object, I might 'reinstantiate' a possibly bad
  lookup path in the dcache because we revalidate the 'path' based on
  the object's state.
- When I test the path, we return ESTALE whenever the path has changed
  due to rename, or on 'volatile objects' (Coda specific, mountpoints,
  objects under repair, etc.).

So neither test ends up being correct. But if we use dentry_revalidate
and inode_revalidate in the correct places it is immediately clear
_what_ we're testing, the path leading up to the object or the object
itself.

Jan