Switch from using mutexes to using semaphores to protect against early

completion of synchronous commands.  Also switch to a per-array bioq as it
appears to improve performance.

Submitted by:	mbr, imp.ch (bioq change)

sys/dev/ips/ips.c

@@ -102,16 +102,23 @@ static void ips_cmd_dmaload(void *cmdptr, bus_dma_segment_t *segments,int segnum
 static __inline__ int ips_cmdqueue_free(ips_softc_t *sc)
 {
 	int i, error = -1;
-	intrmask_t mask = splbio();
+	ips_command_t *command;
+	intrmask_t mask;
+
+	mask = splbio();
 	if(!sc->used_commands){
 		for(i = 0; i < sc->max_cmds; i++){
-			if(!(sc->commandarray[i].command_phys_addr))
+
+			command = &sc->commandarray[i];
+			sema_destroy(&command->cmd_sema);
+
+			if(command->command_phys_addr == 0)
 				continue;
 			bus_dmamap_unload(sc->command_dmatag, 
-					  sc->commandarray[i].command_dmamap);
+					  command->command_dmamap);
 			bus_dmamem_free(sc->command_dmatag, 
-					sc->commandarray[i].command_buffer,
-					sc->commandarray[i].command_dmamap);
+					command->command_buffer,
+					command->command_dmamap);
 		}
 		error = 0;
 		sc->state |= IPS_OFFLINE;
@@ -128,14 +135,11 @@ static __inline__ int ips_cmdqueue_init(ips_softc_t *sc)
 	ips_command_t *command;
 	SLIST_INIT(&sc->free_cmd_list);
 	STAILQ_INIT(&sc->cmd_wait_list);
-	for(i = 0; i < sc->max_cmds; i++){
-		sc->commandarray[i].id = i;
-		sc->commandarray[i].sc = sc;
-		SLIST_INSERT_HEAD(&sc->free_cmd_list, &sc->commandarray[i], 
-				  next);	
-	}
 	for(i = 0; i < sc->max_cmds; i++){
 		command = &sc->commandarray[i];
+		command->id = i;
+		command->sc = sc;
+
 		if(bus_dmamem_alloc(sc->command_dmatag,&command->command_buffer,
 		    BUS_DMA_NOWAIT, &command->command_dmamap))
 			goto error;
@@ -147,12 +151,15 @@ static __inline__ int ips_cmdqueue_init(ips_softc_t *sc)
 			    command->command_buffer, command->command_dmamap);
 			goto error;
 		}
+
+		sema_init(&command->cmd_sema, 0, "IPS Command Semaphore");
+		SLIST_INSERT_HEAD(&sc->free_cmd_list, command, next);	
 	}
 	sc->state &= ~IPS_OFFLINE;
 	return 0;
 error:
-		ips_cmdqueue_free(sc);
-		return ENOMEM;
+	ips_cmdqueue_free(sc);
+	return ENOMEM;
 }
 
 static int ips_add_waiting_command(ips_softc_t *sc, int (*callback)(ips_command_t *), void *data, unsigned long flags)
