Skip to contentSkip to conversation

remove one layer of internal queueing from sym2

PATCH New Christoph Hellwig 1 participant 1 message Nov 26, 2003

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 remove one layer of internal queueing from sym2

->queuecommand can just return 1 to defer the command, no need to queue internally. And while you're at it could you look at killing SYM_OPT_HANDLE_DEVICE_QUEUEING? We're handling QUEUE_FULL conditions just fine at the midlayer. Similarly SYM_OPT_HANDLE_DIR_UNKNOWN and SYM_OPT_LIMIT_COMMAND_REORDERING shouldn't be needed anymore these days.

===== drivers/scsi/sym53c8xx_2/sym_glue.c 1.39 vs edited =====

drivers/scsi/sym53c8xx_2/sym_glue.c +115 -210
@@ -51,6 +51,7 @@
  */
 #define SYM_GLUE_C
 
+#include <linux/completion.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -148,18 +149,17 @@
  *  It is allocated on the eh thread stack.
  */
 struct sym_eh_wait {
-	struct semaphore sem;
+	struct completion eh_done;
 	struct timer_list timer;
 	void (*old_done)(struct scsi_cmnd *);
-	int to_do;
 	int timed_out;
+	int wait:1;
 };
 
 /*
  *  Driver private area in the SCSI command structure.
  */
 struct sym_ucmd {		/* Override the SCSI pointer structure */
-	SYM_QUEHEAD link_cmdq;	/* Must stay at offset ZERO */
 	dma_addr_t data_mapping;
 	u_char	data_mapped;
 	struct sym_eh_wait *eh_wait;
@@ -243,7 +243,6 @@
  */
 void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *ccb)
 {
-	sym_remque(&SYM_UCMD_PTR(ccb)->link_cmdq);
 	unmap_scsi_data(np, ccb);
 	ccb->scsi_done(ccb);
 }
@@ -467,71 +466,6 @@
 }
 
 /*
- *  Queue a SCSI command.
- */
-static int sym_queue_command(struct sym_hcb *np, struct scsi_cmnd *ccb)
-{
-/*	struct scsi_device        *device    = ccb->device; */
-	struct sym_tcb *tp;
-	struct sym_lcb *lp;
-	struct sym_ccb *cp;
-	int	order;
-
-	/*
-	 *  Minimal checkings, so that we will not 
-	 *  go outside our tables.
-	 */
-	if (ccb->device->id == np->myaddr ||
-	    ccb->device->id >= SYM_CONF_MAX_TARGET ||
-	    ccb->device->lun >= SYM_CONF_MAX_LUN) {
-		sym_xpt_done2(np, ccb, CAM_DEV_NOT_THERE);
-		return 0;
-	}
-
-	/*
-	 *  Retreive the target descriptor.
-	 */
-	tp = &np->target[ccb->device->id];
-
-	/*
-	 *  Complete the 1st INQUIRY command with error 
-	 *  condition if the device is flagged NOSCAN 
-	 *  at BOOT in the NVRAM. This may speed up 
-	 *  the boot and maintain coherency with BIOS 
-	 *  device numbering. Clearing the flag allows 
-	 *  user to rescan skipped devices later.
-	 *  We also return error for devices not flagged 
-	 *  for SCAN LUNS in the NVRAM since some mono-lun 
-	 *  devices behave badly when asked for some non 
-	 *  zero LUN. Btw, this is an absolute hack.:-)
-	 */
-	if (ccb->cmnd[0] == 0x12 || ccb->cmnd[0] == 0x0) {
-		if ((tp->usrflags & SYM_SCAN_BOOT_DISABLED) ||
-		    ((tp->usrflags & SYM_SCAN_LUNS_DISABLED) && 
-		     ccb->device->lun != 0)) {
-			tp->usrflags &= ~SYM_SCAN_BOOT_DISABLED;
-			sym_xpt_done2(np, ccb, CAM_DEV_NOT_THERE);
-			return 0;
-		}
-	}
-
-	/*
-	 *  Select tagged/untagged.
-	 */
-	lp = sym_lp(np, tp, ccb->device->lun);
-	order = (lp && lp->s.reqtags) ? M_SIMPLE_TAG : 0;
-
-	/*
-	 *  Queue the SCSI IO.
-	 */
-	cp = sym_get_ccb(np, ccb->device->id, ccb->device->lun, order);
-	if (!cp)
-		return 1;	/* Means resource shortage */
-	sym_queue_scsiio(np, ccb, cp);
-	return 0;
-}
-
-/*
  *  Setup buffers and pointers that address the CDB.
  */
 static inline int sym_setup_cdb(struct sym_hcb *np, struct scsi_cmnd *ccb, struct sym_ccb *cp)
