Hi,
while looking up through *_lookup() sources, I noted there is bunch of code
which is always the same. It's the part after cache_lookup() is called - 
the processing if error != 0.

Originally, I liked the FreeBSD approach, where cache_lookup() call
is generic VOP call and is not called explicitely at all. I discussed
it a bit with mycroft a bit and found out that is not really necessary
to do it that way - it wastes a VOP call with no good reason and even
involves a small performance hit. To remove the code duplication,
it's perfectly enough just to change cache_lookup(). If the vnode is
found in cache, cache_lookup() would do all the necessary locking.
The caller just checks whether the vnode has been found or error occured
and return if anything from those is true.

So my proposal:

Change the type of cache_lookup() to:

int
cache_lookup(dvp, vpp, cnp, found)
        struct vnode *dvp;
        struct vnode **vpp;             
        struct componentname *cnp;
        int *found;

the return value is an error number, if any occurs. *found is
set to 1 if the vnode is found in cache, 0 otherwise.

The code in the fs's lookup routine would then be changed just to

        error = cache_lookup(vdp, vpp, cnp, &found);
        if (error || found)
                return (error);

The rest is handled by cache_lookup() code.

That's basically it. Changes to cache_lookup() are suprisingly trivial,
I'm appending necessary patches. It might be possible
to optimize the added code a bit still, now it's almost plain copy
from what was in msdosfs_lookup.c. I'm also adding the diff to
msdosfs/msdosfs_lookup.c just to highlight the change in fs's code. Doing
the same for other fs's is ~trivial.

I hope it won't change some semantics, but it shouldn't AFAICS.

One remaining issue: our cache code doesn't handle
case-insensitive filesystems quite well - i.e. it won't generate
hit if the name to be found is differs from that cached in case only.
Maybe we should introduce some flags to cache_enter() so that
it would be possible to search case-insensitively optionally.
This is kind of hard doing right (thing "national character sets"),
but even beeing case-insensitive for ASCII chars would be much better
than what we (do not) have now.

The code with the patch below applied might not compile, I didn't try
it actually - just wanted to express the idea.

Best regards

Jaromir Dolecek

--- sys/namei.h.orig	Sat Aug  7 19:26:16 1999
+++ sys/namei.h	Sat Aug  7 19:29:40 1999
@@ -180,7 +180,7 @@
 int	relookup __P((struct vnode *dvp, struct vnode **vpp,
 		      struct componentname *cnp));
 void cache_purge __P((struct vnode *));
-int cache_lookup __P((struct vnode *, struct vnode **, struct componentname *));
+int cache_lookup __P((struct vnode *, struct vnode **, struct componentname *, int *));
 int cache_revlookup __P((struct vnode *, struct vnode **, char **, char *));
 void cache_enter __P((struct vnode *, struct vnode *, struct componentname *));
 void nchinit __P((void));
--- kern/vfs_cache.c.orig	Sat Aug  7 19:28:29 1999
+++ kern/vfs_cache.c	Sat Aug  7 19:27:25 1999
@@ -95,14 +95,17 @@ int doingcache = 1;			/* 1 => enable the
  * is returned. If the lookup fails, a status of zero is returned.
  */
 int
-cache_lookup(dvp, vpp, cnp)
+cache_lookup(dvp, vpp, cnp, found)
 	struct vnode *dvp;
 	struct vnode **vpp;
 	struct componentname *cnp;
