Messages in this thread Patch in this message | | | Date | Thu, 19 Feb 1998 01:05:28 +0200 | From | Itai Nahshon <> | Subject | PATCH for scsi-cdrom - use spin_locks. |
| |
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--- 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(¤t_lock, flags); if (CURRENT != NULL && CURRENT->rq_status == RQ_INACTIVE) { - restore_flags(flags); + spin_unlock_irqrestore(¤t_lock, flags); return; }; @@ -451,6 +450,7 @@ */ if( SDev->host->in_recovery ) { + spin_unlock_irqrestore(¤t_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(¤t_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(¤t_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(¤t_lock, flags); }; if (!SCpnt) | |