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

[Trond Myklebust <trond.myklebust@fys.uio.no>]

>>>>> " " == Jan Harkes <jaharkes@cs.cmu.edu> writes:


    >> Which is the whole point of the patch. If you are trying to
    >> read or modify a directory that is invalid, you need to be
    >> notified of that.

     > Yes, by failing in real_lookup, not randomly crapping out with
     > ESTALE.

You are completely missing the point: open("."); never did call
'real_lookup', and POSIX compatibility say that it shouldn't (see
example below).

     > Sorry it should have been 'reverted, or fixed to actually
     > revalidate all entries on the cached tree leading up to '.' and
     > '..'.

No.

Client                         Server
------                         --------
cd foo
                               mv foo bar
open(".")

You are basically saying that you believe that the above scenario must
always fail and that the VFS should enforce a violation of POSIX
rules. The current code has the possibility to recover from the above
sort of thing: this will not be the case if you have to look up 'foo'
on the server in order to do open(".")

     > /**
     > * d_invalidate - invalidate a dentry
     > * @dentry: dentry to invalidate
     >  *
     > * Try to invalidate the dentry if it turns out to be
     > * possible. If there are other dentries that can be
     > * reached through this one we can't delete it and we
     > * return -EBUSY. On success we return 0.
     >  *
     > * no dcache lock.
     >  */

     > It's the missing 'd_put' that is the problem. The code should
     > probably look like all other places where d_invalidate is
     > called, i.e.

     >     if (!dentry->d_op->d_revalidate(dentry, flags)) {
     > - d_invalidate(dentry);
     > + if (!d_invalidate(dentry)) {
     > + dput(dentry);
     > + dentry = NULL;
     > + }
     > 	break;
     >     }

So? That's just because those lines usually lie just after a
cached_lookup(), which bumps the count. Read the code in question: it
is not leaking dentries.

Cheers,
  Trond