@@ -693,30 +627,6 @@
 	}
 }
 
-
-/*
- *  Requeue awaiting commands.
- */
-static void sym_requeue_awaiting_cmds(struct sym_hcb *np)
-{
-	struct scsi_cmnd *cmd;
-	struct sym_ucmd *ucp = SYM_UCMD_PTR(cmd);
-	SYM_QUEHEAD tmp_cmdq;
-	int sts;
-
-	sym_que_move(&np->s.wait_cmdq, &tmp_cmdq);
-
-	while ((ucp = (struct sym_ucmd *) sym_remque_head(&tmp_cmdq)) != 0) {
-		sym_insque_tail(&ucp->link_cmdq, &np->s.busy_cmdq);
-		cmd = SYM_SCMD_PTR(ucp);
-		sts = sym_queue_command(np, cmd);
-		if (sts) {
-			sym_remque(&ucp->link_cmdq);
-			sym_insque_head(&ucp->link_cmdq, &np->s.wait_cmdq);
-		}
-	}
-}
-
 /*
  * queuecommand method.  Entered with the host adapter lock held and
  * interrupts disabled.
@@ -726,36 +636,79 @@
 {
 	struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
 	struct sym_ucmd *ucp = SYM_UCMD_PTR(cmd);
-	int sts = 0;
+	unsigned int pun = cmd->device->id;
+	unsigned int lun = cmd->device->id;
+	struct sym_tcb *tp;
+	struct sym_lcb *lp;
+	struct sym_ccb *cp;
+	int order;
 
 	cmd->scsi_done     = done;
 	cmd->host_scribble = NULL;
 	memset(ucp, 0, sizeof(*ucp));
 
 	/*
+	 *  Minimal checkings, so that we will not 
+	 *  go outside our tables.
+	 */
+	if (pun == np->myaddr)
+		goto invalid;
+	if (pun >= SYM_CONF_MAX_TARGET || lun >= SYM_CONF_MAX_LUN)
+		goto invalid;
+
+	/*
 	 *  Shorten our settle_time if needed for 
 	 *  this command not to time out.
 	 */
 	if (np->s.settle_time_valid && cmd->timeout_per_command) {
 		unsigned long tlimit = jiffies + cmd->timeout_per_command;
 		tlimit -= SYM_CONF_TIMER_INTERVAL*2;
-		if (time_after(np->s.settle_time, tlimit)) {
+		if (time_after(np->s.settle_time, tlimit))
 			np->s.settle_time = tlimit;
-		}
 	}
 
-	if (np->s.settle_time_valid || !sym_que_empty(&np->s.wait_cmdq)) {
-		sym_insque_tail(&ucp->link_cmdq, &np->s.wait_cmdq);
-		goto out;
-	}
+	if (np->s.settle_time_valid)
+		return 1;
 
