This bit us a few kernels ago, and for some reason never made it's way
upstream.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=144743
Kernel panic - not syncing: drivers/ide/pci/piix.c:231:
spin_lock(drivers/ide/ide.c:c03cef28) already locked by driver/ide/ide-iops.c/1153.
From: Alan Cox <[email protected]>
Signed-off-by: Dave Jones <[email protected]>
--- linux-2.6.12/drivers/ide/pci/piix.c~ 2005-07-11 10:23:24.637181320 +0100
+++ linux-2.6.12/drivers/ide/pci/piix.c 2005-07-11 10:23:24.637181320 +0100
@@ -203,6 +203,8 @@
}
}
+static spinlock_t tune_lock = SPIN_LOCK_UNLOCKED;
+
/**
* piix_tune_drive - tune a drive attached to a PIIX
* @drive: drive to tune
@@ -229,7 +231,12 @@
{ 2, 3 }, };
pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
- spin_lock_irqsave(&ide_lock, flags);
+
+ /* Master v slave is synchronized above us but the slave register is
+ shared by the two hwifs so the corner case of two slave timeouts in
+ parallel must be locked */
+
+ spin_lock_irqsave(&tune_lock, flags);
pci_read_config_word(dev, master_port, &master_data);
if (is_slave) {
master_data = master_data | 0x4000;
@@ -249,7 +256,7 @@
pci_write_config_word(dev, master_port, master_data);
if (is_slave)
pci_write_config_byte(dev, slave_port, slave_data);
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irqrestore(&tune_lock, flags);
}
/**
On 2/16/06, Dave Jones <[email protected]> wrote:
> This bit us a few kernels ago, and for some reason never made it's way
> upstream.
Because has never been submitted...
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=144743
> Kernel panic - not syncing: drivers/ide/pci/piix.c:231:
> spin_lock(drivers/ide/ide.c:c03cef28) already locked by driver/ide/ide-iops.c/1153.
Could we get a decent description of the problem and of the patch?
Starting with the short description: it is piix locking being fixed not IDE one.
As this is piix only patch your bugzilla #144768 is not duplicate of
#144743. Also some people reported the problem for atiixp.c
under #144743 (attixp.c has similar locking problem to piix.c).
So either they didn't care to reopen the bug or it was fixed by
some other change it the core IDE code.
> From: Alan Cox <[email protected]>
> Signed-off-by: Dave Jones <[email protected]>
>
> --- linux-2.6.12/drivers/ide/pci/piix.c~ 2005-07-11 10:23:24.637181320 +0100
> +++ linux-2.6.12/drivers/ide/pci/piix.c 2005-07-11 10:23:24.637181320 +0100
> @@ -203,6 +203,8 @@
> }
> }
>
> +static spinlock_t tune_lock = SPIN_LOCK_UNLOCKED;
> +
> /**
> * piix_tune_drive - tune a drive attached to a PIIX
> * @drive: drive to tune
> @@ -229,7 +231,12 @@
> { 2, 3 }, };
>
> pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
> - spin_lock_irqsave(&ide_lock, flags);
> +
> + /* Master v slave is synchronized above us but the slave register is
> + shared by the two hwifs so the corner case of two slave timeouts in
> + parallel must be locked */
> +
> + spin_lock_irqsave(&tune_lock, flags);
> pci_read_config_word(dev, master_port, &master_data);
> if (is_slave) {
> master_data = master_data | 0x4000;
> @@ -249,7 +256,7 @@
> pci_write_config_word(dev, master_port, master_data);
> if (is_slave)
> pci_write_config_byte(dev, slave_port, slave_data);
> - spin_unlock_irqrestore(&ide_lock, flags);
> + spin_unlock_irqrestore(&tune_lock, flags);
> }
>
> /**
On Gwe, 2006-02-17 at 09:57 +0100, Bartlomiej Zolnierkiewicz wrote:
> Could we get a decent description of the problem and of the patch?
Audit the lock state on all entries to the tune function and it
certainly was the case that the old IDE layer could call it with the
lock either already held or not.
Alan
On 2/17/06, Alan Cox <[email protected]> wrote:
> On Gwe, 2006-02-17 at 09:57 +0100, Bartlomiej Zolnierkiewicz wrote:
> > Could we get a decent description of the problem and of the patch?
>
> Audit the lock state on all entries to the tune function and it
> certainly was the case that the old IDE layer could call it with the
> lock either already held or not.
Thank you but this is not a patch description, this is a recipe
for me to spend nice friday's evening staring all over IDE code
and making patch description myself...
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt ?
Bartlomiej
On Gwe, 2006-02-17 at 15:53 +0100, Bartlomiej Zolnierkiewicz wrote:
> Thank you but this is not a patch description, this is a recipe
> for me to spend nice friday's evening staring all over IDE code
> and making patch description myself...
Best I can do. I did the original analysis months and months ago when I
fixed up that locking. Since then there have been enough changes that it
may not be needed and I no longer remember the finer details
> http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt ?
I have better things to do. If you don't want the patch the its not my
problem. I don't even use drivers/ide any more.
Alan
On 2/17/06, Alan Cox <[email protected]> wrote:
> On Gwe, 2006-02-17 at 15:53 +0100, Bartlomiej Zolnierkiewicz wrote:
> > Thank you but this is not a patch description, this is a recipe
> > for me to spend nice friday's evening staring all over IDE code
> > and making patch description myself...
>
> Best I can do. I did the original analysis months and months ago when I
> fixed up that locking. Since then there have been enough changes that it
> may not be needed and I no longer remember the finer details
Even original analysis would be OK.
> > http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt ?
>
> I have better things to do. If you don't want the patch the its not my
> problem. I don't even use drivers/ide any more.
I want a patch but I would like to know more about it.
Bartlomiej