Run a revision of the devstat interface:

Kernel:

Change statistics to use the *uptime() timescale (ie: relative to
boottime) rather than the UTC aligned timescale.  This makes the
device statistics code oblivious to clock steps.

Change timestamps to bintime format, they are cheaper.

Remove the "busy_count", and replace it with two counter fields:
"start_count" and "end_count", which are updated in the down and
up paths respectively.  This removes the locking constraint on
devstat.

Add a timestamp argument to devstat_start_transaction(), this will
normally be a timestamp set by the *_bio() function in bp->bio_t0.
Use this field to calculate duration of I/O operations.

Add two timestamp arguments to devstat_end_transaction(), one is
the current time, a NULL pointer means "take timestamp yourself",
the other is the timestamp of when this transaction started (see
above).

Change calculation of busy_time to operate on "the salami principle":
Only when we are idle, which we can determine by the start+end
counts being identical, do we update the "busy_from" field in the
down path.  In the up path we accumulate the timeslice in busy_time
and update busy_from.

Change the byte_* and num_* fields into two arrays: bytes[] and
operations[].

Userland:

Change the misleading "busy_time" name to be called "snap_time" and
make the time long double since that is what most users need anyway,
fill it using clock_gettime(CLOCK_MONOTONIC) to put it on the same
timescale as the kernel fields.

Change devstat_compute_etime() to operate on struct bintime.

Remove the version 2 legacy interface: the change to bintime makes
compatibility far too expensive.

Fix a bug in systat's "vm" page where boot relative busy times would
be bogus.

Bump __FreeBSD_version to 500107

Review & Collaboration by:	ken

3 files changed, 48 insertions, 123 deletions

diff --git a/lib/libdevstat/Makefile b/lib/libdevstat/Makefile
index 5d484bf..3c7454e 100644
--- a/lib/libdevstat/Makefile
+++ b/lib/libdevstat/Makefile
@@ -3,7 +3,8 @@
 MAINTAINER=ken@FreeBSD.ORG
 
 LIB=	devstat
-SHLIB_MAJOR= 3
+# Bump DEVSTAT_USER_API_VER in devstat.h every time this is incremented.
+SHLIB_MAJOR= 4
 SRCS=	devstat.c
 INCS=	devstat.h
 
diff --git a/lib/libdevstat/devstat.c b/lib/libdevstat/devstat.c
index 9c715d8..e5047be 100644
--- a/lib/libdevstat/devstat.c
+++ b/lib/libdevstat/devstat.c
@@ -315,6 +315,7 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 	int retval = 0;
 	struct devinfo *dinfo;
 	const char *func_name = "devstat_getdevs";
+	struct timespec ts;
 
 	dinfo = stats->dinfo;
 
@@ -327,8 +328,8 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 	oldnumdevs = dinfo->numdevs;
 	oldgeneration = dinfo->generation;
 
