finer grained use of splbio in the ioctl paths. only use it when dealing

with the ccb queues rather than over the whole ioctl.

also get rid of the CMD_WAIT flag which is used to restrict the ioctl to
only one caller at a time. this is actually protecting from overuse of
ccbs for management tasks (like bioctl) by making sure the scsi paths
will always have their ccbs. replace the flag with a real lock so that
callers can sleep till the mgmt ccb is free, rather than returning EBUSY

2 files changed, 47 insertions, 35 deletions

diff --git a/sys/dev/ic/ami.c b/sys/dev/ic/ami.c
index 65d1d4b7100..f67b4115cd3 100644
--- a/sys/dev/ic/ami.c
+++ b/sys/dev/ic/ami.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ami.c,v 1.133 2006/03/20 10:10:59 dlg Exp $	*/
+/*	$OpenBSD: ami.c,v 1.134 2006/03/20 10:49:53 dlg Exp $	*/
 
 /*
  * Copyright (c) 2001 Michael Shalayeff
@@ -55,6 +55,7 @@
 #include <sys/kernel.h>
 #include <sys/malloc.h>
 #include <sys/proc.h>
+#include <sys/lock.h>
 
 #include <machine/bus.h>
 
@@ -561,6 +562,9 @@ ami_attach(struct ami_softc *sc)
 		printf("%s: firmware buggy, limiting access to first logical "
 		    "disk\n", DEVNAME(sc));
 
+	/* lock around ioctl requests */
+	lockinit(&sc->sc_lock, PZERO, DEVNAME(sc), 0, 0);
+
 #if NBIO > 0
 	if (bio_register(&sc->sc_dev, ami_ioctl) != 0)
 		printf("%s: controller registration failed", DEVNAME(sc));
@@ -1598,7 +1602,6 @@ int
 ami_ioctl(struct device *dev, u_long cmd, caddr_t addr)
 {
 	struct ami_softc *sc = (struct ami_softc *)dev;
-	int s;
 	int error = 0;
 
 	AMI_DPRINTF(AMI_D_IOCTL, ("%s: ioctl ", DEVNAME(sc)));
@@ -1606,14 +1609,6 @@ ami_ioctl(struct device *dev, u_long cmd, caddr_t addr)
 	if (sc->sc_flags & AMI_BROKEN)
 		return (ENODEV); /* can't do this to broken device for now */
 
-	s = splbio();
-	if (sc->sc_flags & AMI_CMDWAIT) {
-		splx(s);
-		return (EBUSY);
-	}
-
-	sc->sc_flags |= AMI_CMDWAIT;
-
 	switch (cmd) {
 	case BIOCINQ:
 		AMI_DPRINTF(AMI_D_IOCTL, ("inq "));
@@ -1645,10 +1640,6 @@ ami_ioctl(struct device *dev, u_long cmd, caddr_t addr)
 		error = EINVAL;
 	}
 
-	sc->sc_flags &= ~AMI_CMDWAIT;
-
-	splx(s);
-
 	return (error);
 }
 
@@ -1660,10 +1651,17 @@ ami_drv_inq(struct ami_softc *sc, u_int8_t ch, u_int8_t tg, u_int8_t page,
 	struct ami_passthrough *pt;
 	struct scsi_inquiry_data *inq = inqbuf;
 	int error = 0;
+	int s;
+
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE, NULL);
 
+	s = splbio();
 	ccb = ami_get_ccb(sc);
-	if (ccb == NULL)
-		return (ENOMEM);
+	splx(s);
+	if (ccb == NULL) {
+		error = ENOMEM;
+		goto err;
+	}
 
 	ccb->ccb_done = ami_done_ioctl;
 