-	sym_insque_tail(&ucp->link_cmdq, &np->s.busy_cmdq);
-	sts = sym_queue_command(np, cmd);
-	if (sts) {
-		sym_remque(&ucp->link_cmdq);
-		sym_insque_tail(&ucp->link_cmdq, &np->s.wait_cmdq);
+	tp = &np->target[pun];
+	lp = sym_lp(np, tp, lun);
+
+	/*
+	 *  Complete the 1st INQUIRY command with error 
+	 *  condition if the device is flagged NOSCAN 
+	 *  at BOOT in the NVRAM. This may speed up 
+	 *  the boot and maintain coherency with BIOS 
+	 *  device numbering. Clearing the flag allows 
+	 *  user to rescan skipped devices later.
+	 *  We also return error for devices not flagged 
+	 *  for SCAN LUNS in the NVRAM since some mono-lun 
+	 *  devices behave badly when asked for some non 
+	 *  zero LUN. Btw, this is an absolute hack.:-)
+	 */
+	if (cmd->cmnd[0] == 0x12 || cmd->cmnd[0] == 0x0) {
+		if ((tp->usrflags & SYM_SCAN_BOOT_DISABLED) ||
+		    ((tp->usrflags & SYM_SCAN_LUNS_DISABLED) && lun != 0)) {
+			tp->usrflags &= ~SYM_SCAN_BOOT_DISABLED;
+			goto invalid;
+		}
 	}
-out:
+
+	/*
+	 *  Select tagged/untagged.
+	 */
+	order = (lp && lp->s.reqtags) ? M_SIMPLE_TAG : 0;
+
+	/*
+	 *  Queue the SCSI IO.
+	 */
+	cp = sym_get_ccb(np, pun, lun, order);
+	if (!cp)
+		return 1;
+
+	sym_queue_scsiio(np, cmd, cp);
+	return 0;
+ invalid:
+	sym_xpt_done2(np, cmd, CAM_DEV_NOT_THERE);
 	return 0;
 }
 
@@ -764,25 +717,13 @@
  */
 static irqreturn_t sym53c8xx_intr(int irq, void *dev_id, struct pt_regs * regs)
 {
-	unsigned long flags;
 	struct sym_hcb *np = (struct sym_hcb *)dev_id;
-
-	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
+	unsigned long flags;
 
 	spin_lock_irqsave(np->s.host->host_lock, flags);
-
 	sym_interrupt(np);
-
-	/*
-	 * push queue walk-through to tasklet
-	 */
-	if (!sym_que_empty(&np->s.wait_cmdq) && !np->s.settle_time_valid)
-		sym_requeue_awaiting_cmds(np);
-
 	spin_unlock_irqrestore(np->s.host->host_lock, flags);
 
-	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("]\n");
-
 	return IRQ_HANDLED;
 }
 
@@ -795,12 +736,7 @@
 	unsigned long flags;
 
 	spin_lock_irqsave(np->s.host->host_lock, flags);
-
 	sym_timer(np);
-
-	if (!sym_que_empty(&np->s.wait_cmdq) && !np->s.settle_time_valid)
-		sym_requeue_awaiting_cmds(np);
-
 	spin_unlock_irqrestore(np->s.host->host_lock, flags);
 }
 
@@ -808,17 +744,12 @@
 /*
  *  What the eh thread wants us to perform.
  */
-#define SYM_EH_ABORT		0
-#define SYM_EH_DEVICE_RESET	1
-#define SYM_EH_BUS_RESET	2
-#define SYM_EH_HOST_RESET	3
-
-/*
- *  What we will do regarding the involved SCSI command.
- */
-#define SYM_EH_DO_IGNORE	0
-#define SYM_EH_DO_COMPLETE	1
-#define SYM_EH_DO_WAIT		2
+enum sym_eh_action {
+	SYM_EH_ABORT,
+	SYM_EH_DEVICE_RESET,
+	SYM_EH_BUS_RESET,
+	SYM_EH_HOST_RESET
+};
 
 /*
  *  Our general completion handler.
@@ -826,44 +757,50 @@
 static void __sym_eh_done(struct scsi_cmnd *cmd, int timed_out)
 {
 	struct sym_eh_wait *ep = SYM_UCMD_PTR(cmd)->eh_wait;
+
 	if (!ep)
 		return;
 
 	/* Try to avoid a race here (not 100% safe) */
 	if (!timed_out) {
 		ep->timed_out = 0;
-		if (ep->to_do == SYM_EH_DO_WAIT && !del_timer(&ep->timer))
+		if (ep->wait && !del_timer(&ep->timer))
 			return;
 	}
 
 	/* Revert everything */