@@ -254,6 +261,10 @@ void ips_insert_free_cmd(ips_softc_t *sc, ips_command_t *command)
 {
 	intrmask_t mask;
 	mask = splbio();
+
+	if (sema_value(&command->cmd_sema) != 0)
+		panic("ips: command returned non-zero semaphore");
+
 	SLIST_INSERT_HEAD(&sc->free_cmd_list, command, next);
 	(sc->used_commands)--;
 	splx(mask);
@@ -375,6 +386,7 @@ int ips_adapter_init(ips_softc_t *sc)
 {
         int i;
         DEVICE_PRINTF(1,sc->dev, "initializing\n");
+
         if (bus_dma_tag_create(	/* parent    */	sc->adapter_dmatag,
 				/* alignemnt */	1,
 				/* boundary  */	0,
@@ -420,8 +432,6 @@ int ips_adapter_init(ips_softc_t *sc)
 	if(sc->ips_adapter_reinit(sc, 0))
 		goto error;
 
-	mtx_init(&sc->cmd_mtx, "ips command mutex", NULL, MTX_DEF);
-
 	/* initialize ffdc values */
 	microtime(&sc->ffdc_resettime);
 	sc->ffdc_resetcount = 1;
@@ -534,8 +544,6 @@ int ips_adapter_free(ips_softc_t *sc)
 	mask = splbio();
 	untimeout(ips_timeout, sc, sc->timer);
 	splx(mask);
-	if (mtx_initialized(&sc->cmd_mtx))
-		mtx_destroy(&sc->cmd_mtx);
 
 	if(sc->sg_dmatag)
 		bus_dma_tag_destroy(sc->sg_dmatag);

sys/dev/ips/ips.h

@@ -39,6 +39,7 @@
 #include <sys/bio.h>
 #include <sys/malloc.h>
 #include <sys/mutex.h>
+#include <sys/sema.h>
 #include <sys/time.h>
 
 #include <machine/bus_memio.h>
@@ -376,6 +377,7 @@ typedef struct ips_command{
 	bus_dmamap_t		command_dmamap;
 	void *			command_buffer;
 	u_int32_t		command_phys_addr;/*WARNING! must be changed if 64bit addressing ever used*/	
+	struct sema		cmd_sema;
 	ips_cmd_status_t	status;
 	SLIST_ENTRY(ips_command)	next;
 	bus_dma_tag_t		data_dmatag;
@@ -427,7 +429,9 @@ typedef struct ips_softc{
         void                    (* ips_adapter_intr)(void *sc);
 	void			(* ips_issue_cmd)(ips_command_t *command);
 	ips_copper_queue_t *	copper_queue;
-	struct mtx		cmd_mtx;
+	struct mtx		queue_mtx;
+	struct bio_queue_head	queue;
+
 }ips_softc_t;
 
 /* function defines from ips_ioctl.c */
@@ -438,7 +442,7 @@ extern void ipsd_finish(struct bio *iobuf);
 
 /* function defines from ips_commands.c */
 extern int ips_flush_cache(ips_softc_t *sc);
-extern void ips_start_io_request(ips_softc_t *sc, struct bio *iobuf);
+extern void ips_start_io_request(ips_softc_t *sc);
 extern int ips_get_drive_info(ips_softc_t *sc);
 extern int ips_get_adapter_info(ips_softc_t *sc);
 extern int ips_ffdc_reset(ips_softc_t *sc);

sys/dev/ips/ips_commands.c

@@ -44,9 +44,7 @@ static void ips_wakeup_callback(ips_command_t *command)
 	status->value = command->status.value;
 	bus_dmamap_sync(command->sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_POSTWRITE);
-	mtx_lock(&command->sc->cmd_mtx);
-	wakeup(status);
-	mtx_unlock(&command->sc->cmd_mtx);
+	sema_post(&command->cmd_sema);
 }
 /* Below are a series of functions for sending an IO request
  * to the adapter.  The flow order is: start, send, callback, finish.
@@ -163,15 +161,24 @@ static int ips_send_io_request(ips_command_t *command)
 	return 0;
 } 
 
-void ips_start_io_request(ips_softc_t *sc, struct bio *iobuf)
+void ips_start_io_request(ips_softc_t *sc)
 {
-	if(ips_get_free_cmd(sc, ips_send_io_request, iobuf, 0)){
-		device_printf(sc->dev, "no mem for command slots!\n");
-		iobuf->bio_flags |= BIO_ERROR;
-		iobuf->bio_error = ENOMEM;
-		ipsd_finish(iobuf);
+	struct bio *iobuf;
+
+	mtx_lock(&sc->queue_mtx);
+	iobuf = bioq_first(&sc->queue);
+	if(!iobuf) {
+		mtx_unlock(&sc->queue_mtx);
 		return;
 	}
+
+	if(ips_get_free_cmd(sc, ips_send_io_request, iobuf, IPS_NOWAIT_FLAG)){
+		mtx_unlock(&sc->queue_mtx);
+		return;
+	}
+	
+	bioq_remove(&sc->queue, iobuf);
+	mtx_unlock(&sc->queue_mtx);
 	return;
 }
 
@@ -236,15 +243,13 @@ static int ips_send_adapter_info_cmd(ips_command_t *command)
 		goto exit;
 	}
 	command->callback = ips_wakeup_callback;
-	mtx_lock(&sc->cmd_mtx);
 	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
 			command->data_buffer,IPS_ADAPTER_INFO_LEN, 
 			ips_adapter_info_callback, command, BUS_DMA_NOWAIT);
 
 	if ((status->value == IPS_ERROR_STATUS) ||
-	    (msleep(status, &sc->cmd_mtx, 0, "ips", 30*hz) == EWOULDBLOCK))
+	    (sema_timedwait(&command->cmd_sema, 30*hz) == 0))
 		error = ETIMEDOUT;
-	mtx_unlock(&sc->cmd_mtx);
 
 	if (error == 0) {
 		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
@@ -343,14 +348,12 @@ static int ips_send_drive_info_cmd(ips_command_t *command)
 		goto exit;
 	}
 	command->callback = ips_wakeup_callback;
-	mtx_lock(&sc->cmd_mtx);
 	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
 			command->data_buffer,IPS_DRIVE_INFO_LEN, 
 			ips_drive_info_callback, command, BUS_DMA_NOWAIT);
 	if ((status->value == IPS_ERROR_STATUS) ||
-	    (msleep(status, &sc->cmd_mtx, 0, "ips", 10*hz) == EWOULDBLOCK))
+	    (sema_timedwait(&command->cmd_sema, 10*hz) == 0))
 		error = ETIMEDOUT;
-	mtx_unlock(&sc->cmd_mtx);
 
 	if (error == 0) {
 		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
@@ -406,11 +409,9 @@ static int ips_send_flush_cache_cmd(ips_command_t *command)
 	command_struct->id = command->id;
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
-	mtx_lock(&sc->cmd_mtx);
 	sc->ips_issue_cmd(command);
 	if (status->value != IPS_ERROR_STATUS)
-		msleep(status, &sc->cmd_mtx, 0, "flush2", 0);
-	mtx_unlock(&sc->cmd_mtx);
+		sema_wait(&command->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
 }
@@ -494,11 +495,9 @@ static int ips_send_ffdc_reset_cmd(ips_command_t *command)
 
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap,
 			BUS_DMASYNC_PREWRITE);
-	mtx_lock(&sc->cmd_mtx);
 	sc->ips_issue_cmd(command);
 	if (status->value != IPS_ERROR_STATUS)
-		msleep(status, &sc->cmd_mtx, 0, "ffdc", 0);
-	mtx_unlock(&sc->cmd_mtx);
+		sema_wait(&command->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
 }
@@ -603,14 +602,12 @@ static int ips_read_nvram(ips_command_t *command){
 		goto exit;
 	}
 	command->callback = ips_write_nvram;
-	mtx_lock(&sc->cmd_mtx);
 	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
 			command->data_buffer,IPS_NVRAM_PAGE_SIZE, 
 			ips_read_nvram_callback, command, BUS_DMA_NOWAIT);
 	if ((status->value == IPS_ERROR_STATUS) ||
-	    (msleep(status, &sc->cmd_mtx, 0, "ips", 0) == EWOULDBLOCK))
+	    (sema_timedwait(&command->cmd_sema, 30*hz) == 0))
 		error = ETIMEDOUT;
-	mtx_unlock(&sc->cmd_mtx);
 
 	if (error == 0) {
 		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
@@ -661,11 +658,9 @@ static int ips_send_config_sync_cmd(ips_command_t *command)
 	command_struct->reserve2 = IPS_POCL;
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
-	mtx_lock(&sc->cmd_mtx);
 	sc->ips_issue_cmd(command);
 	if (status->value != IPS_ERROR_STATUS)
-		msleep(status, &sc->cmd_mtx, 0, "ipssyn", 0);
-	mtx_unlock(&sc->cmd_mtx);
+		sema_wait(&command->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
 }
@@ -684,11 +679,9 @@ static int ips_send_error_table_cmd(ips_command_t *command)
 	command_struct->reserve2 = IPS_CSL;
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
-	mtx_lock(&sc->cmd_mtx);
 	sc->ips_issue_cmd(command);
 	if (status->value != IPS_ERROR_STATUS)
-		msleep(status, &sc->cmd_mtx, 0, "ipsetc", 0);
-	mtx_unlock(&sc->cmd_mtx);
+		sema_wait(&command->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
 }

sys/dev/ips/ips_disk.c

@@ -79,6 +79,9 @@ static int ipsd_close(struct disk *dp)
 /* ipsd_finish is called to clean up and return a completed IO request */
 void ipsd_finish(struct bio *iobuf)
 {
+	ipsdisk_softc_t *dsc;
+	dsc = iobuf->bio_disk->d_drv1;	
+
 	if (iobuf->bio_flags & BIO_ERROR) {
 		ipsdisk_softc_t *dsc;
 		dsc = iobuf->bio_disk->d_drv1; 
@@ -87,6 +90,7 @@ void ipsd_finish(struct bio *iobuf)
 		iobuf->bio_resid = 0;
 
 	biodone(iobuf);	
+	ips_start_io_request(dsc->sc);
 }
 
 
@@ -97,7 +101,10 @@ static void ipsd_strategy(struct bio *iobuf)
 	dsc = iobuf->bio_disk->d_drv1;	
 	DEVICE_PRINTF(8,dsc->dev,"in strategy\n");
 	iobuf->bio_driver1 = (void *)(uintptr_t)dsc->sc->drives[dsc->disk_number].drivenum;
-	ips_start_io_request(dsc->sc, iobuf);
+	mtx_lock(&dsc->sc->queue_mtx);
+	bioq_disksort(&dsc->sc->queue, iobuf);
+	mtx_unlock(&dsc->sc->queue_mtx);
+	ips_start_io_request(dsc->sc);
 }
 
 static int ipsd_probe(device_t dev)
@@ -161,4 +168,3 @@ static int ipsd_detach(device_t dev)
 	disk_destroy(dsc->ipsd_disk);
 	return 0;
 }
-

sys/dev/ips/ips_pci.c

@@ -135,6 +135,8 @@ static int ips_pci_attach(device_t dev)
         }
 	sc->ips_ich.ich_func = ips_intrhook;
 	sc->ips_ich.ich_arg = sc;
+	mtx_init(&sc->queue_mtx, "IPS bioqueue lock", MTX_DEF, 0);
+	bioq_init(&sc->queue);
 	if (config_intrhook_establish(&sc->ips_ich) != 0) {
 		printf("IPS can't establish configuration hook\n");
 		goto error;
@@ -182,7 +184,7 @@ static int ips_pci_detach(device_t dev)
 		if(ips_adapter_free(sc))
 			return EBUSY;
 		ips_pci_free(sc);
-		mtx_destroy(&sc->cmd_mtx);
+		bioq_flush(&sc->queue, NULL, ENXIO);
 	}
 	return 0;
 }