drivers/scsi/psi240i.c - io_request_lock fix
PATCH 1/1 drivers/scsi/psi240i.c - io_request_lock fix
Hey Linus and the rest of you.
A simple fix for the io_request_lock issue leftovers in drivers/scsi/psi240i.c. Not tested, but compiles. Diffed against 2.5.2-pre10. Please apply.
== Morten
Signature · 26 lines
"Det er ikke lett å være menneske" - sitat fra en klok person. mvh Morten Helgesen UNIX System Administrator & C Developer Nextframe AS [email protected] / 93445641 http://www.nextframe.net --- vanilla-linux-2.5.2-pre10/drivers/scsi/psi240i.c Tue Jan 8 10:57:31 2002 +++ patched-linux-2.5.2-pre10/drivers/scsi/psi240i.c Tue Jan 8 14:48:56 2002 @@ -370,10 +370,11 @@ static void do_Irq_Handler (int irq, void *dev_id, struct pt_regs *regs) { unsigned long flags; + struct Scsi_Host *host = PsiHost[irq - 10]; - spin_lock_irqsave(&io_request_lock, flags); + spin_lock_irqsave(&host->host_lock, flags); Irq_Handler(irq, dev_id, regs); - spin_unlock_irqrestore(&io_request_lock, flags); + spin_unlock_irqrestore(&host->host_lock, flags); } /**************************************************************** * Name: Psi240i_QueueCommand
Quoted 6 lines · Morten Helgesen
Hey Linus and the rest of you.
A simple fix for the
io_request_lockissue leftovers indrivers/scsi/psi240i.c. Not tested, but compiles. Diffed against 2.5.2-pre10. Please apply.
Morten,
There is a bit more involved than just switching io_request_lock to host_lock. The former is global so it could be called from anywhere.
Quoted 2 lines
From the look of this line in the patch:
+ structScsi_Host*host = PsiHost[irq - 10];
It will work if the first controller is allocated irq 10,
the second one irq 11, etc. Unlikely ...
Looking at that driver it seems that it will need a bit of surgery to pass perhaps a Scsi_Cmnd pointer through the request_irq() function so you can follow a pointer chain in do_Irq_Handler() to get hold of the appropriate host_lock.
In the lk 2.5 series host_lock should indeed be held when the callback "scsi_done" is invoked and that most likely occurs in Irq_Handler(). So that is right.
BTW To get a better idea of what is involved, diff the sym53c8xx driver in lk 2.4.15 and the one in the lk 2.5 series now [kudos to Gerard Roudier].
Having been burnt by a well meaning advansys patch that converted a kernel compile time error into a kernel boot time freeze, it is a bit worrying the number of "untested" patches of this nature appearing on lkml.
Doug Gilbert
Quoted 16 lines
drivers/scsi/psi240i.c Tue Jan 8 14:48:56 2002 renamed from drivers/scsi/psi240i.c Tue Jan 8 10:57:31 2002 +3
-2@@ -370,10 +370,11 @@ static void do_Irq_Handler (int irq, void *dev_id, struct pt_regs *regs) { unsigned long flags; + struct Scsi_Host *host = PsiHost[irq - 10]; - spin_lock_irqsave(&io_request_lock, flags); + spin_lock_irqsave(&host->host_lock, flags); Irq_Handler(irq, dev_id, regs); - spin_unlock_irqrestore(&io_request_lock, flags); + spin_unlock_irqrestore(&host->host_lock, flags); } /**************************************************************** * Name: Psi240i_QueueCommand
Quoted 13 lines · Douglas Gilbert
Quoted 6 lines · Morten Helgesen
Hey Linus and the rest of you.
A simple fix for the
io_request_lockissue leftovers indrivers/scsi/psi240i.c. Not tested, but compiles. Diffed against 2.5.2-pre10. Please apply.Morten,
There is a bit more involved than just switchingio_request_locktohost_lock. The former is global so it could be called from anywhere.
Quoted 5 lines
Quoted 2 lines
From the look of this line in the patch:
+ structScsi_Host*host = PsiHost[irq - 10];It will work if the first controller is allocated irq 10,
the second one irq 11, etc. Unlikely ...
Irk yes, that is very ugly! And it's even used currently in the driver as well. How about passing the scsi host as device_id for the isr instead?
Signature · 1 line
Jens Axboe
Hey Jens.
I did the changes based on the changes done to the aha1740.c driver. Below is a diff between a clean 2.4.15 and a clean 2.5.2-pre10, which shows the changes done to aha1740.c :
drivers/scsi/aha1740.c Tue Jan 8 10:56:27 2002 renamed from linux/drivers/scsi/aha1740.c Sun Sep 30 21:26:07 2001 +5 -5
@@ -213,6 +213,7 @@ /* A "high" level interrupt handler */ void aha1740_intr_handle(int irq, void *dev_id, struct pt_regs * regs) { + struct Scsi_Host *host = aha_host[irq - 9]; void (*my_done)(Scsi_Cmnd *); int errstatus, adapstat; int number_serviced;
@@ -221,11 +222,10 @@ unsigned int base; unsigned long flags; - spin_lock_irqsave(&io_request_lock, flags); - - if (!aha_host[irq - 9]) + if (!host) panic("aha1740.c: Irq from unknown host!\n"); - base = aha_host[irq - 9]->io_port; + spin_lock_irqsave(&host->host_lock, flags); + base = host->io_port; number_serviced = 0; while(inb(G2STAT(base)) & G2STAT_INTPEND)
@@ -299,7 +299,7 @@ number_serviced++; } - spin_unlock_irqrestore(&io_request_lock, flags); + spin_unlock_irqrestore(&host->host_lock, flags); } int aha1740_queuecommand(Scsi_Cmnd * SCpnt, void (*done)(Scsi_Cmnd *))
I guess this is not the way to do it.
I now see the problem you guys have pointed out. Sorry, my bad.
== Morten
Quoted 32 lines · Jens Axboe
Quoted 13 lines · Douglas Gilbert
Quoted 6 lines · Morten Helgesen
Hey Linus and the rest of you.
A simple fix for the
io_request_lockissue leftovers indrivers/scsi/psi240i.c. Not tested, but compiles. Diffed against 2.5.2-pre10. Please apply.Morten,
There is a bit more involved than just switchingio_request_locktohost_lock. The former is global so it could be called from anywhere.Quoted 5 lines
Quoted 2 lines
From the look of this line in the patch:
+ structScsi_Host*host = PsiHost[irq - 10];It will work if the first controller is allocated irq 10,
the second one irq 11, etc. Unlikely ...Irk yes, that is very ugly! And it's even used currently in the driver as well. How about passing the scsi host as
device_idfor the isr instead?Signature · 7 lines
Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 8 lines
"Det er ikke lett å være menneske" - sitat fra en klok person. mvh Morten Helgesen UNIX System Administrator & C Developer Nextframe AS [email protected] / 93445641 http://www.nextframe.net