Subject: timeout issue in scsi drivers: mstohz()
To: None <tech-kern@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 04/02/2002 22:56:06
--a8Wt8u1KmwUX3Y2C
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
cleaning up my mail I re-read the thread from june 2001 "problems with ahc vs.
format command" (http://mail-index.netbsd.org/tech-kern/2001/06/).
Part of this thread is about 32bit integer overflow when translating the
timeout in millisecond provided by scsipi to hz for callout.
Some drivers tries to get it right in some ad-hoc way, others just ignore
the issue and are broken for large timeout (such as those used for tape,
or scsi format command).
In this thread we agreed that a common function should be provided
for scsi drivers, and a name, mstohz, was proposed.
I implemented it this evening, and converted the SCSI drivers.
I implemented the function in sys/param.h using comparaison instead of
64 bit arithmetic (in the thread is was pointed out that for most architectures
a test is cheaper than the 64bit arithmetic needed here). ports that want
a MD, more efficient implementation can define mstohz() in their
machine/param.h.
Comments ?
I think this should really go in before 1.6. We periodically have PRs about
this problem in one SCSI driver or another ...
--
Manuel Bouyer <bouyer@antioche.eu.org>
--
--a8Wt8u1KmwUX3Y2C
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=mstohz-diff
Index: sys/param.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/param.h,v
retrieving revision 1.137
diff -u -r1.137 param.h
--- param.h 2002/03/17 19:43:07 1.137
+++ param.h 2002/04/02 20:26:17
@@ -275,4 +275,19 @@
#define UBC_NWINS 1024
#endif
+#ifdef _KERNEL
+/*
+ * macro to convert from milliseconds to hz without integer overflow
+ * Default version using only 32bits arithmetics.
+ * 64bit port can define 64bit version in their <machine/param.h>
+ * 100000 is safe for hz < 20000
+ */
+#ifndef mstohz
+#define mstohz(ms) \
+ (__predict_false((ms) >= 100000)) ? \
+ (((ms) / 1000 + 1) * hz) : \
+ (((ms) * hz) / 1000)
+#endif
+#endif /* _KERNEL */
+
#endif /* !_SYS_PARAM_H_ */
Index: arch/arc/dti/btl.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/arc/dti/btl.c,v
retrieving revision 1.7
diff -u -r1.7 btl.c
--- btl.c 2001/08/20 12:00:50 1.7
+++ btl.c 2002/04/02 20:24:13
@@ -806,7 +806,7 @@
if ((ccb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ccb->xs->xs_callout,
- (ccb->timeout * hz) / 1000, bt_timeout, ccb);
+ mstohz(ccb->timeout), bt_timeout, ccb);
++sc->sc_mbofull;
bt_nextmbx(wmbo, wmbx, mbo);
Index: arch/macppc/dev/mesh.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/macppc/dev/mesh.c,v
retrieving revision 1.13
diff -u -r1.13 mesh.c
--- mesh.c 2001/11/04 12:03:41 1.13
+++ mesh.c 2002/04/02 20:24:27
@@ -503,7 +503,7 @@
sc->sc_prevphase = MESH_SELECTING;
sc->sc_nextstate = MESH_IDENTIFY;
- timeout = (u_int64_t)scb->xs->timeout * (u_int64_t)hz / 1000;
+ timeout = mstohz(scb->xs->timeout);
if (timeout == 0)
timeout = 1;
Index: arch/sgimips/hpc/sbic.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/sgimips/hpc/sbic.c,v
retrieving revision 1.8
diff -u -r1.8 sbic.c
--- sbic.c 2002/03/13 13:12:27 1.8
+++ sbic.c 2002/04/02 20:24:39
@@ -82,9 +82,6 @@
*/
#define SBIC_WAIT(regs, until, timeo) wd33c93_wait(regs, until, timeo, __LINE__)
-/* Convert SCSI timeout from millisecs to hz avoiding overflow */
-#define SCSI_TIMEOUT(t) ((t)>1000000 ? ((t)/1000)*hz : ((t)*hz)/1000)
-
void wd33c93_init __P((struct wd33c93_softc *));
void wd33c93_reset __P((struct wd33c93_softc *));
int wd33c93_go __P((struct wd33c93_softc *, struct wd33c93_acb *));
@@ -838,7 +835,7 @@
*/
if (dev->sc_nexus == acb) {
/* Reschedule timeout. */
- callout_reset(&acb->xs->xs_callout, SCSI_TIMEOUT(acb->timeout),
+ callout_reset(&acb->xs->xs_callout, mstohz(acb->timeout),
wd33c93_timeout, acb);
while (asr & SBIC_ASR_DBR) {
@@ -922,7 +919,7 @@
SBIC_DEBUG(PHASE, ("wd33c93_selectbus %d: ", target));
if ((xs->xs_control & XS_CTL_POLL) == 0)
- callout_reset(&xs->xs_callout, SCSI_TIMEOUT(acb->timeout),
+ callout_reset(&xs->xs_callout, mstohz(acb->timeout),
wd33c93_timeout, acb);
/*
Index: arch/x68k/dev/mha.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/x68k/dev/mha.c,v
retrieving revision 1.27
diff -u -r1.27 mha.c
--- mha.c 2001/12/27 02:23:25 1.27
+++ mha.c 2002/04/02 20:24:48
@@ -748,8 +748,7 @@
TAILQ_INSERT_TAIL(&sc->ready_list, acb, chain);
#if 1
callout_reset(&acb->xs->xs_callout,
- ((u_int64_t)xs->timeout * (u_int64_t)hz) / 1000,
- mha_timeout, acb);
+ mstohz(xs->timeout), mha_timeout, acb);
#endif
/*
Index: dev/eisa/ahb.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/eisa/ahb.c,v
retrieving revision 1.34
diff -u -r1.34 ahb.c
--- ahb.c 2001/11/15 09:48:03 1.34
+++ ahb.c 2002/04/02 20:24:54
@@ -325,7 +325,7 @@
if ((ecb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ecb->xs->xs_callout,
- (ecb->timeout * hz) / 1000, ahb_timeout, ecb);
+ mstohz(ecb->timeout), ahb_timeout, ecb);
}
/*
@@ -359,7 +359,7 @@
if ((ecb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ecb->xs->xs_callout,
- (ecb->timeout * hz) / 1000, ahb_timeout, ecb);
+ mstohz(ecb->timeout), ahb_timeout, ecb);
}
/*
Index: dev/eisa/uha_eisa.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/eisa/uha_eisa.c,v
retrieving revision 1.17
diff -u -r1.17 uha_eisa.c
--- uha_eisa.c 2001/11/15 09:48:03 1.17
+++ uha_eisa.c 2002/04/02 20:24:55
@@ -265,7 +265,7 @@
if ((mscp->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&mscp->xs->xs_callout,
- (mscp->timeout * hz) / 1000, uha_timeout, mscp);
+ mstohz(mscp->timeout), uha_timeout, mscp);
}
int
Index: dev/i2o/iop.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/i2o/iop.c,v
retrieving revision 1.23
diff -u -r1.23 iop.c
--- iop.c 2002/03/08 11:37:04 1.23
+++ iop.c 2002/04/02 20:24:59
@@ -2208,7 +2208,7 @@
splx(s);
return;
}
- rv = tsleep(im, PRIBIO, "iopmsg", timo * hz / 1000);
+ rv = tsleep(im, PRIBIO, "iopmsg", mstohz(timo));
splx(s);
#ifdef I2ODEBUG
Index: dev/ic/adv.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/adv.c,v
retrieving revision 1.30
diff -u -r1.30 adv.c
--- adv.c 2001/11/15 09:48:03 1.30
+++ adv.c 2002/04/02 20:25:02
@@ -341,8 +341,7 @@
if ((ccb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ccb->xs->xs_callout,
- ((u_int64_t)ccb->timeout * (u_int64_t)hz) / 1000,
- adv_timeout, ccb);
+ mstohz(ccb->timeout), adv_timeout, ccb);
}
}
Index: dev/ic/adw.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/adw.c,v
retrieving revision 1.37
diff -u -r1.37 adw.c
--- adw.c 2001/11/15 09:48:04 1.37
+++ adw.c 2002/04/02 20:25:04
@@ -362,7 +362,7 @@
if ((ccb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ccb->xs->xs_callout,
- (ccb->timeout * hz) / 1000, adw_timeout, ccb);
+ mstohz(ccb->timeout), adw_timeout, ccb);
}
return(errcode);
@@ -884,7 +884,7 @@
* we will reset the bus.
*/
callout_reset(&xs->xs_callout,
- (ccb->timeout * hz) / 1000, adw_timeout, ccb);
+ mstohz(ccb->timeout), adw_timeout, ccb);
} else {
/*
* Abort the operation that has timed out.
@@ -917,7 +917,7 @@
* which timed-out.
*/
callout_reset(&xs->xs_callout,
- (ccb->timeout * hz) / 1000, adw_timeout, ccb);
+ mstohz(ccb->timeout), adw_timeout, ccb);
}
splx(s);
Index: dev/ic/aha.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/aha.c,v
retrieving revision 1.38
diff -u -r1.38 aha.c
--- aha.c 2001/11/15 09:48:05 1.38
+++ aha.c 2002/04/02 20:25:07
@@ -675,7 +675,7 @@
if ((ccb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ccb->xs->xs_callout,
- (ccb->timeout * hz) / 1000, aha_timeout, ccb);
+ mstohz(ccb->timeout), aha_timeout, ccb);
++sc->sc_mbofull;
aha_nextmbx(wmbo, wmbx, mbo);
Index: dev/ic/aic6360.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/aic6360.c,v
retrieving revision 1.77
diff -u -r1.77 aic6360.c
--- aic6360.c 2001/11/15 09:48:05 1.77
+++ aic6360.c 2002/04/02 20:25:11
@@ -1855,8 +1855,7 @@
/* On our first connection, schedule a timeout. */
if ((acb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&acb->xs->xs_callout,
- (acb->timeout * hz) / 1000,
- aic_timeout, acb);
+ mstohz(acb->timeout), aic_timeout, acb);
sc->sc_state = AIC_CONNECTED;
} else if ((sstat1 & SELTO) != 0) {
Index: dev/ic/aic7xxx.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/aic7xxx.c,v
retrieving revision 1.87
diff -u -r1.87 aic7xxx.c
--- aic7xxx.c 2002/01/16 03:27:37 1.87
+++ aic7xxx.c 2002/04/02 20:25:21
@@ -3384,9 +3384,7 @@
if (!(txs->xs_control & XS_CTL_POLL)) {
callout_reset(&scbp->xs->xs_callout,
- (scbp->xs->timeout > 1000000) ?
- (scbp->xs->timeout / 1000) * hz :
- (scbp->xs->timeout * hz) / 1000,
+ mstohz(scbp->xs->timeout),
ahc_timeout, scbp);
}
scbp = LIST_NEXT(scbp, plinks);
@@ -4102,8 +4100,7 @@
scb->flags |= SCB_ACTIVE;
if (!(xs->xs_control & XS_CTL_POLL))
- callout_reset(&scb->xs->xs_callout, xs->timeout > 1000000 ?
- (xs->timeout / 1000) * hz : (xs->timeout * hz) / 1000,
+ callout_reset(&scb->xs->xs_callout, mstohz(xs->timeout),
ahc_timeout, scb);
if ((scb->flags & SCB_TARGET_IMMEDIATE) != 0) {
@@ -4722,10 +4719,7 @@
newtimeout = MAX(active_scb->xs->timeout,
scb->xs->timeout);
callout_reset(&scb->xs->xs_callout,
- newtimeout > 1000000 ?
- (newtimeout / 1000) * hz :
- (newtimeout * hz) / 1000,
- ahc_timeout, scb);
+ mstohz(newtimeout), ahc_timeout, scb);
splx(s);
return;
}
Index: dev/ic/bha.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/bha.c,v
retrieving revision 1.48
diff -u -r1.48 bha.c
--- bha.c 2001/11/15 09:48:05 1.48
+++ bha.c 2002/04/02 20:25:24
@@ -1524,7 +1524,7 @@
if ((ccb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&ccb->xs->xs_callout,
- (ccb->timeout * hz) / 1000, bha_timeout, ccb);
+ mstohz(ccb->timeout), bha_timeout, ccb);
++sc->sc_mbofull;
mbo = bha_nextmbo(sc, mbo);
Index: dev/ic/iha.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/iha.c,v
retrieving revision 1.16
diff -u -r1.16 iha.c
--- iha.c 2002/03/15 12:57:21 1.16
+++ iha.c 2002/04/02 20:25:29
@@ -1193,9 +1193,7 @@
scb->bufaddr = dm->dm_segs[0].ds_addr;
if ((xs->xs_control & XS_CTL_POLL) == 0) {
- int timeout = xs->timeout;
- timeout = (timeout > 100000) ?
- timeout / 1000 * hz : timeout * hz / 1000;
+ int timeout = mstohz(xs->timeout);
if (timeout == 0)
timeout = 1;
callout_reset(&xs->xs_callout, timeout, iha_timeout, scb);
Index: dev/ic/mb89352.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/mb89352.c,v
retrieving revision 1.11
diff -u -r1.11 mb89352.c
--- mb89352.c 2001/11/15 09:48:06 1.11
+++ mb89352.c 2002/04/02 20:25:33
@@ -1749,8 +1749,7 @@
/* On our first connection, schedule a timeout. */
if ((acb->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&acb->xs->xs_callout,
- (acb->timeout * hz) / 1000,
- spc_timeout, acb);
+ mstohz(acb->timeout), spc_timeout, acb);
sc->sc_state = SPC_CONNECTED;
} else if ((ints & INTS_TIMEOUT) != 0) {
Index: dev/ic/ncr5380sbc.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/ncr5380sbc.c,v
retrieving revision 1.47
diff -u -r1.47 ncr5380sbc.c
--- ncr5380sbc.c 2001/11/26 20:15:49 1.47
+++ ncr5380sbc.c 2002/04/02 20:25:38
@@ -1026,7 +1026,7 @@
* Schedule a timeout for the job we are starting.
*/
if ((sr->sr_flags & SR_IMMED) == 0) {
- i = (xs->timeout * hz) / 1000;
+ i = mstohz(xs->timeout);
NCR_TRACE("sched: set timeout=%d\n", i);
callout_reset(&sr->sr_xs->xs_callout, i,
ncr5380_cmd_timeout, sr);
Index: dev/ic/ncr53c9x.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/ncr53c9x.c,v
retrieving revision 1.91
diff -u -r1.91 ncr53c9x.c
--- ncr53c9x.c 2002/03/24 17:16:19 1.91
+++ ncr53c9x.c 2002/04/02 20:25:43
@@ -646,14 +646,7 @@
* always possible that the interrupt may never happen.
*/
if ((ecb->xs->xs_control & XS_CTL_POLL) == 0) {
- int timeout = ecb->timeout;
-
- if (timeout > 1000000)
- timeout = (timeout / 1000) * hz;
- else
- timeout = (timeout * hz) / 1000;
-
- callout_reset(&ecb->xs->xs_callout, timeout,
+ callout_reset(&ecb->xs->xs_callout, mstohz(ecb->timeout),
ncr53c9x_timeout, ecb);
}
@@ -2842,8 +2835,6 @@
ecb->flags |= ECB_ABORT;
if (ecb == sc->sc_nexus) {
- int timeout;
-
/*
* If we're still selecting, the message will be scheduled
* after selection is complete.
@@ -2854,12 +2845,7 @@
/*
* Reschedule timeout.
*/
- timeout = ecb->timeout;
- if (timeout > 1000000)
- timeout = (timeout / 1000) * hz;
- else
- timeout = (timeout * hz) / 1000;
- callout_reset(&ecb->xs->xs_callout, timeout,
+ callout_reset(&ecb->xs->xs_callout, mstohz(ecb->timeout),
ncr53c9x_timeout, ecb);
} else {
/*
Index: dev/ic/osiop.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/osiop.c,v
retrieving revision 1.8
diff -u -r1.8 osiop.c
--- osiop.c 2001/12/16 04:18:13 1.8
+++ osiop.c 2002/04/02 20:25:47
@@ -1046,10 +1046,8 @@
/* handle timeout */
if ((xs->xs_control & XS_CTL_POLL) == 0) {
- int timeout = acb->xs->timeout;
+ int timeout = mstohz(acb->xs->timeout);
/* start expire timer */
- timeout = (timeout > 100000) ?
- timeout / 1000 * hz : timeout * hz / 1000;
if (timeout == 0)
timeout = 1;
callout_reset(&xs->xs_callout, timeout,
Index: dev/ic/siop.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/siop.c,v
retrieving revision 1.50
diff -u -r1.50 siop.c
--- siop.c 2002/03/01 21:37:03 1.50
+++ siop.c 2002/04/02 20:25:51
@@ -1454,8 +1454,7 @@
/* handle timeout */
if ((siop_cmd->xs->xs_control & XS_CTL_POLL) == 0) {
/* start exire timer */
- timeout =
- (u_int64_t)siop_cmd->xs->timeout * (u_int64_t)hz / 1000;
+ timeout = mstohz(siop_cmd->xs->timeout);
if (timeout == 0)
timeout = 1;
callout_reset( &siop_cmd->xs->xs_callout,
Index: dev/ic/sunscpal.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/sunscpal.c,v
retrieving revision 1.11
diff -u -r1.11 sunscpal.c
--- sunscpal.c 2002/03/22 00:15:21 1.11
+++ sunscpal.c 2002/04/02 20:25:55
@@ -1159,9 +1159,7 @@
* Schedule a timeout for the job we are starting.
*/
if ((sr->sr_flags & SR_IMMED) == 0) {
- i = (xs->timeout > 1000000) ?
- (xs->timeout / 1000) * hz :
- (xs->timeout * hz) / 1000;
+ i = mstohz(xs->timeout);
SUNSCPAL_TRACE("sched: set timeout=%d\n", i);
callout_reset(&sr->sr_xs->xs_callout, i,
sunscpal_cmd_timeout, sr);
Index: dev/isa/seagate.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/isa/seagate.c,v
retrieving revision 1.48
diff -u -r1.48 seagate.c
--- seagate.c 2002/01/28 03:47:14 1.48
+++ seagate.c 2002/04/02 20:25:58
@@ -624,7 +624,7 @@
if ((flags & XS_CTL_POLL) == 0) {
callout_reset(&scb->xs->xs_callout,
- (xs->timeout * hz) / 1000, sea_timeout, scb);
+ mstohz(xs->timeout), sea_timeout, scb);
splx(s);
return;
}
Index: dev/isa/uha_isa.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/isa/uha_isa.c,v
retrieving revision 1.22
diff -u -r1.22 uha_isa.c
--- uha_isa.c 2002/01/07 21:47:13 1.22
+++ uha_isa.c 2002/04/02 20:25:59
@@ -324,7 +324,7 @@
if ((mscp->xs->xs_control & XS_CTL_POLL) == 0)
callout_reset(&mscp->xs->xs_callout,
- (mscp->timeout * hz) / 1000, uha_timeout, mscp);
+ mstohz(mscp->timeout), uha_timeout, mscp);
}
/*
Index: dev/isa/wds.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/isa/wds.c,v
retrieving revision 1.48
diff -u -r1.48 wds.c
--- wds.c 2002/01/07 21:47:14 1.48
+++ wds.c 2002/04/02 20:26:01
@@ -790,7 +790,7 @@
if ((scb->flags & SCB_POLLED) == 0)
callout_reset(&scb->xs->xs_callout,
- (scb->timeout * hz) / 1000, wds_timeout, scb);
+ mstohz(scb->timeout), wds_timeout, scb);
++sc->sc_mbofull;
wds_nextmbx(wmbo, wmbx, mbo);
Index: dev/pci/trm.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/trm.c,v
retrieving revision 1.8
diff -u -r1.8 trm.c
--- trm.c 2002/03/13 22:23:50 1.8
+++ trm.c 2002/04/02 20:26:07
@@ -966,10 +966,8 @@
DPRINTF(("trm_select.....\n"));
if ((srb->xs->xs_control & XS_CTL_POLL) == 0) {
- int timeout = srb->xs->timeout;
- timeout = (timeout > 100000) ?
- timeout / 1000 * hz : timeout * hz / 1000;
- callout_reset(&srb->xs->xs_callout, timeout, trm_timeout, srb);
+ callout_reset(&srb->xs->xs_callout, mstohz(srb->xs->timeout),
+ trm_timeout, srb);
}
bus_space_write_1(iot, ioh, TRM_SCSI_HOSTID, sc->sc_id);
Index: dev/scsipi/atapi_wdc.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/scsipi/atapi_wdc.c,v
retrieving revision 1.49
diff -u -r1.49 atapi_wdc.c
--- atapi_wdc.c 2002/03/31 14:36:59 1.49
+++ atapi_wdc.c 2002/04/02 20:26:11
@@ -410,7 +410,7 @@
}
/* start timeout machinery */
if ((sc_xfer->xs_control & XS_CTL_POLL) == 0)
- callout_reset(&chp->ch_callout, sc_xfer->timeout * hz / 1000,
+ callout_reset(&chp->ch_callout, mstohz(sc_xfer->timeout),
wdctimeout, chp);
/* Do control operations specially. */
if (drvp->state < READY) {
@@ -935,7 +935,7 @@
if (wdcwait(chp, WDCS_DSC, WDCS_DSC, 10)) {
/* 10ms not enouth, try again in 1 tick */
if (xfer->c_dscpoll++ >
- sc_xfer->timeout * hz / 1000) {
+ mstohz(sc_xfer->timeout)) {
printf("%s:%d:%d: wait_for_dsc "
"failed\n",
chp->wdc->sc_dev.dv_xname,
--a8Wt8u1KmwUX3Y2C--