Re: clock_gettime: add CLOCK_BOOTTIME clockid

2017-12-14 Thread Mike Larkin 
On Thu, Dec 14, 2017 at 06:47:02PM -0600, Scott Cheloha wrote:
> Hi,
> 
> Attached is a diff adding a new clockid, CLOCK_BOOTTIME, for use with
> clock_gettime(2).  The value of CLOCK_BOOTTIME is the time elapsed
> since the system booted, i.e. the system uptime.  The diff puts it to
> use in top(1), w(1), and snmpd(8).
> 
> CLOCK_BOOTTIME originated in Linux in the 2.6.39 kernel [1, 2].
> 
> The base motivation for adding CLOCK_BOOTTIME to OpenBSD is that there
> is currently no direct interface for userland to query the system
> uptime, i.e., the time elapsed since the machine was last (re)booted.
> 
> We have several callers using the kern.boottime sysctl, but all of
> them immediately subtract the result from the current time of day to
> derive the system uptime.  This is is (a) cumbersome, and (b) involves
> a race with settimeofday(2), etc.
> 
> We could add a new sysctl, but sysctl(3) itself is cumbersome,
> especially when compared to clock_gettime(2).
> 
> Although CLOCK_MONOTONIC is currently implemented with nanouptime(9)
> and is thus an interface to the system uptime, this is only coincidentally
> so.  The standard notes that the value returned by CLOCK_MONOTONIC is
> meaningless in isolation: it is only portably useful for interval
> measurement.  Overloading CLOCK_MONOTONIC as an interface to the system
> uptime would confuse the purpose of the clock, promote the creation of non-
> portable code, and prevent us from changing the clock's implementation
> in the future.
> 
> On top of enriching base with a nice interface to the system uptime,
> this diff also makes CLOCK_BOOTTIME available to ports with the same
> semantics it has in Linux.  CLOCK_BOOTTIME has been available in Linux
> for over five years, so there are almost certainly ports that can make
> use of it.
> 
> The diff in its current form is the result of discussion and revision
> with guenther@, jca@, and tb@.  All gave their ok.
> 
> Thoughts and feedback?
> 


No objections, you can add me to the ok list...

-ml