@@ -1693,8 +1691,8 @@ ami_drv_inq(struct ami_softc *sc, u_int8_t ch, u_int8_t tg, u_int8_t page,
 
 	if (ami_load_ptmem(sc, ccb, inqbuf, sizeof(struct scsi_inquiry_data),
 	    1, 0) != 0) {
-		ami_put_ccb(ccb);
-		return (ENOMEM);
+		error = ENOMEM;
+		goto ptmemerr;
 	}
 
 	ami_start(sc, ccb);
@@ -1716,8 +1714,13 @@ ami_drv_inq(struct ami_softc *sc, u_int8_t ch, u_int8_t tg, u_int8_t page,
 	else if ((inq->device & SID_TYPE) != T_DIRECT)
 		error = EINVAL;
 
+ptmemerr:
+	s = splbio();
 	ami_put_ccb(ccb);
+	splx(s);
 
+err:
+	lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
 	return (error);
 }
 
@@ -1729,15 +1732,23 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8_t par1, u_int8_t par2,
 	struct ami_iocmd *cmd;
 	struct ami_mem *am = NULL;
 	char *idata = NULL;
-	int error = EINVAL;
+	int error = 0;
+	int s;
+
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE, NULL);
 
+	s = splbio();
 	ccb = ami_get_ccb(sc);
-	if (ccb == NULL)
-		return (ENOMEM);
+	splx(s);
+	if (ccb == NULL) {
+		error = ENOMEM;
+		goto err;
+	}
 
 	if (size) {
 		if ((am = ami_allocmem(sc, size)) == NULL) {
-			ami_put_ccb(ccb);
+			error = ENOMEM;
+			goto memerr;
 			return (ENOMEM);
 		}
 		idata = AMIMEM_KVA(am);
@@ -1754,10 +1765,6 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8_t par1, u_int8_t par2,
 	 */
 	switch (opcode) {
 	case AMI_SPEAKER:
-		if (!idata) {
-			ami_put_ccb(ccb);
-			return (ENOMEM);
-		}
 		*idata = par1;
 		break;
 	default:
@@ -1773,17 +1780,21 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8_t par1, u_int8_t par2,
 	while (ccb->ccb_state != AMI_CCB_READY)
 		tsleep(ccb, PRIBIO,"ami_mgmt", 0);
 
-	if (!(ccb->ccb_flags & AMI_CCB_F_ERR)) {
-		if (buffer && size)
-			memcpy(buffer, idata, size);
-
-		error = 0;
-	}
+	if (ccb->ccb_flags & AMI_CCB_F_ERR)
+		error = EIO;
+	else if (buffer && size)
+		memcpy(buffer, idata, size);
 
-	ami_put_ccb(ccb);
 	if (am)
 		ami_freemem(sc, am);
 
+memerr:
+	s = splbio();
+	ami_put_ccb(ccb);
+	splx(s);
+
+err:
+	lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
 	return (error);
 }
 
diff --git a/sys/dev/ic/amivar.h b/sys/dev/ic/amivar.h
index 640b9c37869..0dfe7618eb1 100644
--- a/sys/dev/ic/amivar.h
+++ b/sys/dev/ic/amivar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: amivar.h,v 1.42 2006/03/20 10:10:59 dlg Exp $	*/
+/*	$OpenBSD: amivar.h,v 1.43 2006/03/20 10:49:53 dlg Exp $	*/
 
 /*
  * Copyright (c) 2001 Michael Shalayeff
@@ -90,7 +90,6 @@ struct ami_softc {
 	int			sc_flags;
 #define AMI_CHECK_SIGN	0x0001
 #define AMI_BROKEN 	0x0002
-#define	AMI_CMDWAIT	0x0004
 #define AMI_QUARTZ	0x0008
 
 	/* low-level interface */
@@ -120,6 +119,8 @@ struct ami_softc {
 	struct timeout		sc_run_tmo;
 	int			sc_dis_poll;
 
+	struct lock		sc_lock;
+
 	char			sc_fwver[16];
 	char			sc_biosver[16];
 	int			sc_maxcmds;