From owner-freebsd-fs@FreeBSD.ORG Wed Oct 18 16:20:23 2006 Return-Path: X-Original-To: fs@freebsd.org Delivered-To: freebsd-fs@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8AF4616A40F for ; Wed, 18 Oct 2006 16:20:23 +0000 (UTC) (envelope-from chucklever@gmail.com) Received: from ug-out-1314.google.com (ug-out-1314.google.com [66.249.92.173]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0228843D55 for ; Wed, 18 Oct 2006 16:20:12 +0000 (GMT) (envelope-from chucklever@gmail.com) Received: by ug-out-1314.google.com with SMTP id m2so189706uge for ; Wed, 18 Oct 2006 09:20:12 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=EtG/n6vuRcapG4MZJ7GklWoR7PACska6J3SG2RY8lGRZd0UzCp/SBXJ+dmHEKxrTF6MQBmb0FiEKiVcQiuVj9xjFAwnLctfFTLFPz+CbtZLhnyaaBApGvcXO9FXWb7Nf35IhfraGxkL1tl/Vdga1qibI5786xFhUBKEtugT3rRY= Received: by 10.82.109.19 with SMTP id h19mr2350949buc; Wed, 18 Oct 2006 09:20:11 -0700 (PDT) Received: by 10.78.202.20 with HTTP; Wed, 18 Oct 2006 09:20:11 -0700 (PDT) Message-ID: <76bd70e30610180920m1918e84s8e1c3b0f02de712e@mail.gmail.com> Date: Wed, 18 Oct 2006 12:20:11 -0400 From: "Chuck Lever" To: "Bruce Evans" In-Reply-To: <20061018153336.E72684@delplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200610140725.k9E7PC37008454@repoman.freebsd.org> <20061015051044.GA42764@xor.obsecurity.org> <20061014222221.H97880@ns1.feral.com> <20061014222437.N4701@ns1.feral.com> <20061015153454.G59979@delplex.bde.org> <76bd70e30610150837w61689cf6ya2499d100a15c3e8@mail.gmail.com> <20061016164122.S63585@delplex.bde.org> <76bd70e30610160620x67e5d3a5j938c26744d0b9759@mail.gmail.com> <20061017113943.C67620@delplex.bde.org> <20061018153336.E72684@delplex.bde.org> Cc: mjacob@freebsd.org, fs@freebsd.org, Kris Kennaway Subject: Re: negative cache hits for nfs (was: cvs commit: ...) X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Oct 2006 16:20:23 -0000 Hi Bruce- On 10/18/06, Bruce Evans wrote: > Here is a merge of some bits from NetBSD for review. It is mostly the > 1997 version, with updates to use timespecs instead of time_t's, but > not updates to use changes that don't seem to be related to correctness, > or ones less than 18 months old (if any). > > % Index: nfs_vnops.c > % =================================================================== > % RCS file: /home/ncvs/src/sys/nfsclient/nfs_vnops.c,v > % retrieving revision 1.270 > % diff -u -2 -r1.270 nfs_vnops.c > % --- nfs_vnops.c 14 Oct 2006 07:25:11 -0000 1.270 > % +++ nfs_vnops.c 18 Oct 2006 01:41:14 -0000 > % @@ -852,7 +869,17 @@ > % return (error); > % } > % - if ((error = cache_lookup(dvp, vpp, cnp)) && error != ENOENT) { > % + if ((error = cache_lookup(dvp, vpp, cnp)) != 0) { > % struct vattr vattr; > % > % + if (error == ENOENT) { > % + /* Negative cache hit. Use it unless stale. */ > % + if (VOP_GETATTR(dvp, &vattr, cnp->cn_cred, td) == 0 && > % + timespeccmp(&vattr.va_mtime, &np->n_nctime, ==)) > % + return (ENOENT); > % + > % + cache_purge(dvp); > % + timespecclear(&np->n_nctime); > % + goto dorpc; > % + } > % newvp = *vpp; > % if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td) > % @@ -871,4 +898,5 @@ > % *vpp = NULLVP; > % } > % +dorpc: > % error = 0; > % newvp = NULLVP; > % @@ -951,4 +979,11 @@ > % nfsmout: > % if (error) { > % + if (error == ENOENT && (cnp->cn_flags & MAKEENTRY) && > % + cnp->cn_nameiop != CREATE) { > % + /* Negative cache entry. */ > % + if (!timespecisset(&np->n_nctime)) > % + np->n_nctime = np->n_vattr.va_mtime; > % + cache_enter(dvp, NULL, cnp); > % + } > % if (newvp != NULLVP) { > % vput(newvp); > % @@ -1931,6 +1966,9 @@ > % if (newvp) > % vput(newvp); > % - } else > % + } else { > % + if (cnp->cn_flags & MAKEENTRY) > % + cache_enter(dvp, newvp, cnp); > % *ap->a_vpp = newvp; > % + } > % return (error); > % } > % Index: nfsnode.h > % =================================================================== > % RCS file: /home/ncvs/src/sys/nfsclient/nfsnode.h,v > % retrieving revision 1.59 > % diff -u -2 -r1.59 nfsnode.h > % --- nfsnode.h 13 Sep 2006 18:39:09 -0000 1.59 > % +++ nfsnode.h 18 Oct 2006 00:48:44 -0000 > % @@ -100,4 +100,5 @@ > % struct timespec n_mtime; /* Prev modify time. */ > % time_t n_ctime; /* Prev create time. */ > % + struct timespec n_nctime; /* Last neg cache entry (dir) */ > % time_t n_expiry; /* Lease expiry time */ > % nfsfh_t *n_fhp; /* NFS File Handle */ This looks reasonable, but there are always tricky corner cases for this kind of change. The reason many of these changes haven't been made sooner is because they usually break something else. Now that you've verified the positive performance impact of this change, you should start by testing this with the Connectathon test suite (both NFSv2 and NFSv3). Another useful test in this case is to observe how the client behaves in the face of replaced file objects. Use another client to remove and recreate a file that your test client already has cached, or in this case create first after your client has already cached the negative lookup. Another interesting test case is to enable READDIRPLUS on NFSv3 mounts to see if there is any strange interaction with your patch. Additionally, I would hold off on putting this in 6.2. Try it in CURRENT for a while, then MFC it after 6.3 branches. Have you reviewed the NetBSD change logs for any fixes in this area since 1997? > Now another pessimization is more obvious -- why should building kernels > be doing all those Fsstat's? I haven't located them for sure, but > that opendir always calls statfs(2) for the silly purpose of determining > whether it needs to do extra work to support unionfs. I've also noticed that behavior. Certainly the FreeBSD client can do a better job of caching the results of Fsstat. -- "We who cut mere stones must always be envisioning cathedrals" -- Quarry worker's creed