[patch] scsi-fixes-2.1.90-A

MOLNAR Ingo (mingo@chiara.csoma.elte.hu)
Wed, 11 Mar 1998 18:37:55 +0100 (CET)


On Wed, 4 Mar 1998, Chris Loveland wrote:

> In recent kernels I have noticed a problem which causes my system to hang.

> spin_lock_irqsave(&some_lock, flags1);
> /* interupts now off */
> save_flags(flags2);
> cli();
> restore_flags(flags2);
> /* interupts are back on, bad stuff happens */
> spin_unlock_irqrestore(&some_lock, flags1);
>
>
> This happens because the above code turns into a mix of calls to the
> __save_flags(), __cli(), __restore_flags() and
> __global_save_flags(), __global_restore_flags()
> functions.

Thanks for reporting this. The attached pre1-2.1.90 based patch should fix
all those places where we called cli/sti/restore_flags with spinlocks
held. I've booted/tested the patch, and those crashes went away. Let me
know if there is any other place in the SCSI code where we call cli/sti
with spinlocks held. The patch also cleans ll_rw_blk.c up a bit.

-- mingo

--- linux/drivers/block/ll_rw_blk.c.orig Sat Mar 14 01:32:49 1998
+++ linux/drivers/block/ll_rw_blk.c Tue Mar 17 01:34:29 1998
@@ -24,9 +24,6 @@

#include <linux/module.h>

-#define ATOMIC_ON() do { } while (0)
-#define ATOMIC_OFF() do { } while (0)
-
/*
* The request-struct contains all necessary data
* to load a nr of sectors into memory
@@ -419,8 +416,6 @@
* not to schedule or do something nonatomic
*/
spin_lock_irqsave(&io_request_lock,flags);
- ATOMIC_ON();
-
req = *get_queue(bh->b_rdev);
if (!req) {
/* MD and loop can't handle plugging without deadlocking */
@@ -480,7 +475,6 @@
continue;

mark_buffer_clean(bh);
- ATOMIC_OFF();
spin_unlock_irqrestore(&io_request_lock,flags);
return;

@@ -490,7 +484,6 @@
/* find an unused request. */
req = get_request(max_req, bh->b_rdev);

- ATOMIC_OFF();
spin_unlock_irqrestore(&io_request_lock,flags);

/* if no request available: if rw_ahead, forget it; otherwise try again blocking.. */
@@ -665,9 +658,7 @@
} else {
unsigned long flags;
spin_lock_irqsave(&io_request_lock,flags);
- ATOMIC_ON();
req[j] = get_request(max_req, rdev);
- ATOMIC_OFF();
spin_unlock_irqrestore(&io_request_lock,flags);
if (req[j] == NULL)
break;
--- linux/drivers/block/raid5.c.orig Tue Jan 27 04:22:13 1998
+++ linux/drivers/block/raid5.c Tue Mar 17 01:34:29 1998
@@ -837,8 +837,11 @@
struct raid5_data *raid_conf = sh->raid_conf;
struct buffer_head *bh_req;

- if (sh->bh_new[dd_idx])
+ if (sh->bh_new[dd_idx]) {
printk("raid5: bug: stripe->bh_new[%d], sector %lu exists\n", dd_idx, sh->sector);
+ printk("forcing oops.\n");
+ *(int*)=0;
+ }

set_bit(BH_Lock, &bh->b_state);

--- linux/drivers/scsi/scsi.c.orig Tue Mar 17 01:27:51 1998
+++ linux/drivers/scsi/scsi.c Tue Mar 17 01:34:30 1998
@@ -28,6 +28,8 @@
* Major improvements to the timeout, abort, and reset processing,
* as well as performance modifications for large queue depths by
* Leonard N. Zubkoff <lnz@dandelion.com>
+ *
+ * Converted cli() code to spinlocks, Ingo Molnar
*/

#include <linux/config.h>
@@ -351,8 +353,8 @@
* (DB, 4 Feb 1995)
*/

- save_flags(flags);
- cli();
+
+ spin_lock_irqsave(&io_request_lock, flags);
host_active = NULL;

for(shpnt=scsi_hostlist; shpnt; shpnt = shpnt->next) {
@@ -384,7 +386,7 @@
sh[index]->host_no);
}

- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
}

static void scan_scsis_done (Scsi_Cmnd * SCpnt)
@@ -1131,21 +1133,21 @@
SCpnt = found;
}

