Fixing the SCSI layer

Alan Cox (alan@lxorguk.ukuu.org.uk)
Sat, 4 Sep 1999 14:35:24 +0100 (BST)


I've been trying to debug a high performance fibrechannel HA under Linux. I'm
now in a situation where its 50/50 whether the bugs remaining are in the
scsi layer or the adapter.

The scsi code however is so hard to read its going to be quicker to clean
it up and fix bits than to try and debug it further. Is anyone else trying
to clean up the scsi mess currently.

I'm not going to write some new scsi layer, sorry someone crazier can do that
just to clean up the cruft, including stuff like banging it all through
indent and extracting common code into functions, moving long complex conditional
code into functions etc.

I'd appreciate a couple of eyeballs over the following first small patches
to extract 'do a command and wait for it'

--- /usr/src/LINUS/linux.vanilla/drivers/scsi/scsi.c Mon Aug 23 15:12:13 1999
+++ scsi.c Sat Sep 4 14:28:15 1999
@@ -459,6 +459,28 @@
#endif

/*
+ * Issue a command and wait for it to complete
+ */
+
+void scsi_wait_cmd (Scsi_Cmnd * SCpnt, const void *cmnd ,
+ void *buffer, unsigned bufflen, void (*done)(Scsi_Cmnd *),
+ int timeout, int retries)
+{
+ unsigned long flags;
+ DECLARE_MUTEX_LOCKED(sem);
+
+ SCpnt->request.sem = &sem;
+ SCpnt->request.rq_status = RQ_SCSI_BUSY;
+ spin_lock_irqsave(&io_request_lock, flags);
+ scsi_do_cmd (SCpnt, (void *) cmnd,
+ buffer, bufflen, done, timeout, retries);
+ spin_unlock_irqrestore(&io_request_lock, flags);
+ down (&sem);
+ SCpnt->request.sem = NULL;
+}
+
+
+/*
* Detecting SCSI devices :
* We scan all present host adapter's busses, from ID 0 to ID (max_id).
* We use the INQUIRY command, determine device type, and pass the ID /
@@ -705,18 +727,10 @@
SCpnt->target = SDpnt->id;
SCpnt->lun = SDpnt->lun;
SCpnt->channel = SDpnt->channel;
- {
- DECLARE_MUTEX_LOCKED(sem);
- SCpnt->request.sem = &sem;
- SCpnt->request.rq_status = RQ_SCSI_BUSY;
- spin_lock_irq(&io_request_lock);
- scsi_do_cmd (SCpnt, (void *) scsi_cmd,
+
+ scsi_wait_cmd (SCpnt, (void *) scsi_cmd,
(void *) NULL,
0, scan_scsis_done, SCSI_TIMEOUT + 4 * HZ, 5);
- spin_unlock_irq(&io_request_lock);
- down (&sem);
- SCpnt->request.sem = NULL;
- }

SCSI_LOG_SCAN_BUS(3, printk ("scsi: scan_scsis_single id %d lun %d. Return code 0x%08x\n",
dev, lun, SCpnt->result));
@@ -748,18 +762,10 @@
scsi_cmd[4] = 255;
scsi_cmd[5] = 0;
SCpnt->cmd_len = 0;
- {
- DECLARE_MUTEX_LOCKED(sem);
- SCpnt->request.sem = &sem;
- SCpnt->request.rq_status = RQ_SCSI_BUSY;
- spin_lock_irq(&io_request_lock);
- scsi_do_cmd (SCpnt, (void *) scsi_cmd,
+
+ scsi_wait_cmd (SCpnt, (void *) scsi_cmd,
(void *) scsi_result,
256, scan_scsis_done, SCSI_TIMEOUT, 3);
- spin_unlock_irq(&io_request_lock);
- down (&sem);
- SCpnt->request.sem = NULL;
- }

SCSI_LOG_SCAN_BUS(3,printk ("scsi: INQUIRY %s with code 0x%x\n",
SCpnt->result ? "failed" : "successful", SCpnt->result));
@@ -894,18 +900,9 @@
scsi_cmd[4] = 0x2a;
scsi_cmd[5] = 0;
SCpnt->cmd_len = 0;
- {
- DECLARE_MUTEX_LOCKED(sem);
- SCpnt->request.rq_status = RQ_SCSI_BUSY;
- SCpnt->request.sem = &sem;
- spin_lock_irq(&io_request_lock);
- scsi_do_cmd (SCpnt, (void *) scsi_cmd,
+ scsi_wait_cmd (SCpnt, (void *) scsi_cmd,
(void *) scsi_result, 0x2a,
scan_scsis_done, SCSI_TIMEOUT, 3);
- spin_unlock_irq(&io_request_lock);
- down (&sem);
- SCpnt->request.sem = NULL;
- }
}

/*
--- /usr/src/LINUS/linux.vanilla/drivers/scsi/sd.c Thu Aug 26 14:42:15 1999
+++ sd.c Sat Sep 4 14:33:55 1999
@@ -1123,6 +1123,22 @@
return retval;
}

+static void sd_wait_cmd (Scsi_Cmnd * SCpnt, const void *cmnd ,
+ void *buffer, unsigned bufflen, void (*done)(Scsi_Cmnd *),
+ int timeout, int retries)
+{
+ DECLARE_MUTEX_LOCKED(sem);
+
+ SCpnt->request.sem = &sem;
+ SCpnt->request.rq_status = RQ_SCSI_BUSY;
+ scsi_do_cmd (SCpnt, (void *) cmnd,
+ buffer, bufflen, done, timeout, retries);
+ spin_unlock_irq(&io_request_lock);
+ down (&sem);
+ spin_lock_irq(&io_request_lock, flags);
+ SCpnt->request.sem = NULL;
+}
+
static void sd_init_done (Scsi_Cmnd * SCpnt)
{
struct request * req;
@@ -1140,8 +1156,8 @@
unsigned char cmd[10];
char nbuff[6];
unsigned char *buffer;
- unsigned long spintime;
- int the_result, retries;
+ unsigned long spintime_value = 0;
+ int the_result, retries, spintime;
Scsi_Cmnd * SCpnt;

/*
@@ -1183,20 +1199,9 @@
SCpnt->sense_buffer[0] = 0;
SCpnt->sense_buffer[2] = 0;

- {
- DECLARE_MUTEX_LOCKED(sem);
- /* Mark as really busy again */
- SCpnt->request.rq_status = RQ_SCSI_BUSY;
- SCpnt->request.sem = &sem;
- scsi_do_cmd (SCpnt,
- (void *) cmd, (void *) buffer,
- 512, sd_init_done, SD_TIMEOUT,
- MAX_RETRIES);
- spin_unlock_irq(&io_request_lock);
- down(&sem);
- spin_lock_irq(&io_request_lock);
- SCpnt->request.sem = NULL;
- }
+ sd_wait_cmd (SCpnt, (void *) cmd, (void *) buffer,
+ 512, sd_init_done, SD_TIMEOUT,
+ MAX_RETRIES);

