d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix 32-bit overflow on latency measurements

Browse Source 
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
This commit is contained in:
Warner Losh 2017-08-24 22:10:58 +00:00
parent a4658c801e
commit e4c9cba71f
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=322862
3 changed files with 44 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
sys/cam/cam_iosched.c
View File

@ -1428,7 +1428,8 @@ cam_iosched_bio_complete(struct cam_iosched_softc *isc, struct bio *bp,
}
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;

38
sys/cam/cam_iosched.h
View File

@ -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 *);

7
sys/cam/cam_xpt.c
View File

@ -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);
}