dont try to prevent multiple concurrent runs of a devices start routine

by setting flags around the loop. there is a race which can prevent
necessary work being completed by any of the currently running instances
of xxstart.

the caveat with the removal of this code is because multiple xxstarts can
be running at the same time they can cause io reordering, but that is less
of a problem than no io.

found by and fix tested by claudio@
debugged with krw@ claudio@ beck@ deraadt@

6 files changed, 6 insertions, 60 deletions

diff --git a/sys/scsi/cd.c b/sys/scsi/cd.c
index 5b993c3077c..49ab9d6f73b 100644
--- a/sys/scsi/cd.c
+++ b/sys/scsi/cd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cd.c,v 1.157 2009/12/16 10:51:28 dlg Exp $	*/
+/*	$OpenBSD: cd.c,v 1.158 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: cd.c,v 1.100 1997/04/02 02:29:30 mycroft Exp $	*/
 
 /*
@@ -104,7 +104,6 @@ struct cd_softc {
 #define	CDF_LABELLING	0x08		/* writing label */
 #define	CDF_ANCIENT	0x10		/* disk is ancient; for minphys */
 #define CDF_WAITING	0x100
-#define CDF_STARTING	0x200
 	struct scsi_link *sc_link;	/* contains our targ, lun, etc. */
 	struct cd_parms {
 		u_int32_t blksize;
@@ -611,15 +610,6 @@ cdstart(void *v)
 	 * Check if the device has room for another command
 	 */
 
-	mtx_enter(&sc->sc_start_mtx);
-	if (ISSET(sc->sc_flags, CDF_STARTING)) {
-		mtx_leave(&sc->sc_start_mtx);
-		return;
-	}
-
-	SET(sc->sc_flags, CDF_STARTING);
-	mtx_leave(&sc->sc_start_mtx);
-
 	CLR(sc->sc_flags, CDF_WAITING);
 	while (!ISSET(sc->sc_flags, CDF_WAITING) &&
 	    (bp = cd_buf_dequeue(sc)) != NULL) {
@@ -700,9 +690,6 @@ cdstart(void *v)
 
 		scsi_xs_exec(xs);
 	}
-	mtx_enter(&sc->sc_start_mtx);
-	CLR(sc->sc_flags, CDF_STARTING);
-	mtx_leave(&sc->sc_start_mtx);
 }
 
 void
diff --git a/sys/scsi/sd.c b/sys/scsi/sd.c
index 35ee5bfcf79..f00ca42ab28 100644
--- a/sys/scsi/sd.c
+++ b/sys/scsi/sd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sd.c,v 1.175 2010/01/05 01:21:34 dlg Exp $	*/
+/*	$OpenBSD: sd.c,v 1.176 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $	*/
 
 /*-
@@ -693,15 +693,6 @@ sdstart(void *v)
 
 	SC_DEBUG(link, SDEV_DB2, ("sdstart\n"));
 
-	mtx_enter(&sc->sc_start_mtx);
-	if (ISSET(sc->flags, SDF_STARTING)) {
-		mtx_leave(&sc->sc_start_mtx);
-		return;
-	}
-
-	SET(sc->flags, SDF_STARTING);
-	mtx_leave(&sc->sc_start_mtx);
-
 	CLR(sc->flags, SDF_WAITING);
 	while (!ISSET(sc->flags, SDF_WAITING) &&
 	    (bp = sd_buf_dequeue(sc)) != NULL) {
@@ -768,10 +759,6 @@ sdstart(void *v)
 
 		scsi_xs_exec(xs);
 	}
-
-	mtx_enter(&sc->sc_start_mtx);
-	CLR(sc->flags, SDF_STARTING);
-	mtx_leave(&sc->sc_start_mtx);
 }
 
 void
diff --git a/sys/scsi/sdvar.h b/sys/scsi/sdvar.h
index 115d3ee07be..ad44adf6b85 100644
--- a/sys/scsi/sdvar.h
+++ b/sys/scsi/sdvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sdvar.h,v 1.25 2009/12/03 14:31:03 dlg Exp $	*/
+/*	$OpenBSD: sdvar.h,v 1.26 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: sdvar.h,v 1.7 1998/08/17 00:49:03 mycroft Exp $	*/
 
 /*-
@@ -61,7 +61,6 @@ struct sd_softc {
 #define	SDF_DIRTY	0x20		/* disk is dirty; needs cache flush */
 #define	SDF_DYING	0x40		/* dying, when deactivated */
 #define SDF_WAITING	0x80		/* bus is busy, try again later */
