On Fri, 11 Jan 2008 13:52:29 -0700
Matthew Wilcox <matthew@xxxxxx> wrote:
> On Fri, Jan 11, 2008 at 09:50:46PM +0100, Krzysztof Helt wrote:
> > + BUG_ON(!sym_data->io_reset);
> > + sym_data->io_reset = &eh_done;
>
> Isn't that BUG_ON still the wrong sense?
>
Unfortunately yes.
Here is the 3rd version of the patch. Thank you for your patience.
---
From: Krzysztof Helt <krzysztof.h1@xxxxx>
This patch fixes two bugs pointed by James Bottomley:
1. the if (!sym_data->io_reset). That variable is only ever filled
by a stack based completion. If we find it non empty it means
this code has been entered twice and we have a severe problem,
so that should just become a BUG_ON(!sym_data->io_reset).
2. sym_data->io_reset should be set to NULL before the routine is
exited otherwise the PCI recovery code could end up completing
what will be a bogus pointer into the stack.
Big thanks to James Bottomley for help with the patch.
Signed-off-by: Krzysztof Helt <krzysztof.h1@xxxx>
---
diff -urp linux-ref/drivers/scsi/sym53c8xx_2/sym_glue.c linux-new/drivers/scsi/sym53c8xx_2/sym_glue.c
--- linux-ref/drivers/scsi/sym53c8xx_2/sym_glue.c 2007-12-23 20:39:44.000000000 +0100
+++ linux-new/drivers/scsi/sym53c8xx_2/sym_glue.c 2008-01-10 21:26:32.000000000 +0100
@@ -609,22 +609,24 @@ static int sym_eh_handler(int op, char *
*/
#define WAIT_FOR_PCI_RECOVERY 35
if (pci_channel_offline(pdev)) {
- struct completion *io_reset;
int finished_reset = 0;
init_completion(&eh_done);
spin_lock_irq(shost->host_lock);
/* Make sure we didn't race */
if (pci_channel_offline(pdev)) {
- if (!sym_data->io_reset)
- sym_data->io_reset = &eh_done;
- io_reset = sym_data->io_reset;
+ BUG_ON(sym_data->io_reset);
+ sym_data->io_reset = &eh_done;
} else {
finished_reset = 1;
}
spin_unlock_irq(shost->host_lock);
if (!finished_reset)
- finished_reset = wait_for_completion_timeout(io_reset,
+ finished_reset = wait_for_completion_timeout
+ (sym_data->io_reset,
WAIT_FOR_PCI_RECOVERY*HZ);
+ spin_lock_irq(shost->host_lock);
+ sym_data->io_reset = NULL;
+ spin_unlock_irq(shost->host_lock);
if (!finished_reset)
return SCSI_FAILED;
}
@@ -1879,7 +1881,6 @@ static void sym2_io_resume(struct pci_de
spin_lock_irq(shost->host_lock);
if (sym_data->io_reset)
complete_all(sym_data->io_reset);
- sym_data->io_reset = NULL;
spin_unlock_irq(shost->host_lock);
}
----------------------------------------------------------------------
Rozdajemy nagrody!
Sprawdz >> http://link.interia.pl/f1cbf
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
