* Fix IDE locking error.
@ 2006-02-16 22:39 Dave Jones
2006-02-17 8:57 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2006-02-16 22:39 UTC (permalink / raw)
To: Linux Kernel; +Cc: linux-ide
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 <alan@redhat.com>
Signed-off-by: Dave Jones <davej@redhat.com>
--- 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);
}
/**
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Fix IDE locking error.
2006-02-16 22:39 Fix IDE locking error Dave Jones
@ 2006-02-17 8:57 ` Bartlomiej Zolnierkiewicz
2006-02-17 14:28 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2006-02-17 8:57 UTC (permalink / raw)
To: Dave Jones, Linux Kernel, linux-ide
On 2/16/06, Dave Jones <davej@redhat.com> 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 <alan@redhat.com>
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> --- 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);
> }
>
> /**
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Fix IDE locking error.
2006-02-17 8:57 ` Bartlomiej Zolnierkiewicz
@ 2006-02-17 14:28 ` Alan Cox
2006-02-17 14:53 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2006-02-17 14:28 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Dave Jones, Linux Kernel, linux-ide
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix IDE locking error.
2006-02-17 14:28 ` Alan Cox
@ 2006-02-17 14:53 ` Bartlomiej Zolnierkiewicz
2006-02-17 15:33 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2006-02-17 14:53 UTC (permalink / raw)
To: Alan Cox; +Cc: Dave Jones, Linux Kernel, linux-ide
On 2/17/06, Alan Cox <alan@lxorguk.ukuu.org.uk> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix IDE locking error.
2006-02-17 14:53 ` Bartlomiej Zolnierkiewicz
@ 2006-02-17 15:33 ` Alan Cox
2006-02-17 15:43 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2006-02-17 15:33 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Dave Jones, Linux Kernel, linux-ide
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix IDE locking error.
2006-02-17 15:33 ` Alan Cox
@ 2006-02-17 15:43 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2006-02-17 15:43 UTC (permalink / raw)
To: Alan Cox; +Cc: Dave Jones, Linux Kernel, linux-ide
On 2/17/06, Alan Cox <alan@lxorguk.ukuu.org.uk> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-02-17 15:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-16 22:39 Fix IDE locking error Dave Jones
2006-02-17 8:57 ` Bartlomiej Zolnierkiewicz
2006-02-17 14:28 ` Alan Cox
2006-02-17 14:53 ` Bartlomiej Zolnierkiewicz
2006-02-17 15:33 ` Alan Cox
2006-02-17 15:43 ` Bartlomiej Zolnierkiewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).