From owner-svn-src-all@freebsd.org Thu Aug 24 22:11:00 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 54F11DE8FDF; Thu, 24 Aug 2017 22:11:00 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 222F867CA4; Thu, 24 Aug 2017 22:11:00 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v7OMAxlL012500; Thu, 24 Aug 2017 22:10:59 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v7OMAxZ3012497; Thu, 24 Aug 2017 22:10:59 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201708242210.v7OMAxZ3012497@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Thu, 24 Aug 2017 22:10:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r322862 - head/sys/cam X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/cam X-SVN-Commit-Revision: 322862 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Aug 2017 22:11:00 -0000 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 #include +#include #include #include #include @@ -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); }