From owner-freebsd-fs@FreeBSD.ORG Fri Jan 18 17:13:18 2013 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 7A0205A2; Fri, 18 Jan 2013 17:13:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 500B388F; Fri, 18 Jan 2013 17:13:18 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id C3F63B993; Fri, 18 Jan 2013 12:13:17 -0500 (EST) From: John Baldwin To: Bruce Evans Subject: Re: [PATCH] Use vfs_timestamp() instead of getnanotime() in NFS Date: Fri, 18 Jan 2013 12:12:41 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64; ; ) References: <460209850.2108683.1358475815866.JavaMail.root@erie.cs.uoguelph.ca> <20130118165934.K1042@besplex.bde.org> In-Reply-To: <20130118165934.K1042@besplex.bde.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201301181212.41321.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 18 Jan 2013 12:13:17 -0500 (EST) Cc: freebsd-fs@freebsd.org, Rick Macklem X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jan 2013 17:13:18 -0000 On Friday, January 18, 2013 1:19:29 am Bruce Evans wrote: > On Thu, 17 Jan 2013, Rick Macklem wrote: > > > John Baldwin wrote: > >> On Tuesday, January 15, 2013 2:58:42 pm John Baldwin wrote: > >>> Fixing NFS to properly use vfs_timestamp() seems to be a larger > >>> project. > >> > >> Actually, I have a patch that I think does this below. It builds, have > >> not > >> yet booted it (but will do so in a bit). > >> > >> Index: fs/nfsclient/nfs_clstate.c > >> =================================================================== > >> --- fs/nfsclient/nfs_clstate.c (revision 245225) > >> +++ fs/nfsclient/nfs_clstate.c (working copy) > >> @@ -4611,7 +4611,7 @@ > >> } > >> dp = nfscl_finddeleg(clp, np->n_fhp->nfh_fh, np->n_fhp->nfh_len); > >> if (dp != NULL && (dp->nfsdl_flags & NFSCLDL_WRITE)) { > >> - NFSGETNANOTIME(&dp->nfsdl_modtime); > >> + vfs_timestamp(&dp->nfsdl_modtime); > >> dp->nfsdl_flags |= NFSCLDL_MODTIMESET; > >> } > >> NFSUNLOCKCLSTATE(); > > Not sure about this case. Although nfsdl_modtime is being set for local > > use, it replaces the mtime returned by the NFS server while the delegation > > is in use. Ideally it would be the same resolution as the NFS server, but > > that resolution isn't known to the client. (It is often better than 1sec, > > which is the default for vfs_timestamp().) > > The patch seems about right except for this. > > > I'd be tempted to leave it (although the function used by the macro might > > need to be changed, since Bruce mentions getnanotime() isn't supposed to > > be used?). > > For maximal precision and accuracy, it nanotime() should be used. I'm > not sure if you need to be at least as precise and accurate as the server. > Having them synced to nanoseconds accuracy is impossible, but > getnanotime() gives <= 1/HZ of accuracy and it is easy for them to be > synced with more accuracy than that. Then the extra accuracy can be > seen in server timestamps if the server is FreeBSD and uses vfs_timestamp() > with a either microtime() or nanotime(). I've certainly seen NFS servers use much more finely-grained VFS timestamps (e.g. Isilon nodes run with vfs.timestamp_precision of 2 or 3 so they give more precise timestamps than just getnanotime()). OTOH, clock drift between the client and server could easily screw this up. I will leave this as-is for now and just commit the vfs_timestamp() changes first. > Further style fixes: > - remove the NFSGETNANOTIME() macro. It is only used in the above, and > in 3 other instances where its use is bogus because only the seconds > part is used. The `time_second' global gives seconds part with the > same (in)accuracy as getnanotime(). If you want maximal accuracy > for just the seconds part, then bintime() should be used (this is > slightly faster than microtime() and nanotime(). > (get*time()'s seconds part is the same as time_second. This > inaccurate since it lags bintime()'s seconds part by up to 1/HZ > seconds (so it differs by a full second for an everage of 1 one > in every HZ readings). The difference is visible if one reader, > say make(1) reads the time using bintime() while another reader, > say vfs_timestamp() reads the time using getbintime().) Yes, I wondered if I could replace those with time_second. The same is true of the remaining uses of NFSGETTIME() as they also only use the seconds portion. -- John Baldwin