-	SYM_UCMD_PTR(cmd)->eh_wait = 0;
+	SYM_UCMD_PTR(cmd)->eh_wait = NULL;
 	cmd->scsi_done = ep->old_done;
 
 	/* Wake up the eh thread if it wants to sleep */
-	if (ep->to_do == SYM_EH_DO_WAIT)
-		up(&ep->sem);
+	if (ep->wait)
+		complete(&ep->eh_done);
 }
 
 /*
  *  scsi_done() alias when error recovery is in progress. 
  */
-static void sym_eh_done(struct scsi_cmnd *cmd) { __sym_eh_done(cmd, 0); }
+static void sym_eh_done(struct scsi_cmnd *cmd)
+{
+	__sym_eh_done(cmd, 0);
+}
 
 /*
  *  Some timeout handler to avoid waiting too long.
  */
-static void sym_eh_timeout(u_long p) { __sym_eh_done((struct scsi_cmnd *)p, 1); }
+static void sym_eh_timeout(u_long p)
+{
+	__sym_eh_done((struct scsi_cmnd *)p, 1);
+}
 
 /*
  *  Generic method for our eh processing.
  *  The 'op' argument tells what we have to do.
  */
-static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
+static int sym_eh_handler(enum sym_eh_action op, char *opname, struct scsi_cmnd *cmd)
 {
 	struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
 	SYM_QUEHEAD *qp;
-	int to_do = SYM_EH_DO_IGNORE;
 	int sts = -1;
 	struct sym_eh_wait eh, *ep = &eh;
 	char devname[20];
@@ -872,48 +809,23 @@
 
 	printf_warning("%s: %s operation started.\n", devname, opname);
 
-#if 0
-	/* This one should be the result of some race, thus to ignore */
-	if (cmd->serial_number != cmd->serial_number_at_timeout)
-		goto prepare;
-#endif
-
-	/* This one is not queued to the core driver -> to complete here */ 
-	FOR_EACH_QUEUED_ELEMENT(&np->s.wait_cmdq, qp) {
-		if (SYM_SCMD_PTR(qp) == cmd) {
-			to_do = SYM_EH_DO_COMPLETE;
-			goto prepare;
-		}
-	}
-
 	/* This one is queued in some place -> to wait for completion */
 	FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
 		struct sym_ccb *cp = sym_que_entry(qp, struct sym_ccb, link_ccbq);
-		if (cp->cam_ccb == cmd) {
-			to_do = SYM_EH_DO_WAIT;
-			goto prepare;
-		}
+		if (cp->cam_ccb == cmd)
+			goto found;
 	}
 
-prepare:
-	/* Prepare stuff to either ignore, complete or wait for completion */
-	switch(to_do) {
-	default:
-	case SYM_EH_DO_IGNORE:
-		goto finish;
-		break;
-	case SYM_EH_DO_WAIT:
-		init_MUTEX_LOCKED(&ep->sem);
-		/* fall through */
-	case SYM_EH_DO_COMPLETE:
-		ep->old_done = cmd->scsi_done;
-		cmd->scsi_done = sym_eh_done;
-		SYM_UCMD_PTR(cmd)->eh_wait = ep;
-	}
+	goto finish;
+
+ found:
+	init_completion(&ep->eh_done);
+	ep->old_done = cmd->scsi_done;
+	cmd->scsi_done = sym_eh_done;
+	SYM_UCMD_PTR(cmd)->eh_wait = ep;
 
 	/* Try to proceed the operation we have been asked for */
