svn commit: r322862 - head/sys/cam

[Warner Losh]

Author: imp
Date: Thu Aug 24 22:10:58 2017
New Revision: 322862
URL: https://svnweb.freebsd.org/changeset/base/322862

Log:
  Fix 32-bit overflow on latency measurements
  
  o Allow I/O scheduler to gather times on 32-bit systems. We do this by shifting
    the sbintime_t over by 8 bits and truncating to 32-bits. This gives us 8.24
    time. This is sufficient both in range (256 seconds is about 128x what current
    users need) and precision (60ns easily meets the 1ms smallest bucket size
    measurements). 64-bit systems are unchanged. Centralize all the time math so
    it's easy to tweak tha range / precision tradeoffs in the future.
  o While I'm here, the I/O scheduler should be using periph_data rather than
    sim_data since it is operating on behalf of the periph.
  
  Differential Review: https://reviews.freebsd.org/D12119

Modified:
  head/sys/cam/cam_iosched.c
  head/sys/cam/cam_iosched.h
  head/sys/cam/cam_xpt.c

Modified: head/sys/cam/cam_iosched.c
==============================================================================
--- head/sys/cam/cam_iosched.c	Thu Aug 24 21:49:44 2017	(r322861)
+++ head/sys/cam/cam_iosched.c	Thu Aug 24 22:10:58 2017	(r322862)
@@ -1428,7 +1428,8 @@ cam_iosched_bio_complete(struct cam_iosched_softc *isc
 	}
 
 	if (!(bp->bio_flags & BIO_ERROR))
-		cam_iosched_io_metric_update(isc, done_ccb->ccb_h.qos.sim_data,
+		cam_iosched_io_metric_update(isc,
+		    cam_iosched_sbintime_t(done_ccb->ccb_h.qos.periph_data),
 		    bp->bio_cmd, bp->bio_bcount);
 #endif
 	return retval;

Modified: head/sys/cam/cam_iosched.h
==============================================================================
--- head/sys/cam/cam_iosched.h	Thu Aug 24 21:49:44 2017	(r322861)
+++ head/sys/cam/cam_iosched.h	Thu Aug 24 22:10:58 2017	(r322862)
@@ -41,6 +41,44 @@ struct sysctl_oid;
 union ccb;
 struct bio;
 
+/*
+ * For 64-bit platforms, we know that uintptr_t is the same size as sbintime_t
+ * so we can store values in it. For 32-bit systems, however, uintptr_t is only
+ * 32-bits, so it won't fit. For those systems, store 24 bits of fraction and 8
+ * bits of seconds. This allows us to measure an interval of up to ~256s, which
+ * is ~200x what our current uses require. Provide some convenience functions to
+ * get the time, subtract two times and convert back to sbintime_t in a safe way
+ * that can be centralized.
+ */
+#ifdef __LP64__
+#define CAM_IOSCHED_TIME_SHIFT 0
+#else
+#define CAM_IOSCHED_TIME_SHIFT 8
+#endif
+static inline uintptr_t
+cam_iosched_now(void)
+{
+
+	/* Cast here is to avoid right shifting a signed value */
+	return (uintptr_t)((uint64_t)sbinuptime() >> CAM_IOSCHED_TIME_SHIFT);
+}
+
+static inline uintptr_t
+cam_iosched_delta_t(uintptr_t then)
+{
+
+	/* Since the types are identical, wrapping works correctly */
+	return (cam_iosched_now() - then);
+}
+
+static inline sbintime_t
+cam_iosched_sbintime_t(uintptr_t delta)
+{
+
+	/* Cast here is to widen the type so the left shift doesn't lose precision */
+	return (sbintime_t)((uint64_t)delta << CAM_IOSCHED_TIME_SHIFT);
+}
+
 int cam_iosched_init(struct cam_iosched_softc **, struct cam_periph *periph);
 void cam_iosched_fini(struct cam_iosched_softc *);
 void cam_iosched_sysctl_init(struct cam_iosched_softc *, struct sysctl_ctx_list *, struct sysctl_oid *);

Modified: head/sys/cam/cam_xpt.c
==============================================================================
--- head/sys/cam/cam_xpt.c	Thu Aug 24 21:49:44 2017	(r322861)
+++ head/sys/cam/cam_xpt.c	Thu Aug 24 22:10:58 2017	(r322862)
@@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$");
 
 #include <cam/cam.h>
 #include <cam/cam_ccb.h>
+#include <cam/cam_iosched.h>
 #include <cam/cam_periph.h>
 #include <cam/cam_queue.h>
 #include <cam/cam_sim.h>
@@ -3494,7 +3495,7 @@ xpt_run_devq(struct cam_devq *devq)
 			mtx_lock(mtx);
 		else
 			mtx = NULL;
-		work_ccb->ccb_h.qos.sim_data = sbinuptime(); // xxx uintprt_t too small 32bit platforms
+		work_ccb->ccb_h.qos.periph_data = cam_iosched_now();
 		(*(sim->sim_action))(sim, work_ccb);
 		if (mtx)
 			mtx_unlock(mtx);
@@ -4641,7 +4642,7 @@ xpt_done(union ccb *done_ccb)
 		return;
 
 	/* Store the time the ccb was in the sim */
-	done_ccb->ccb_h.qos.sim_data = sbinuptime() - done_ccb->ccb_h.qos.sim_data;
+	done_ccb->ccb_h.qos.periph_data = cam_iosched_delta_t(done_ccb->ccb_h.qos.periph_data);
 	hash = (done_ccb->ccb_h.path_id + done_ccb->ccb_h.target_id +
 	    done_ccb->ccb_h.target_lun) % cam_num_doneqs;
 	queue = &cam_doneqs[hash];
@@ -4664,7 +4665,7 @@ xpt_done_direct(union ccb *done_ccb)
 		return;
 
 	/* Store the time the ccb was in the sim */
-	done_ccb->ccb_h.qos.sim_data = sbinuptime() - done_ccb->ccb_h.qos.sim_data;
+	done_ccb->ccb_h.qos.periph_data = cam_iosched_delta_t(done_ccb->ccb_h.qos.periph_data);
 	xpt_done_process(&done_ccb->ccb_h);
 }