SMP-friendly "fdomain" is timer-hostile

Kevin Buhr (buhr@stat.wisc.edu)
12 Jul 1998 12:41:28 -0500


The interrupt handler in "linux/drivers/scsi/fdomain.c" was never
written to be run with interrupts disabled. It does slow ISA
transfers of sizeable chunks of data and, in theory, can wait up to
350 milliseconds for device selection to complete!

I've found that, on my modest 486 machine, moderately loading a SCSI
partition with reads will cause 10-50% of all timer ticks to be lost.
I would have discovered this sooner, but until last night I didn't
notice that "xntpd" had been periodically stepping the time by 10-90
seconds after disk I/O.

The following patch fixes the problem on my UP machine with a 2.1.106
SMP kernel. However, I'm pretty fuzzy on the "io_request_lock"
semantics, even after reading through old "linux-kernel" posts, so
it's quite possible it's dead wrong on an SMP machine.

For one thing, it's not clear to me exactly what request queue
activities this lock is supposed to protect. If I understand it
correctly, the only critical sections a *typical* SCSI driver
non-sharing interrupt handler needs to protect are calls to
"scsi_done" (as I've done with this patch), because the handler
normally won't be fooling with the request queue. Am I right about
this?

Kevin <buhr@stat.wisc.edu>

* * *

--- linux-vanilla/drivers/scsi/fdomain.c Thu Jun 4 17:52:21 1998
+++ linux/drivers/scsi/fdomain.c Sun Jul 12 11:27:30 1998
@@ -950,7 +950,7 @@
/* Register the IRQ with the kernel */

retcode = request_irq( interrupt_level,
- do_fdomain_16x0_intr, SA_INTERRUPT, "fdomain", NULL);
+ do_fdomain_16x0_intr, 0, "fdomain", NULL);

if (retcode < 0) {
if (retcode == -EINVAL) {
@@ -1181,8 +1181,9 @@
#endif
}

-void fdomain_16x0_intr( int irq, void *dev_id, struct pt_regs * regs )
+void do_fdomain_16x0_intr( int irq, void *dev_id, struct pt_regs * regs )
{
+ unsigned long flags;
int status;
int done = 0;
unsigned data_count;
@@ -1225,7 +1226,9 @@
#if EVERY_ACCESS
printk( " AFAIL " );
#endif
+ spin_lock_irqsave(&io_request_lock, flags);
my_done( DID_BUS_BUSY << 16 );
+ spin_unlock_irqrestore(&io_request_lock, flags);
return;
}
current_SC->SCp.phase = in_selection;
@@ -1249,7 +1252,9 @@
#if EVERY_ACCESS
printk( " SFAIL " );
#endif
+ spin_lock_irqsave(&io_request_lock, flags);
my_done( DID_NO_CONNECT << 16 );
+ spin_unlock_irqrestore(&io_request_lock, flags);
return;
} else {
#if EVERY_ACCESS
@@ -1593,8 +1598,10 @@
#if EVERY_ACCESS
printk( "BEFORE MY_DONE. . ." );
#endif
+ spin_lock_irqsave(&io_request_lock, flags);
my_done( (current_SC->SCp.Status & 0xff)
| ((current_SC->SCp.Message & 0xff) << 8) | (DID_OK << 16) );
+ spin_unlock_irqrestore(&io_request_lock, flags);
#if EVERY_ACCESS
printk( "RETURNING.\n" );
#endif
@@ -1611,15 +1618,6 @@
in_interrupt_flag = 0;
#endif
return;
-}
-
-void do_fdomain_16x0_intr( int irq, void *dev_id, struct pt_regs * regs )
-{
- unsigned long flags;
-
- spin_lock_irqsave(&io_request_lock, flags);
- fdomain_16x0_intr(irq, dev_id, regs);
- spin_unlock_irqrestore(&io_request_lock, flags);
}

int fdomain_16x0_queue( Scsi_Cmnd * SCpnt, void (*done)(Scsi_Cmnd *))

-
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.altern.org/andrebalsa/doc/lkml-faq.html