- save_flags(flags);
- cli();
+ __save_flags(flags);
+ __cli();
/* See if this request has already been queued by an interrupt routine
*/
if (req && (req->rq_status == RQ_INACTIVE || req->rq_dev != dev)) {
- restore_flags(flags);
+ __restore_flags(flags);
return NULL;
}
if (!SCpnt || SCpnt->request.rq_status != RQ_INACTIVE) /* Might have changed */
{
if (wait && SCwait && SCwait->request.rq_status != RQ_INACTIVE){
sleep_on(&device->device_wait);
- restore_flags(flags);
+ __restore_flags(flags);
} else {
- restore_flags(flags);
+ __restore_flags(flags);
if (!wait) return NULL;
if (!SCwait) {
printk("Attempt to allocate device channel %d,"
@@ -1195,7 +1197,7 @@
* to complete */
}
atomic_inc(&SCpnt->host->host_active);
- restore_flags(flags);
+ __restore_flags(flags);
SCSI_LOG_MLQUEUE(5, printk("Activating command for device %d (%d)\n",
SCpnt->target,
atomic_read(&SCpnt->host->host_active)));
@@ -1292,8 +1294,7 @@

host = SCpnt->host;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
/* Assign a unique nonzero serial_number. */
if (++serial_number == 0) serial_number = 1;
SCpnt->serial_number = serial_number;
@@ -1303,6 +1304,8 @@
* we can avoid the drive not being ready.
*/
timeout = host->last_reset + MIN_RESET_DELAY;
+ spin_unlock(&io_request_lock);
+
if (jiffies < timeout) {
int ticks_remaining = timeout - jiffies;
/*
@@ -1314,11 +1317,11 @@
* interrupt handler (assuming there is one irq-level per
* host).
*/
- sti();
+ __sti();
while (--ticks_remaining >= 0) udelay(1000000/HZ);
host->last_reset = jiffies - MIN_RESET_DELAY;
}
- restore_flags(flags);
+ __restore_flags(flags); /* this possibly puts us back into __cli() */

if( host->hostt->use_new_eh_code )
{
@@ -1449,21 +1452,20 @@
* ourselves.
*/

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
SCpnt->pid = scsi_pid++;

while (SCSI_BLOCK((Scsi_Device *) NULL, host)) {
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
SCSI_SLEEP(&host->host_wait, SCSI_BLOCK((Scsi_Device *) NULL, host));
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
}

if (host->block) host_active = host;

host->host_busy++;
device->device_busy++;
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);

/*
* Our own function scsi_done (which marks the host as not busy, disables
@@ -1817,8 +1819,7 @@
if(len % SECTOR_SIZE != 0 || len > PAGE_SIZE)
return NULL;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
nbits = len >> 9;
mask = (1 << nbits) - 1;

@@ -1826,7 +1827,7 @@
for(j=0; j<=SECTORS_PER_PAGE - nbits; j++){
if ((dma_malloc_freelist[i] & (mask << j)) == 0){
dma_malloc_freelist[i] |= (mask << j);
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
scsi_dma_free_sectors -= nbits;
#ifdef DEBUG
SCSI_LOG_MLQUEUE(3,printk("SMalloc: %d %p [From:%p]\n",len, dma_malloc_pages[i] + (j << 9)));
@@ -1835,7 +1836,7 @@
return (void *) ((unsigned long) dma_malloc_pages[i] + (j << 9));
}
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
return NULL; /* Nope. No more */
}

@@ -1869,19 +1870,20 @@
if ((mask << sector) >= (1 << SECTORS_PER_PAGE))
panic ("scsi_free:Bad memory alignment");

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
if((dma_malloc_freelist[page] &
(mask << sector)) != (mask<<sector)){
+ spin_unlock_irqrestore(&io_request_lock, flags);
#ifdef DEBUG
printk("scsi_free(obj=%p, len=%d) called from %08lx\n",
obj, len, ret);
#endif
panic("scsi_free:Trying to free unused memory");
+ spin_lock_irqsave(&io_request_lock, flags);
}
scsi_dma_free_sectors += nbits;
dma_malloc_freelist[page] &= ~(mask << sector);
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
return 0;
}
}
@@ -2545,8 +2547,7 @@
/* When we dick with the actual DMA list, we need to
* protect things
*/
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
if (dma_malloc_freelist)
{
size = (dma_sectors / SECTORS_PER_PAGE)*sizeof(FreeSectorBitmap);
@@ -2566,7 +2567,7 @@
dma_malloc_pages = new_dma_malloc_pages;
dma_sectors = new_dma_sectors;
scsi_need_isa_buffer = new_need_isa_buffer;
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);

#ifdef DEBUG_INIT
printk("resize_dma_pool: dma free sectors = %d\n", scsi_dma_free_sectors);
@@ -2805,11 +2806,10 @@
{
online_status = SDpnt->online;
SDpnt->online = FALSE;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
if(SCpnt->request.rq_status != RQ_INACTIVE)
{
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
printk("SCSI device not inactive - state=%d, id=%d\n",
SCpnt->request.rq_status, SCpnt->target);
for(SDpnt1 = shpnt->host_queue; SDpnt1;
@@ -2830,7 +2830,7 @@
*/
SCpnt->state = SCSI_STATE_DISCONNECTING;
SCpnt->request.rq_status = RQ_SCSI_DISCONNECTING; /* Mark as busy */
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
}
}
}
--- linux/drivers/scsi/sd.c.orig Wed Mar 4 15:16:14 1998
+++ linux/drivers/scsi/sd.c Tue Mar 17 01:34:30 1998
@@ -1598,15 +1598,14 @@
target = DEVICE_NR(dev);
gdev = &GENDISK_STRUCT;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&io_request_lock, flags);
if (DEVICE_BUSY || USAGE > maxusage) {
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);
printk("Device busy for revalidation (usage=%d)\n", USAGE);
return -EBUSY;
}
DEVICE_BUSY = 1;
- restore_flags(flags);
+ spin_unlock_irqrestore(&io_request_lock, flags);

max_p = gdev->max_p;
start = target << gdev->minor_shift;

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