svn commit: r265472 - head/bin/dd

[Alan Somers]

On Wed, May 7, 2014 at 2:26 PM, Jilles Tjoelker <jilles at stack.nl> wrote:
> On Wed, May 07, 2014 at 12:10:31PM -0600, Alan Somers wrote:
>> On Tue, May 6, 2014 at 9:47 PM, Bruce Evans <brde at optusnet.com.au> wrote:
>> > On Tue, 6 May 2014, Alan Somers wrote:
>> >
>> >> Log:
>> >>  dd(1) uses gettimeofday(2) to compute the throughput statistics.
>> >> However,
>> >>  gettimeofday returns the system clock, which may jump forward or back,
>> >>  especially if NTP is in use.  If the time jumps backwards, then dd will
>> >> see
>> >>  negative elapsed time, round it up to 1usec, and print an absurdly fast
>> >>  transfer rate.
>> >>
>> >>  The solution is to use clock_gettime(2) with CLOCK_MONOTONIC_PRECISE as
>> >> the
>> >>  clock_id.  That clock advances steadily, regardless of changes to the
>> >> system
>> >>  clock.
>> >>
>> >>  Reviewed by:   delphij
>> >>  MFC after:     3 days
>> >>  Sponsored by:  Spectra Logic
>> >> ...
>> >>
>> >> Modified: head/bin/dd/dd.c
>> >>
>> >> ==============================================================================
>> >> --- head/bin/dd/dd.c    Tue May  6 22:04:50 2014        (r265471)
>> >> +++ head/bin/dd/dd.c    Tue May  6 22:06:39 2014        (r265472)
>> >> @@ -50,7 +50,6 @@ __FBSDID("$FreeBSD$");
>> >> #include <sys/conf.h>
>> >> #include <sys/disklabel.h>
>> >> #include <sys/filio.h>
>> >> -#include <sys/time.h>
>> >>
>> >> #include <ctype.h>
>> >> #include <err.h>
>> >> @@ -61,6 +60,8 @@ __FBSDID("$FreeBSD$");
>> >> #include <stdio.h>
>> >> #include <stdlib.h>
>> >> #include <string.h>
>> >> +#include <sysexits.h>
>
>> > Use of <sysexits.h> is a style bug.  It is not used in BSD or KNF code
>> > like dd used to be.
>
>> sysexits.h is recommended by the err(3) man page.  Is that
>> recommendation meant to apply selectively, or is it obsolete, or is
>> some sort of edit war being waged by man page authors?
>
> The recommendation for <sysexits.h> was incompletely removed, yes.
>
>> [snip]
>> >> -       st.start = tv.tv_sec + tv.tv_usec * 1e-6;
>> >> +       if (clock_gettime(CLOCK_MONOTONIC_PRECISE, &tv))
>> >> +               err(EX_OSERR, "clock_gettime");
>> [snip]
>> >> +       st.start = tv.tv_sec + tv.tv_nsec * 1.0e-9;
>> >> }
>
> The floating point addition starts losing precision after 8388608
> seconds (slightly more than 97 days, a plausible uptime for a server).
> It is better to subtract the timespecs to avoid this issue.
>
> With microseconds, the precision of a double is sufficient for 272
> years, so that calculation is probably acceptable.

Good point.  I'll fix it.  BTW, FreeBSD does not have a timespecsub,
but I notice that NetBSD does.  This seems like a good candidate to
import.

http://netbsd.gw.com/cgi-bin/man-cgi?timespecsub++NetBSD-current

>
> I think I agree with Bruce that using CLOCK_MONOTONIC_PRECISE is not
> necessary here; the standard CLOCK_MONOTONIC should suffice.
>
>> [snip]
>> Even if nanosecond resolution isn't useful, monotonicity is.  Nobody
>> should be using a nonmonotonic clock just to measure durations.  I
>> started an audit of all of FreeBSD to look for other programs that use
>> gettimeofday to measure durations.  I haven't finished, but I've
>> already found a lot, including xz, ping, hastd, fetch, systat, powerd,
>> and others.  I don't have time to fix them, though.  Would you be
>> interested, or do you know anyone else who would?
>
> I have a local patch for time(1).
>
> Whether the monotonic clock is right also depends on how long the
> durations typically are. For very long durations, users might refer to
> wall clocks and CLOCK_REALTIME may be more appropriate.

I would say that gettimeofday() or CLOCK_REALTIME() should be used
whenever the absolute value of the result matters.  For example, if
it's written to disk, displayed to the user, or used as an absolute
wakeup time for something like at(1).  But if only the relative value
of 2 or more calls matter, then I think that a monotonic clock should
always be used.  A corollary is that durations that cross a reboot
cycle, or when the start and end times are measured by different
machines, must use the realtime clock.

-Alan

>
> --
> Jilles Tjoelker