+	int *found;
 {
 	register struct namecache *ncp;
 	register struct nchashhead *ncpp;
+	int vpid;
 
+	*found = 0;
 	if (!doingcache) {
 		cnp->cn_flags &= ~MAKEENTRY;
 		return (0);
@@ -156,7 +159,7 @@ cache_lookup(dvp, vpp, cnp)
 			TAILQ_INSERT_TAIL(&nclruhead, ncp, nc_lru);
 		}
 		*vpp = ncp->nc_vp;
-		return (-1);
+		goto found;
 	}
 
 	/*
@@ -172,6 +175,59 @@ cache_lookup(dvp, vpp, cnp)
 		ncp->nc_vhash.le_prev = 0;
 	}
 	TAILQ_INSERT_HEAD(&nclruhead, ncp, nc_lru);
+	return (0);
+
+    found:
+	/*
+	 * take care of necessary locking and special cases, so the caller
+	 * won't need to mess with the returned vnode at all
+	 */
+
+	/*
+	 * Get the next vnode in the path.
+	 * See comment below starting `Step through' for
+	 * an explaination of the locking protocol.
+	 */
+	dvp = *vpp;
+	vpid = dvp->v_id;
+	if (pdp == dvp) {   /* lookup on "." */
+		VREF(dvp);
+		error = 0;
+	} else if (flags & ISDOTDOT) {
+		VOP_UNLOCK(pdp, 0);
+		cnp->cn_flags |= PDIRUNLOCK;
+		error = vget(dvp, LK_EXCLUSIVE);
+		if (!error && lockparent && (flags & ISLASTCN)){
+			error = vn_lock(pdp, LK_EXCLUSIVE);
+			if (error == 0)
+				cnp->cn_flags &= ~PDIRUNLOCK;
+		}
+	} else {
+		error = vget(dvp, LK_EXCLUSIVE);
+		if (!lockparent || error || !(flags & ISLASTCN)) {
+			VOP_UNLOCK(pdp, 0);
+			cnp->cn_flags |= PDIRUNLOCK;
+		}
+	}
+
+	/*
+	 * Check that the capability number did not change
+	 * while we were waiting for the lock.
+	 */
+	if (!error) {
+		if (vpid == dvp->v_id) {
+			*found = 1;
+			return (0);
+		}
+		vput(dvp);
+		if (lockparent && pdp != dvp && (flags & ISLASTCN)) {
+			VOP_UNLOCK(pdp, 0);
+			cnp->cn_flags |= PDIRUNLOCK;
+		}
+	}
+	if ((error = vn_lock(pdp, LK_EXCLUSIVE)) != 0)
+		return (error);
+	cnp->cn_flags &= ~PDIRUNLOCK;
 	return (0);
 }
 
--- msdosfs/msdosfs_lookup.c.orig	Sat Aug  7 19:28:13 1999
+++ msdosfs/msdosfs_lookup.c	Sat Aug  7 19:27:44 1999
@@ -113,6 +113,7 @@ msdosfs_lookup(v)
 	int wincnt = 1;
 	int chksum = -1;
 	int olddos = 1;
+	int found;
 
 	cnp->cn_flags &= ~PDIRUNLOCK;
 	flags = cnp->cn_flags;
@@ -147,64 +148,9 @@ msdosfs_lookup(v)
 	 * check the name cache to see if the directory/name pair
 	 * we are looking for is known already.
 	 */
-	if ((error = cache_lookup(vdp, vpp, cnp)) != 0) {
-		int vpid;
-
-		if (error == ENOENT)
-			return (error);
-		/*
-		 * Get the next vnode in the path.
-		 * See comment below starting `Step through' for
-		 * an explaination of the locking protocol.
-		 */
-		pdp = vdp;
-		dp = VTODE(*vpp);
-		vdp = *vpp;
-		vpid = vdp->v_id;
-		if (pdp == vdp) {   /* lookup on "." */
-			VREF(vdp);
-			error = 0;
-		} else if (flags & ISDOTDOT) {
-			VOP_UNLOCK(pdp, 0);
-			cnp->cn_flags |= PDIRUNLOCK;
-			error = vget(vdp, LK_EXCLUSIVE);
-			if (!error && lockparent && (flags & ISLASTCN)){
-				error = vn_lock(pdp, LK_EXCLUSIVE);
-				if (error == 0)
-					cnp->cn_flags &= ~PDIRUNLOCK;
-			}
-		} else {
-			error = vget(vdp, LK_EXCLUSIVE);
-			if (!lockparent || error || !(flags & ISLASTCN)) {
-				VOP_UNLOCK(pdp, 0);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
-		}
-		/*
-		 * Check that the capability number did not change
-		 * while we were waiting for the lock.
-		 */
-		if (!error) {
-			if (vpid == vdp->v_id) {
-#ifdef MSDOSFS_DEBUG
-				printf("msdosfs_lookup(): cache hit, vnode %p, file %s\n",
-				       vdp, dp->de_Name);
-#endif
-				return (0);
-			}
-			vput(vdp);
-			if (lockparent && pdp != vdp && (flags & ISLASTCN)) {
-				VOP_UNLOCK(pdp, 0);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
-		}
-		if ((error = vn_lock(pdp, LK_EXCLUSIVE)) != 0)
-			return (error);
-		cnp->cn_flags &= ~PDIRUNLOCK;
-		vdp = pdp;
-		dp = VTODE(vdp);
-		*vpp = NULL;
-	}
+	error = cache_lookup(vdp, vpp, cnp, &found);
+	if (error || found)
+		return (error);
 
 	/*
 	 * If they are going after the . or .. entry in the root directory,