From owner-freebsd-fs@FreeBSD.ORG Fri Jan 18 06:19:42 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 039A4B54; Fri, 18 Jan 2013 06:19:42 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail13.syd.optusnet.com.au (mail13.syd.optusnet.com.au [211.29.132.194]) by mx1.freebsd.org (Postfix) with ESMTP id 8715DF66; Fri, 18 Jan 2013 06:19:40 +0000 (UTC) Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail13.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r0I6JTZv001575 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 18 Jan 2013 17:19:30 +1100 Date: Fri, 18 Jan 2013 17:19:29 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Rick Macklem Subject: Re: [PATCH] Use vfs_timestamp() instead of getnanotime() in NFS In-Reply-To: <460209850.2108683.1358475815866.JavaMail.root@erie.cs.uoguelph.ca> Message-ID: <20130118165934.K1042@besplex.bde.org> References: <460209850.2108683.1358475815866.JavaMail.root@erie.cs.uoguelph.ca> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Zty1sKHG c=1 sm=1 a=kdfE0iePi98A:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=9IEQsz3md4oA:10 a=g0RNfcms4CO3QVWdHTYA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 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 06:19:42 -0000 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(). 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().) >> Index: nfsserver/nfs_srvsubs.c >> =================================================================== >> --- nfsserver/nfs_srvsubs.c (revision 245225) >> +++ nfsserver/nfs_srvsubs.c (working copy) >> @@ -1393,7 +1393,7 @@ >> toclient = 1; >> break; >> case NFSV3SATTRTIME_TOSERVER: >> - getnanotime(&(a)->va_atime); >> + vfs_timestamp(&(a)->va_atime); >> a->va_vaflags |= VA_UTIMES_NULL; >> break; >> } >> @@ -1409,7 +1409,7 @@ >> a->va_vaflags &= ~VA_UTIMES_NULL; >> break; >> case NFSV3SATTRTIME_TOSERVER: >> - getnanotime(&(a)->va_mtime); >> + vfs_timestamp(&(a)->va_mtime); >> if (toclient == 0) >> a->va_vaflags |= VA_UTIMES_NULL; >> break; - parenthesizing 'a' is bogus. Bruce