the_result = SCpnt->result;
retries++;
@@ -1236,16 +1241,18 @@
SCpnt->request.sem = NULL;
}

- spintime = jiffies;
+ spintime = 1;
+ spintime_value = jiffies;
}

time1 = jiffies + HZ;
spin_unlock_irq(&io_request_lock);
- while(jiffies < time1); /* Wait 1 second for next try */
+ while(time_before(jiffies, time1)); /* Wait 1 second for next try */
printk( "." );
spin_lock_irq(&io_request_lock);
}
- } while(the_result && spintime && spintime+100*HZ > jiffies);
+ } while(the_result && spintime &&
+ time_after(spintime_value+100*HZ, jiffies));
if (spintime) {
if (the_result)
printk( "not responding...\n" );
--- /usr/src/LINUS/linux.vanilla/drivers/scsi/sr.c Thu Sep 2 00:48:36 1999
+++ sr.c Sat Sep 4 14:28:14 1999
@@ -901,17 +901,9 @@
memset(buffer, 0, 8);

/* Do the command and wait.. */
- {
- DECLARE_MUTEX_LOCKED(sem);
- SCpnt->request.sem = &sem;
- spin_lock_irqsave(&io_request_lock, flags);
- scsi_do_cmd (SCpnt,
- (void *) cmd, (void *) buffer,
- 512, sr_init_done, SR_TIMEOUT,
- MAX_RETRIES);
- spin_unlock_irqrestore(&io_request_lock, flags);
- down(&sem);
- }
+
+ scsi_wait_cmd (SCpnt, (void *) cmd, (void *) buffer,
+ 512, sr_init_done, SR_TIMEOUT, MAX_RETRIES);

the_result = SCpnt->result;
retries--;
@@ -1045,7 +1037,6 @@
{
Scsi_Cmnd *SCpnt;
Scsi_Device *device = scsi_CDs[MINOR(cdi->dev)].device;
- DECLARE_MUTEX_LOCKED(sem);
unsigned long flags;
int stat;

@@ -1059,15 +1050,11 @@

/* do the locking and issue the command */
SCpnt->request.rq_dev = cdi->dev;
- SCpnt->request.rq_status = RQ_SCSI_BUSY;
/* scsi_do_cmd sets the command length */
SCpnt->cmd_len = 0;
- SCpnt->request.sem = &sem;
- spin_lock_irqsave(&io_request_lock, flags);
- scsi_do_cmd (SCpnt, (void *)cgc->cmd, (void *)cgc->buffer, cgc->buflen,
+
+ scsi_wait_cmd (SCpnt, (void *)cgc->cmd, (void *)cgc->buffer, cgc->buflen,
sr_init_done, SR_TIMEOUT, MAX_RETRIES);
- spin_unlock_irqrestore(&io_request_lock, flags);
- down(&sem);

stat = SCpnt->result;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/