-#define SDF_STARTING	0x100		/* sdstart is already running */
 	struct scsi_link	*sc_link; /* contains our targ, lun, etc. */
 	struct disk_parms {
 		u_long	heads;		/* number of heads */
diff --git a/sys/scsi/ss.c b/sys/scsi/ss.c
index e1c09979613..900431f23a3 100644
--- a/sys/scsi/ss.c
+++ b/sys/scsi/ss.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ss.c,v 1.65 2010/01/02 23:28:51 dlg Exp $	*/
+/*	$OpenBSD: ss.c,v 1.66 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: ss.c,v 1.10 1996/05/05 19:52:55 christos Exp $	*/
 
 /*
@@ -673,15 +673,6 @@ ssstart(v)
 
 	SC_DEBUG(sc_link, SDEV_DB2, ("ssstart\n"));
 
-	mtx_enter(&ss->start_mtx);
-	if (ISSET(ss->flags, SSF_STARTING)) {
-		mtx_leave(&ss->start_mtx);
-		return;
-	}
-
-	SET(ss->flags, SSF_STARTING);
-	mtx_leave(&ss->start_mtx);
-
 	CLR(ss->flags, SSF_WAITING);
 	while (!ISSET(ss->flags, SSF_WAITING) &&
 	    (bp = ss_buf_dequeue(ss)) != NULL) {
@@ -710,9 +701,6 @@ ssstart(v)
 			scsi_xs_exec(xs);
 		}
 	}
-	mtx_enter(&ss->start_mtx);
-	CLR(ss->flags, SSF_STARTING);
-	mtx_leave(&ss->start_mtx);
 }
 
 void
diff --git a/sys/scsi/ssvar.h b/sys/scsi/ssvar.h
index 441e0e0ba80..7af6756a314 100644
--- a/sys/scsi/ssvar.h
+++ b/sys/scsi/ssvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ssvar.h,v 1.13 2010/01/02 23:28:51 dlg Exp $	*/
+/*	$OpenBSD: ssvar.h,v 1.14 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: ssvar.h,v 1.2 1996/03/30 21:47:11 christos Exp $	*/
 
 /*
@@ -68,7 +68,6 @@ struct ss_softc {
 #define SSF_TRIGGERED	0x01	/* read operation has been primed */
 #define	SSF_LOADED	0x02	/* parameters loaded */
 #define SSF_WAITING	0x04
-#define SSF_STARTING	0x08
 	struct scsi_link *sc_link;	/* contains our targ, lun, etc.	*/
 	struct scan_io sio;
 	struct buf buf_queue;		/* the queue of pending IO operations */
diff --git a/sys/scsi/st.c b/sys/scsi/st.c
index 81389185f9f..6525da556ff 100644
--- a/sys/scsi/st.c
+++ b/sys/scsi/st.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: st.c,v 1.88 2009/12/26 09:12:55 dlg Exp $	*/
+/*	$OpenBSD: st.c,v 1.89 2010/01/09 21:12:06 dlg Exp $	*/
 /*	$NetBSD: st.c,v 1.71 1997/02/21 23:03:49 thorpej Exp $	*/
 
 /*
@@ -287,7 +287,6 @@ struct scsi_device st_switch = {
 #define	ST_MOUNTED	0x0800	/* Device is presently mounted */
 #define	ST_DONTBUFFER	0x1000	/* Disable buffering/caching */
 #define ST_WAITING	0x2000
-#define ST_STARTING	0x4000
 
 #define	ST_PER_ACTION	(ST_AT_FILEMARK | ST_EIO_PENDING | ST_BLANK_READ)
 #define	ST_PER_MOUNT	(ST_INFO_VALID | ST_BLOCK_SET | ST_WRITTEN | \
@@ -991,15 +990,6 @@ ststart(void *v)
 	 * doing one
 	 */
 
-	mtx_enter(&st->start_mtx);
-	if (ISSET(st->flags, ST_STARTING)) {
-		mtx_leave(&st->start_mtx);
-		return;
-	}
-
-	SET(st->flags, ST_STARTING);
-	mtx_leave(&st->start_mtx);
-
 	CLR(st->flags, ST_WAITING);
 	while (!ISSET(st->flags, ST_WAITING) &&
 	    (bp = st_buf_dequeue(st)) != NULL) {
@@ -1124,10 +1114,6 @@ ststart(void *v)
 		 */
 		scsi_xs_exec(xs);
 	} /* go back and see if we can cram more work in.. */
-
-	mtx_enter(&st->start_mtx);
-	CLR(st->flags, ST_STARTING);
-	mtx_leave(&st->start_mtx);
 }
 
 void