-	/* Get the current time when we get the stats */
-	gettimeofday(&stats->busy_time, NULL);
+	clock_gettime(CLOCK_MONOTONIC, &ts);
+	stats->snap_time = ts.tv_sec + ts.tv_nsec * 1e-9;
 
 	if (kd == NULL) {
 		/* If this is our first time through, mem_ptr will be null. */
@@ -338,7 +339,7 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 			 * error.  Don't bother setting the error string, since
 			 * getnumdevs() has already done that for us.
 			 */
-			if ((dinfo->numdevs = getnumdevs()) < 0)
+			if ((dinfo->numdevs = devstat_getnumdevs(NULL)) < 0)
 				return(-1);
 			
 			/*
@@ -371,9 +372,9 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 			if (errno == ENOMEM) {
 				/*
 				 * No need to set the error string here, 
-				 * getnumdevs() will do that if it fails.
+				 * devstat_getnumdevs() will do that if it fails.
 				 */
-				if ((dinfo->numdevs = getnumdevs()) < 0)
+				if ((dinfo->numdevs = devstat_getnumdevs(NULL)) < 0)
 					return(-1);
 
 				dssize = (dinfo->numdevs * 
@@ -403,7 +404,7 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 		 * This is of course non-atomic, but since we are working
 		 * on a core dump, the generation is unlikely to change
 		 */
-		if ((dinfo->numdevs = getnumdevs()) == -1)
+		if ((dinfo->numdevs = devstat_getnumdevs(NULL)) == -1)
 			return(-1);
 		if ((dinfo->mem_ptr = get_devstat_kvm(kd)) == NULL)
 			return(-1);
@@ -430,8 +431,8 @@ devstat_getdevs(kvm_t *kd, struct statinfo *stats)
 	 * necessary.
 	 */
 	if (oldgeneration != dinfo->generation) {
-		if (getnumdevs() != dinfo->numdevs) {
-			if ((dinfo->numdevs = getnumdevs()) < 0)
+		if (devstat_getnumdevs(NULL) != dinfo->numdevs) {
+			if ((dinfo->numdevs = devstat_getnumdevs(NULL)) < 0)
 				return(-1);
 			dssize = (dinfo->numdevs * sizeof(struct devstat)) +
 				sizeof(long);
@@ -1142,21 +1143,21 @@ compute_stats(struct devstat *current, struct devstat *previous,
 	       DSM_NONE));
 }
 
+
+/* This is 1/2^64 */
+#define BINTIME_SCALE 5.42101086242752217003726400434970855712890625e-20
+
 long double
-devstat_compute_etime(struct timeval cur_time, struct timeval prev_time)
+devstat_compute_etime(struct bintime *cur_time, struct bintime *prev_time)
 {
-	struct timeval busy_time;
-	u_int64_t busy_usec;
 	long double etime;
 
-	timersub(&cur_time, &prev_time, &busy_time);
-
-        busy_usec = busy_time.tv_sec;  
-        busy_usec *= 1000000;          
-        busy_usec += busy_time.tv_usec;
-        etime = busy_usec;
-        etime /= 1000000;
-
+	etime = cur_time->sec;
+	etime += cur_time->frac * BINTIME_SCALE;
+	if (prev_time != NULL) {
+		etime -= prev_time->sec;
+		etime -= prev_time->frac * BINTIME_SCALE;
+	}
 	return(etime);
 }
 
@@ -1186,21 +1187,21 @@ devstat_compute_statistics(struct devstat *current, struct devstat *previous,
 		return(-1);
 	}
 
-	totalbytesread = current->bytes_read -
-			 ((previous) ? previous->bytes_read : 0);
-	totalbyteswrite = current->bytes_written -
-			    ((previous) ? previous->bytes_written : 0);
+	totalbytesread = current->bytes[DEVSTAT_READ] -
+			 ((previous) ? previous->bytes[DEVSTAT_READ] : 0);
+	totalbyteswrite = current->bytes[DEVSTAT_WRITE] -
+			    ((previous) ? previous->bytes[DEVSTAT_WRITE] : 0);
 
 	totalbytes = totalbytesread + totalbyteswrite;
 
-	totaltransfersread = current->num_reads -
-			     ((previous) ? previous->num_reads : 0);
+	totaltransfersread = current->operations[DEVSTAT_READ] -
+			     ((previous) ? previous->operations[DEVSTAT_READ] : 0);
 
-	totaltransferswrite = current->num_writes -
-			      ((previous) ? previous->num_writes : 0);
+	totaltransferswrite = current->operations[DEVSTAT_WRITE] -
+			      ((previous) ? previous->operations[DEVSTAT_WRITE] : 0);
 
-	totaltransfersother = current->num_other -
-			      ((previous) ? previous->num_other : 0);
+	totaltransfersother = current->operations[DEVSTAT_NO_DATA] -
+			      ((previous) ? previous->operations[DEVSTAT_NO_DATA] : 0);
 
 	totaltransfers = totaltransfersread + totaltransferswrite +
 			 totaltransfersother;
@@ -1496,7 +1497,7 @@ get_devstat_kvm(kvm_t *kd)
 	char *rv = NULL;
 	const char *func_name = "get_devstat_kvm";
 
-	if ((num_devs = getnumdevs()) <= 0)
+	if ((num_devs = devstat_getnumdevs(kd)) <= 0)
 		return(NULL);
 	error = 0;
 	if (KREADNL(kd, X_DEVICE_STATQ, dhead) == -1)
@@ -1510,7 +1511,7 @@ get_devstat_kvm(kvm_t *kd)
 			 func_name);
 		return(NULL);
 	}
-	gen = getgeneration();
+	gen = devstat_getgeneration(kd);
 	memcpy(rv, &gen, sizeof(gen));
 	wp = sizeof(gen);
 	/*
@@ -1536,66 +1537,3 @@ get_devstat_kvm(kvm_t *kd)
 	}
 	return(rv);
 }
-
-/*
- * Compatability functions for libdevstat 2. These are deprecated and may
- * eventually be removed.
- */
-int
-getnumdevs(void)
-{
-	return(devstat_getnumdevs(NULL));
-}
-
-long
-getgeneration(void)
-{
-	return(devstat_getgeneration(NULL));
-}
-
-int
-getversion(void)
-{
-	return(devstat_getversion(NULL));
-}
-
-int
-checkversion(void)
-{
-	return(devstat_checkversion(NULL));
-}
-
-int
-getdevs(struct statinfo *stats)
-{
-	return(devstat_getdevs(NULL, stats));
-}
-
-int
-selectdevs(struct device_selection **dev_select, int *num_selected,
-	   int *num_selections, long *select_generation, 
-	   long current_generation, struct devstat *devices, int numdevs,
-	   struct devstat_match *matches, int num_matches,
-	   char **dev_selections, int num_dev_selections,
-	   devstat_select_mode select_mode, int maxshowdevs,
-	   int perf_select)
-{
-
-	return(devstat_selectdevs(dev_select, num_selected, num_selections,
-	       select_generation, current_generation, devices, numdevs,
-	       matches, num_matches, dev_selections, num_dev_selections,
-	       select_mode, maxshowdevs, perf_select));
-}
-
-int
-buildmatch(char *match_str, struct devstat_match **matches,
-	   int *num_matches)
-{
-	return(devstat_buildmatch(match_str, matches, num_matches));
-}
-
-long double
-compute_etime(struct timeval cur_time, struct timeval prev_time)
-{
-	return(devstat_compute_etime(cur_time, prev_time));
-}
diff --git a/lib/libdevstat/devstat.h b/lib/libdevstat/devstat.h
index c1ae355..09af217 100644
--- a/lib/libdevstat/devstat.h
+++ b/lib/libdevstat/devstat.h
@@ -35,6 +35,16 @@
 
 #include <kvm.h>
 
+/*
+ * Bumped every time we change the userland API.  Hopefully this doesn't
+ * happen very often!  This should be bumped every time we have to
+ * increment SHLIB_MAJOR in the libdevstat Makefile (for non-backwards
+ * compatible API changes) and should also be bumped every time we make
+ * backwards-compatible API changes, so application writers have a way to 
+ * determine when a particular feature is available.
+ */
+#define	DEVSTAT_USER_API_VER	4
+
 #define DEVSTAT_ERRBUF_SIZE  2048 /* size of the devstat library error string */
 
 extern char devstat_errbuf[];
@@ -111,7 +121,7 @@ struct statinfo {
 	long		tk_nin;
 	long		tk_nout;
 	struct devinfo	*dinfo;
-	struct timeval	busy_time;
+	long double 	snap_time;
 };
 
 typedef enum {
@@ -122,31 +132,7 @@ typedef enum {
 } devstat_select_mode;
 
 __BEGIN_DECLS
-/* Legacy interfaces. */
-int getnumdevs(void);
-long getgeneration(void);
-int getversion(void);
-int checkversion(void);
-int getdevs(struct statinfo *stats);
-int selectdevs(struct device_selection **dev_select, int *num_selected,
-	       int *num_selections, long *select_generation, 
-	       long current_generation, struct devstat *devices, int numdevs,
-	       struct devstat_match *matches, int num_matches,
-	       char **dev_selections, int num_dev_selections,
-	       devstat_select_mode select_mode, int maxshowdevs,
-	       int perf_select);
-int buildmatch(char *match_str, struct devstat_match **matches,
-	       int *num_matches);
-int compute_stats(struct devstat *current, struct devstat *previous,
-		  long double etime, u_int64_t *total_bytes,
-		  u_int64_t *total_transfers, u_int64_t *total_blocks,
-		  long double *kb_per_transfer,
-		  long double *transfers_per_second, long double *mb_per_second,
-		  long double *blocks_per_second,
-		  long double *ms_per_transaction);
-long double compute_etime(struct timeval cur_time, struct timeval prev_time);
-
-/* New interfaces. */
+
 int devstat_getnumdevs(kvm_t *kd);
 long devstat_getgeneration(kvm_t *kd);
 int devstat_getversion(kvm_t *kd);
@@ -164,8 +150,8 @@ int devstat_buildmatch(char *match_str, struct devstat_match **matches,
 int devstat_compute_statistics(struct devstat *current,
 			       struct devstat *previous,
 			       long double etime, ...);
-long double devstat_compute_etime(struct timeval cur_time,
-				  struct timeval prev_time);
+long double devstat_compute_etime(struct bintime *cur_time,
+				  struct bintime *prev_time);
 __END_DECLS
 
 #endif /* _DEVSTAT_H  */