PATCH for scsi-cdrom - use spin_locks.

Itai Nahshon (nahshon@actcom.co.il)
Thu, 19 Feb 1998 01:05:28 +0200

This is a multi-part message in MIME format.
--------------B5ECFA43961511BA1D3E5F7C
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

The coming patch fixes the Ughs and that hangs that I have had
with ATAPI cdrom + ide-scsi + scsi cdrom driver. I use a single
processor but have __SMP__ defined as a default.
To use as a module you also need to export current_lock.

However, I think there is one BAD thing about the scsi subsystem
in a real SMP environment.
All the low-level drivers are using save_flags+cli, the scsi-disk
system is using spin_lock_irqsave and the scsi-cdrom is using (until the
following patch) a bad mixture.
According to Documentation/spinlocks.txt "they are NOT mutually
exclusive".

So, what is the spin_lock_irqsave in sd.c (and now in sr.c) protecting
against?
(is it just against interrupts on the local CPU?)

Please also see my previous postings to linux-kernel with subject
"Missing exports(patch) + Ugh".

Itai

-- 
Itai Nahshon   nahshon@actcom.co.il
        Also   nahshon@vnet.ibm.com
--------------B5ECFA43961511BA1D3E5F7C
Content-Type: text/plain; charset=us-ascii; name="sr-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="sr-patch"


--- linux-2.1.87-orig/drivers/scsi/sr.c	Wed Feb 18 01:31:27 1998
+++ linux-2.1.87/drivers/scsi/sr.c	Thu Feb 19 00:19:52 1998
@@ -432,10 +432,9 @@
     int flag = 0;
 
     while (1==1){
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&current_lock, flags);
 	if (CURRENT != NULL && CURRENT->rq_status == RQ_INACTIVE) {
-	    restore_flags(flags);
+	    spin_unlock_irqrestore(&current_lock, flags);
 	    return;
 	};
 
@@ -451,6 +450,7 @@
          */
         if( SDev->host->in_recovery )
           {
+	    spin_unlock_irqrestore(&current_lock, flags);
             return;
           }
 
@@ -489,7 +489,7 @@
 	    SCpnt = scsi_allocate_device(&CURRENT,
 				    scsi_CDs[DEVICE_NR(CURRENT->rq_dev)].device, 0);
 	else SCpnt = NULL;
-	restore_flags(flags);
+	spin_unlock_irqrestore(&current_lock, flags);
 
 	/* This is a performance enhancement.  We dig down into the request list and
 	 * try to find a queueable request (i.e. device not busy, and host able to
@@ -501,8 +501,7 @@
 	if (!SCpnt && sr_template.nr_dev > 1){
 	    struct request *req1;
 	    req1 = NULL;
-	    save_flags(flags);
-	    cli();
+	    spin_lock_irqsave(&current_lock, flags);
 	    req = CURRENT;
 	    while(req){
 		SCpnt = scsi_request_queueable(req,
@@ -517,7 +516,7 @@
 		else
 		    req1->next = req->next;
 	    };
-	    restore_flags(flags);
+	    spin_unlock_irqrestore(&current_lock, flags);
 	};
 
 	if (!SCpnt)


--------------B5ECFA43961511BA1D3E5F7C--




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu