[OpenAFS-devel] funny lookup/dnlc/dcache/...

[chas williams - CONTRACTOR]

In message <20050422013246.GA21341@tellurium.club.cc.cmu.edu>,"John S. Bucy" writes:
>Bingo ... Chaz' patch fixes it.  On my stat microbenchmark, AFS now is
>just as fast as the local filesystem, as it should be since its all
>dentry cache hits.

it not really a fix but more of a reversion.  at some point the
d_revalidate was rewritten.  the original looked something like
the attached.  afs_VerifyVCache() is used to check the status of the
vcache entry.

the current version of d_revalidate() seems to want to double check the
inode/vcache currently held by the dentry.  this shouldnt be necessary?
if we have a dentry with a non-negative inode reference, it should
always point to a valid inode/vcache.  at one point this might not
have been true due to trouble handling dentries in the afs code.
its been a while, since i looked at this so i could be completely
off the wall.

anyway, d_revalidate could probably be even simpler.  just checking
Cstatd and calling vcache2inode() for the 'good' dentries should be
enough.

Index: src/afs/LINUX/osi_vnodeops.c
===================================================================
RCS file: /cvs/openafs/src/afs/LINUX/osi_vnodeops.c,v
retrieving revision 1.100
diff -u -u -r1.100 osi_vnodeops.c
--- src/afs/LINUX/osi_vnodeops.c	18 Apr 2005 14:28:05 -0000	1.100
+++ src/afs/LINUX/osi_vnodeops.c	22 Apr 2005 01:43:47 -0000
@@ -865,16 +865,11 @@
 afs_linux_dentry_revalidate(struct dentry *dp)
 #endif
 {
-    char *name = NULL;
-    cred_t *credp = crref();
+    cred_t *credp = NULL;
     struct vrequest treq;
-    struct vcache *lookupvcp = NULL;
-    int code, bad_dentry = 1;
-    struct sysname_info sysState;
+    int code, bad_dentry;
     struct vcache *vcp, *parentvcp;
 
-    sysState.allocked = 0;
-
 #ifdef AFS_LINUX24_ENV
     lock_kernel();
 #endif
@@ -885,53 +880,37 @@
 
     /* If it's a negative dentry, then there's nothing to do. */
     if (!vcp || !parentvcp)
-	goto done;
+	goto bad_dentry;
 
     /* If it is the AFS root, then there's no chance it needs 
      * revalidating */
-    if (vcp == afs_globalVp) {
-	bad_dentry = 0;
-	goto done;
-    }
-
-    if ((code = afs_InitReq(&treq, credp)))
-	goto done;
-
-    Check_AtSys(parentvcp, dp->d_name.name, &sysState, &treq);
-    name = sysState.name;
-
-    /* First try looking up the DNLC */
-    if ((lookupvcp = osi_dnlc_lookup(parentvcp, name, WRITE_LOCK))) {
-	/* Verify that the dentry does not point to an old inode */
-	if (vcp != lookupvcp)
-	    goto done;
-	/* Check and correct mvid */
-	if (*name != '/' && vcp->mvstat == 2)
-	    check_bad_parent(dp);
+    if (vcp == afs_globalVp)
+	goto good_dentry;
+
+    /* check parentvcp->CStatd as well? */
+
+    /* Make this a fast path (no crref), since it's called so often. */
+    if (vcp->states & CStatd) {
+        if (*dp->d_name.name != '/' && vcp->mvstat == 2)        /* root vnode */
+            check_bad_parent(dp);       /* check and correct mvid */
 	vcache2inode(vcp);
-	bad_dentry = 0;
-	goto done;
+	goto good_dentry;
     }
 
-    /* A DNLC lookup failure cannot be trusted. Try a real lookup. 
-       Make sure to try the real name and not the @sys expansion; 
-       afs_lookup will expand @sys itself. */
-  
-    code = afs_lookup(parentvcp, dp->d_name.name, &lookupvcp, credp);
-
-    /* Verify that the dentry does not point to an old inode */
-    if (vcp != lookupvcp)
-	goto done;
+    credp = crref();
 
-    bad_dentry = 0;
+    /* get a validated vcache entry */
+    code = afs_InitReq(&treq, credp);
+    if (code)
+	goto bad_dentry;
+    code = afs_VerifyVCache(vcp, &treq);
+    if (code)
+	goto bad_dentry;
 
-  done:
-    /* Clean up */
-    if (lookupvcp)
-	afs_PutVCache(lookupvcp);
-    if (sysState.allocked)
-	osi_FreeLargeSpace(name);
+  good_dentry:
+    bad_dentry = 0;
 
+  out:
     AFS_GUNLOCK();
 
     if (bad_dentry) {
@@ -942,9 +921,15 @@
 #ifdef AFS_LINUX24_ENV
     unlock_kernel();
 #endif
-    crfree(credp);
+
+    if (credp)
+	crfree(credp);
 
     return !bad_dentry;
+
+  bad_dentry:
+    bad_dentry = 1;
+    goto out;
 }
 
 #if !defined(AFS_LINUX26_ENV)