-	sts = -1;
-	switch(op) {
+	switch (op) {
 	case SYM_EH_ABORT:
 		sts = sym_abort_scsiio(np, cmd, 1);
 		break;
@@ -930,41 +842,40 @@
 		sts = 0;
 		break;
 	default:
+		sts = -1;
 		break;
 	}
 
 	/* On error, restore everything and cross fingers :) */
 	if (sts) {
-		SYM_UCMD_PTR(cmd)->eh_wait = 0;
+		SYM_UCMD_PTR(cmd)->eh_wait = NULL;
 		cmd->scsi_done = ep->old_done;
-		to_do = SYM_EH_DO_IGNORE;
+		goto finish;
 	}
 
-finish:
-	ep->to_do = to_do;
-	/* Complete the command with locks held as required by the driver */
-	if (to_do == SYM_EH_DO_COMPLETE)
-		sym_xpt_done2(np, cmd, CAM_REQ_ABORTED);
-
-	/* Wait for completion with locks released, as required by kernel */
-	if (to_do == SYM_EH_DO_WAIT) {
-		init_timer(&ep->timer);
-		ep->timer.expires = jiffies + (5*HZ);
-		ep->timer.function = sym_eh_timeout;
-		ep->timer.data = (u_long)cmd;
-		ep->timed_out = 1;	/* Be pessimistic for once :) */
-		add_timer(&ep->timer);
-		spin_unlock_irq(np->s.host->host_lock);
-		down(&ep->sem);
-		spin_lock_irq(np->s.host->host_lock);
-		if (ep->timed_out)
-			sts = -2;
-	}
-	printf_warning("%s: %s operation %s.\n", devname, opname,
-			sts==0?"complete":sts==-2?"timed-out":"failed");
-	return sts? SCSI_FAILED : SCSI_SUCCESS;
-}
+	ep->wait = 1;
 
+	init_timer(&ep->timer);
+	ep->timer.expires = jiffies + (5*HZ);
+	ep->timer.function = sym_eh_timeout;
+	ep->timer.data = (u_long)cmd;
+	ep->timed_out = 1;	/* Be pessimistic for once :) */
+	add_timer(&ep->timer);
+
+	spin_unlock_irq(np->s.host->host_lock);
+	wait_for_completion(&ep->eh_done);
+	spin_lock_irq(np->s.host->host_lock);
+
+	if (ep->timed_out)
+		sts = -2;
+
+ finish:
+	printf_warning("%s: %s operation %s.\n", devname, opname, sts == 0 ?
+			"complete" : sts == -2 ?
+			"timed-out":
+			"failed");
+	return sts ? FAILED : SUCCESS;
+}
 
 /*
  * Error handlers called from the eh thread (one thread per HBA).
@@ -1778,12 +1689,6 @@
 	spin_lock_irqsave(instance->host_lock, flags);
 	if (sym_reset_scsi_bus(np, 0))
 		goto reset_failed;
-
-	/*
-	 *  Initialize some queue headers.
-	 */
-	sym_que_init(&np->s.wait_cmdq);
-	sym_que_init(&np->s.busy_cmdq);
 
 	/*
 	 *  Start the SCRIPTS.

===== drivers/scsi/sym53c8xx_2/sym_glue.h 1.16 vs edited =====

drivers/scsi/sym53c8xx_2/sym_glue.h +0 -3
@@ -382,9 +382,6 @@
 	u_short		io_ws;		/* IO window size		*/
 	int		irq;		/* IRQ number			*/
 
-	SYM_QUEHEAD	wait_cmdq;	/* Awaiting SCSI commands	*/
-	SYM_QUEHEAD	busy_cmdq;	/* Enqueued SCSI commands	*/
-
 	struct timer_list timer;	/* Timer handler link header	*/
 	u_long		lasttime;
 	u_long		settle_time;	/* Resetting the SCSI BUS	*/