[PATCH] Use vfs_timestamp() instead of getnanotime() in NFS

[John Baldwin]

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