Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Jul 1999 20:31:38 -0700 (PDT)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        Bill Paul <wpaul@skynet.ctr.columbia.edu>
Cc:        peter@netplex.com.au, crossd@cs.rpi.edu, current@freebsd.org
Subject:   Re: readdirplus client side fix (was Re: IRIX 6.5.4 NFS v3 TCP client + FreeBSD server = bewm)
Message-ID:  <199907300331.UAA81350@apollo.backplane.com>
References:   <199907300316.XAA17754@skynet.ctr.columbia.edu>

next in thread | previous in thread | raw e-mail | index | archive | help
:Crap, I just sent out an incomplete message. Let me pick up from where
:I left off. Here's a diff that shows the changes I made to nfs_vfsops.c:
:
:*** nfs_vnops.c.orig	Thu Jul 29 22:46:28 1999
:--- nfs_vnops.c	Thu Jul 29 22:36:39 1999
:***************
:*** 2342,2348 ****
:  				    IFTODT(VTTOIF(np->n_vattr.va_type));
:  				ndp->ni_vp = newvp;
:  				cnp->cn_hash = 0;
:! 				for (cp = cnp->cn_nameptr, i = 1; i <= len;
:  				    i++, cp++)
:  				    cnp->cn_hash += (unsigned char)*cp * i;
:  			        cache_enter(ndp->ni_dvp, ndp->ni_vp, cnp);
:--- 2342,2351 ----
:  				    IFTODT(VTTOIF(np->n_vattr.va_type));
:  				ndp->ni_vp = newvp;
:  				cnp->cn_hash = 0;
:! 				/*for (cp = cnp->cn_nameptr, i = 1; i <= len;*/
:! 				if (len != cnp->cn_namelen)
:! 					printf("bogus: %d %d\n", len, cnp->cn_namelen);
:! 				for (cp = cnp->cn_nameptr, i = 1; i <= cnp->cn_namelen;
:  				    i++, cp++)
:  				    cnp->cn_hash += (unsigned char)*cp * i;
:  			        cache_enter(ndp->ni_dvp, ndp->ni_vp, cnp);
:
:Basically, at some point, the code tries to calculate a new hash value
:(what for I don't know) of a name that was read from the directory
:listing. However it uses "len" as the length of the name, which for
:some reason I can't understand turns out not matching the cn_namelen
:value in cnp. The "bogus" printf shows about a half dozen occasions
:where len and cn_namelen don't agree. Sometimes "len" is larger
:than "cnp->cn_namelen," sometimes it's smaller. By using cn_namelen
:instead of len, everything seems to work correctly. It looks like
:this loop makes more than one pass over directory entries, so it could
:be that len is sometimes stale. If you can make sense of why this
:happens, I would appreciate it: I don't like to commit changes when
:I don't fully understand what's going on.

    Look up a bit in the code.  If bigenough is not true, cnp does not 
    get initialized.  This could lead to the bogus length -- or rather,
    it would be the cnp that is bogus, not the 'len'.

    The question is how to fix it.  I think we can safely avoid doing the
    cache_enter so try changing the 'if (doit)' to 'if (doit && bigenough)'.
    I've included the patch below.

    I am not 100% sure about this.

:>     Presumably this will not fix the SGI client.  I've no idea what the
:>     problem there is.  There may be a bug in the SGI client or there may
:>     be a bug in the client & server implementation of the protocol in FreeBSD.
:
:Er, I think you misunderstood: there's nothing wrong with the SGI in
:this case. I mentioned it because I was using it as a _server_ when
:testing the client side readdirplus support: the behavior with the
:...
:was just a consequence of the filesystems being laid out differently.
:The patched FreeBSD client works fine now with the SGI server.
:
:-Bill

   Ah, ok.  I mistook the SGI for being a client.

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

Index: nfs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.135
diff -u -r1.135 nfs_vnops.c
--- nfs_vnops.c	1999/07/01 13:32:54	1.135
+++ nfs_vnops.c	1999/07/30 03:28:36
@@ -2343,7 +2343,7 @@
 					newvp = NFSTOV(np);
 				}
 			    }
-			    if (doit) {
+			    if (doit && bigenough) {
 				dpossav2 = dpos;
 				dpos = dpossav1;
 				mdsav2 = md;
@@ -2367,7 +2367,10 @@
 			    nfsm_adv(nfsm_rndup(i));
 			}
 			if (newvp != NULLVP) {
-			    vrele(newvp);
+			    if (newvp == vp)
+				vrele(newvp);
+			    else
+				vput(newvp);
 			    newvp = NULLVP;
 			}
 			nfsm_dissect(tl, u_int32_t *, NFSX_UNSIGNED);


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199907300331.UAA81350>