> --
> Scott Cheloha
> 
> [1] https://marc.info/?l=linux-kernel=129783004314557=2
> [2] 
> https://github.com/torvalds/linux/commit/420c1c572d4ceaa2f37b6311b7017ac6cf049fe2
> 
> Index: lib/libc/sys/clock_gettime.2
> ===
> RCS file: /cvs/src/lib/libc/sys/clock_gettime.2,v
> retrieving revision 1.27
> diff -u -p -r1.27 clock_gettime.2
> --- lib/libc/sys/clock_gettime.2  10 Sep 2015 17:55:21 -  1.27
> +++ lib/libc/sys/clock_gettime.2  15 Dec 2017 00:43:29 -
> @@ -73,6 +73,9 @@ time that increments as a wall clock sho
>  is meaningless and cannot jump,
>  providing accurate realtime interval measurement,
>  even across suspend and resume
> +.It Dv CLOCK_BOOTTIME
> +time whose absolute value is the time that has elapsed since the
> +system was booted
>  .It Dv CLOCK_UPTIME
>  time whose absolute value is the time the system has been running
>  and not suspended,
> @@ -157,8 +160,10 @@ functions conform to
>  .St -p1003.1-2008 .
>  .Pp
>  The
> +.Dv CLOCK_BOOTTIME
> +and
>  .Dv CLOCK_UPTIME
> -clock is an extension to that.
> +clocks are extensions to that.
>  .Sh HISTORY
>  The
>  .Dv CLOCK_PROCESS_CPUTIME_ID
> @@ -174,3 +179,11 @@ and was added to
>  .Ox
>  in
>  .Ox 5.5 .
> +The
> +.Dv CLOCK_BOOTTIME
> +clock first appeared in
> +Linux 2.6.39
> +and was added to
> +.Ox
> +in
> +.Ox 6.3 .
> Index: sys/kern/kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 kern_time.c
> --- sys/kern/kern_time.c  24 Jan 2017 00:58:55 -  1.99
> +++ sys/kern/kern_time.c  15 Dec 2017 00:43:29 -
> @@ -124,6 +124,7 @@ clock_gettime(struct proc *p, clockid_t 
>   bintime2timespec(, tp);
>   break;
>   case CLOCK_MONOTONIC:
> + case CLOCK_BOOTTIME:
>   nanouptime(tp);
>   break;
>   case CLOCK_PROCESS_CPUTIME_ID:
> @@ -223,6 +224,7 @@ sys_clock_getres(struct proc *p, void *v
>   switch (clock_id) {
>   case CLOCK_REALTIME:
>   case CLOCK_MONOTONIC:
> + case CLOCK_BOOTTIME:
>   case CLOCK_UPTIME:
>   case CLOCK_PROCESS_CPUTIME_ID:
>   case CLOCK_THREAD_CPUTIME_ID:
> Index: sys/sys/_time.h
> ===
> RCS file: /cvs/src/sys/sys/_time.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 _time.h
> --- sys/sys/_time.h   3 Sep 2016 15:06:06 -   1.8
> +++ sys/sys/_time.h   15 Dec 2017 00:43:29 -
> @@ -37,6 +37,7 @@
>  #define CLOCK_MONOTONIC  3
>  #define CLOCK_THREAD_CPUTIME_ID  4
>  #define CLOCK_UPTIME 5
> +#define CLOCK_BOOTTIME   6
>  
>  #if __BSD_VISIBLE
>  /*
> Index: usr.bin/w/w.c
> ===
> RCS file: /cvs/src/usr.bin/w/w.c,v
> retrieving

Re: clock_gettime: add CLOCK_BOOTTIME clockid

2017-12-14 Thread Philip Guenther 
On Thu, Dec 14, 2017 at 6:33 PM, David Gwynne  wrote:
...

> however, it implies that our CLOCK_MONOTONIC behaviour needs to change.
> right now it represents wall time since boot, but it should be time after
> boot minus how long it has been suspended?
>

Nope.  To quote clock_gettime(3)
 CLOCK_UPTIME time whose absolute value is the time the system has
  been running and not suspended, providing accurate
  uptime measurement, both absolute and interval


Philip Guenther

Re: clock_gettime: add CLOCK_BOOTTIME clockid

2017-12-14 Thread David Gwynne 

> On 15 Dec 2017, at 10:47, Scott Cheloha  wrote:
> 
> Hi,
> 
> Attached is a diff adding a new clockid, CLOCK_BOOTTIME, for use with
> clock_gettime(2).  The value of CLOCK_BOOTTIME is the time elapsed
> since the system booted, i.e. the system uptime.  The diff puts it to
> use in top(1), w(1), and snmpd(8).
> 
> CLOCK_BOOTTIME originated in Linux in the 2.6.39 kernel [1, 2].
> 
> The base motivation for adding CLOCK_BOOTTIME to OpenBSD is that there
> is currently no direct interface for userland to query the system
> uptime, i.e., the time elapsed since the machine was last (re)booted.
> 
> We have several callers using the kern.boottime sysctl, but all of
> them immediately subtract the result from the current time of day to
> derive the system uptime.  This is is (a) cumbersome, and (b) involves
> a race with settimeofday(2), etc.
> 
> We could add a new sysctl, but sysctl(3) itself is cumbersome,
> especially when compared to clock_gettime(2).
> 
> Although CLOCK_MONOTONIC is currently implemented with nanouptime(9)
> and is thus an interface to the system uptime, this is only coincidentally
> so.  The standard notes that the value returned by CLOCK_MONOTONIC is
> meaningless in isolation: it is only portably useful for interval
> measurement.  Overloading CLOCK_MONOTONIC as an interface to the system
> uptime would confuse the purpose of the clock, promote the creation of non-
> portable code, and prevent us from changing the clock's implementation
> in the future.
> 
> On top of enriching base with a nice interface to the system uptime,
> this diff also makes CLOCK_BOOTTIME available to ports with the same
> semantics it has in Linux.  CLOCK_BOOTTIME has been available in Linux
> for over five years, so there are almost certainly ports that can make
> use of it.
> 
> The diff in its current form is the result of discussion and revision
> with guenther@, jca@, and tb@.  All gave their ok.
> 
> Thoughts and feedback?

i think we should do this, and this is the right way to start.

however, it implies that our CLOCK_MONOTONIC behaviour needs to change. right 
now it represents wall time since boot, but it should be time after boot minus 
how long it has been suspended?

also, i've always wanted to add a random offset on boot to CLOCK_MONOTONIC. 
having CLOCK_BOOTTIME helps justify doing things like that to MONOTONIC.

it has my ok, but you need some more people to say so too.

dlg

> 
> --
> Scott Cheloha
> 
> [1] https://marc.info/?l=linux-kernel=129783004314557=2
> [2] 
> https://github.com/torvalds/linux/commit/420c1c572d4ceaa2f37b6311b7017ac6cf049fe2
> 
> Index: lib/libc/sys/clock_gettime.2
> ===
> RCS file: /cvs/src/lib/libc/sys/clock_gettime.2,v
> retrieving revision 1.27
> diff -u -p -r1.27 clock_gettime.2
> --- lib/libc/sys/clock_gettime.2  10 Sep 2015 17:55:21 -  1.27
> +++ lib/libc/sys/clock_gettime.2  15 Dec 2017 00:43:29 -
> @@ -73,6 +73,9 @@ time that increments as a wall clock sho
> is meaningless and cannot jump,
> providing accurate realtime interval measurement,
> even across suspend and resume
> +.It Dv CLOCK_BOOTTIME
> +time whose absolute value is the time that has elapsed since the
> +system was booted
> .It Dv CLOCK_UPTIME
> time whose absolute value is the time the system has been running
> and not suspended,
> @@ -157,8 +160,10 @@ functions conform to
> .St -p1003.1-2008 .
> .Pp
> The
> +.Dv CLOCK_BOOTTIME
> +and
> .Dv CLOCK_UPTIME
> -clock is an extension to that.
> +clocks are extensions to that.
> .Sh HISTORY
> The
> .Dv CLOCK_PROCESS_CPUTIME_ID
> @@ -174,3 +179,11 @@ and was added to
> .Ox
> in
> .Ox 5.5 .
> +The
> +.Dv CLOCK_BOOTTIME
> +clock first appeared in
> +Linux 2.6.39
> +and was added to
> +.Ox
> +in
> +.Ox 6.3 .
> Index: sys/kern/kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 kern_time.c
> --- sys/kern/kern_time.c  24 Jan 2017 00:58:55 -  1.99
> +++ sys/kern/kern_time.c  15 Dec 2017 00:43:29 -
> @@ -124,6 +124,7 @@ clock_gettime(struct proc *p, clockid_t 
>   bintime2timespec(, tp);
>   break;
>   case CLOCK_MONOTONIC:
> + case CLOCK_BOOTTIME:
>   nanouptime(tp);
>   break;
>   case CLOCK_PROCESS_CPUTIME_ID:
> @@ -223,6 +224,7 @@ sys_clock_getres(struct proc *p, void *v
>   switch (clock_id) {
>   case CLOCK_REALTIME:
>   case CLOCK_MONOTONIC:
> + case CLOCK_BOOTTIME:
>   case CLOCK_UPTIME:
>   case CLOCK_PROCESS_CPUTIME_ID:
>   case CLOCK_THREAD_CPUTIME_ID:
> Index: sys/sys/_time.h
> ===
> RCS file: /cvs/src/sys/sys/_time.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 _time.h
> --- sys/sys/_time.h   3 Sep 2016 15:06:06 -   1.8
> +++ sys/sys/_time.h

clock_gettime: add CLOCK_BOOTTIME clockid

2017-12-14 Thread Scott Cheloha 
Hi,

Attached is a diff adding a new clockid, CLOCK_BOOTTIME, for use with
clock_gettime(2).  The value of CLOCK_BOOTTIME is the time elapsed
since the system booted, i.e. the system uptime.  The diff puts it to
use in top(1), w(1), and snmpd(8).

CLOCK_BOOTTIME originated in Linux in the 2.6.39 kernel [1, 2].

The base motivation for adding CLOCK_BOOTTIME to OpenBSD is that there
is currently no direct interface for userland to query the system
uptime, i.e., the time elapsed since the machine was last (re)booted.

We have several callers using the kern.boottime sysctl, but all of
them immediately subtract the result from the current time of day to
derive the system uptime.  This is is (a) cumbersome, and (b) involves
a race with settimeofday(2), etc.

We could add a new sysctl, but sysctl(3) itself is cumbersome,
especially when compared to clock_gettime(2).

Although CLOCK_MONOTONIC is currently implemented with nanouptime(9)
and is thus an interface to the system uptime, this is only coincidentally
so.  The standard notes that the value returned by CLOCK_MONOTONIC is
meaningless in isolation: it is only portably useful for interval
measurement.  Overloading CLOCK_MONOTONIC as an interface to the system
uptime would confuse the purpose of the clock, promote the creation of non-
portable code, and prevent us from changing the clock's implementation
in the future.

On top of enriching base with a nice interface to the system uptime,
this diff also makes CLOCK_BOOTTIME available to ports with the same
semantics it has in Linux.  CLOCK_BOOTTIME has been available in Linux
for over five years, so there are almost certainly ports that can make
use of it.

The diff in its current form is the result of discussion and revision
with guenther@, jca@, and tb@.  All gave their ok.

Thoughts and feedback?

--
Scott Cheloha

[1] https://marc.info/?l=linux-kernel=129783004314557=2
[2] 
https://github.com/torvalds/linux/commit/420c1c572d4ceaa2f37b6311b7017ac6cf049fe2

Index: lib/libc/sys/clock_gettime.2
===
RCS file: /cvs/src/lib/libc/sys/clock_gettime.2,v
retrieving revision 1.27
diff -u -p -r1.27 clock_gettime.2
--- lib/libc/sys/clock_gettime.210 Sep 2015 17:55:21 -  1.27
+++ lib/libc/sys/clock_gettime.215 Dec 2017 00:43:29 -
@@ -73,6 +73,9 @@ time that increments as a wall clock sho
 is meaningless and cannot jump,
 providing accurate realtime interval measurement,
 even across suspend and resume
+.It Dv CLOCK_BOOTTIME
+time whose absolute value is the time that has elapsed since the
+system was booted
 .It Dv CLOCK_UPTIME
 time whose absolute value is the time the system has been running
 and not suspended,
@@ -157,8 +160,10 @@ functions conform to
 .St -p1003.1-2008 .
 .Pp
 The
+.Dv CLOCK_BOOTTIME
+and
 .Dv CLOCK_UPTIME
-clock is an extension to that.
+clocks are extensions to that.
 .Sh HISTORY
 The
 .Dv CLOCK_PROCESS_CPUTIME_ID
@@ -174,3 +179,11 @@ and was added to
 .Ox
 in
 .Ox 5.5 .
+The
+.Dv CLOCK_BOOTTIME
+clock first appeared in
+Linux 2.6.39
+and was added to
+.Ox
+in
+.Ox 6.3 .
Index: sys/kern/kern_time.c
===
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.99
diff -u -p -r1.99 kern_time.c
--- sys/kern/kern_time.c24 Jan 2017 00:58:55 -  1.99
+++ sys/kern/kern_time.c15 Dec 2017 00:43:29 -
@@ -124,6 +124,7 @@ clock_gettime(struct proc *p, clockid_t 
bintime2timespec(, tp);
break;
case CLOCK_MONOTONIC:
+   case CLOCK_BOOTTIME:
nanouptime(tp);
break;
case CLOCK_PROCESS_CPUTIME_ID:
@@ -223,6 +224,7 @@ sys_clock_getres(struct proc *p, void *v
switch (clock_id) {
case CLOCK_REALTIME:
case CLOCK_MONOTONIC:
+   case CLOCK_BOOTTIME:
case CLOCK_UPTIME:
case CLOCK_PROCESS_CPUTIME_ID:
case CLOCK_THREAD_CPUTIME_ID:
Index: sys/sys/_time.h
===
RCS file: /cvs/src/sys/sys/_time.h,v
retrieving revision 1.8
diff -u -p -r1.8 _time.h
--- sys/sys/_time.h 3 Sep 2016 15:06:06 -   1.8
+++ sys/sys/_time.h 15 Dec 2017 00:43:29 -
@@ -37,6 +37,7 @@
 #define CLOCK_MONOTONIC3
 #define CLOCK_THREAD_CPUTIME_ID4
 #define CLOCK_UPTIME   5
+#define CLOCK_BOOTTIME 6
 
 #if __BSD_VISIBLE
 /*
Index: usr.bin/w/w.c
===
RCS file: /cvs/src/usr.bin/w/w.c,v
retrieving revision 1.64
diff -u -p -r1.64 w.c
--- usr.bin/w/w.c   14 Dec 2017 18:03:03 -  1.64
+++ usr.bin/w/w.c   15 Dec 2017 00:43:29 -
@@ -66,7 +66,6 @@
 
 #include "extern.h"
 
-struct timeval boottime;
 struct utmputmp;
 struct winsize ws;
 kvm_t *kd;
@@ -426,10 +425,9 @@ static void
 pr_header(time_t *nowp, int