Re: [patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
On Mer, 2005-09-07 at 10:50 -0700, Ravikiran G Thirumalai wrote: > Then the change to piix controller in my patchset is bad, How about changing > the ide_lock to per-driver lock in this case? Locking for rest of the > controllers in the system is left equivalent to what ide_lock did earlier.. Thats what we did in Fedora Core. Its neccessary because the state of ide_lock is undefined when the tuning functions get called and the base kernel didn't handle this well - we had various reports from users of hangs as a result. Its not a full fix, really PIIX needs to wrap the entire command issue with a semaphore or similar to avoid the error handling (CRC error) breaking the driver. Unfortunately we do the CRC error handling and speed change in an IRQ handler polled so I've never seen how we could fix that without rewriting the entire error recovery code to work like the scsi layer (in a thread). > > fixing the IDE layer locking properly (or forward porting my patches and > > then fixing them for all the refcounting changes and other stuff done > > since). > > Can you please point me to the patchset... 2.6.11-ac kernels. I will send you (off list) the last full version of the fixes. Part of what it fixes (especially the proc file locking changes) are mostly handled now by the reworking of the IDE code to use refcounts that Bartlomiej did, and which is definitely the better way to fix it. Alan - 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/
Re: [patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
On Wed, Sep 07, 2005 at 06:06:23PM +0100, Alan Cox wrote: > On Maw, 2005-09-06 at 16:44 -0700, Ravikiran G Thirumalai wrote: > > Patch to convert piix driver to use per-driver/hwgroup lock and kill > > ide_lock. In the case of piix, hwgroup->lock should be sufficient. > > PIIX requires that both channels are quiescent when retuning in some > cases. It wasn't totally safe before, its now totally broken. Start by Then the change to piix controller in my patchset is bad, How about changing the ide_lock to per-driver lock in this case? Locking for rest of the controllers in the system is left equivalent to what ide_lock did earlier.. > fixing the IDE layer locking properly (or forward porting my patches and > then fixing them for all the refcounting changes and other stuff done > since). Can you please point me to the patchset... Thanks, Kiran - 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/
Re: [patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
On Wed, Sep 07, 2005 at 06:06:23PM +0100, Alan Cox wrote: On Maw, 2005-09-06 at 16:44 -0700, Ravikiran G Thirumalai wrote: Patch to convert piix driver to use per-driver/hwgroup lock and kill ide_lock. In the case of piix, hwgroup-lock should be sufficient. PIIX requires that both channels are quiescent when retuning in some cases. It wasn't totally safe before, its now totally broken. Start by Then the change to piix controller in my patchset is bad, How about changing the ide_lock to per-driver lock in this case? Locking for rest of the controllers in the system is left equivalent to what ide_lock did earlier.. fixing the IDE layer locking properly (or forward porting my patches and then fixing them for all the refcounting changes and other stuff done since). Can you please point me to the patchset... Thanks, Kiran - 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/
Re: [patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
On Mer, 2005-09-07 at 10:50 -0700, Ravikiran G Thirumalai wrote: Then the change to piix controller in my patchset is bad, How about changing the ide_lock to per-driver lock in this case? Locking for rest of the controllers in the system is left equivalent to what ide_lock did earlier.. Thats what we did in Fedora Core. Its neccessary because the state of ide_lock is undefined when the tuning functions get called and the base kernel didn't handle this well - we had various reports from users of hangs as a result. Its not a full fix, really PIIX needs to wrap the entire command issue with a semaphore or similar to avoid the error handling (CRC error) breaking the driver. Unfortunately we do the CRC error handling and speed change in an IRQ handler polled so I've never seen how we could fix that without rewriting the entire error recovery code to work like the scsi layer (in a thread). fixing the IDE layer locking properly (or forward porting my patches and then fixing them for all the refcounting changes and other stuff done since). Can you please point me to the patchset... 2.6.11-ac kernels. I will send you (off list) the last full version of the fixes. Part of what it fixes (especially the proc file locking changes) are mostly handled now by the reworking of the IDE code to use refcounts that Bartlomiej did, and which is definitely the better way to fix it. Alan - 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/
[patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
Patch to convert piix driver to use per-driver/hwgroup lock and kill
ide_lock. In the case of piix, hwgroup->lock should be sufficient.
Signed-off-by: Ravikiran Thirumalai <[EMAIL PROTECTED]>
Index: linux-2.6.13/drivers/ide/pci/piix.c
===
--- linux-2.6.13.orig/drivers/ide/pci/piix.c2005-09-06 12:00:25.0
-0700
+++ linux-2.6.13/drivers/ide/pci/piix.c 2005-09-06 13:22:49.0 -0700
@@ -231,7 +231,6 @@
pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
spin_lock_irqsave(>lock, flags);
- spin_lock(_lock);
pci_read_config_word(dev, master_port, _data);
if (is_slave) {
master_data = master_data | 0x4000;
@@ -251,7 +250,6 @@
pci_write_config_word(dev, master_port, master_data);
if (is_slave)
pci_write_config_byte(dev, slave_port, slave_data);
- spin_unlock(_lock);
spin_unlock_irqrestore(>lock, flags);
}
-
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/
[patch 4/4] ide: Break ide_lock -- remove ide_lock from piix driver
Patch to convert piix driver to use per-driver/hwgroup lock and kill
ide_lock. In the case of piix, hwgroup-lock should be sufficient.
Signed-off-by: Ravikiran Thirumalai [EMAIL PROTECTED]
Index: linux-2.6.13/drivers/ide/pci/piix.c
===
--- linux-2.6.13.orig/drivers/ide/pci/piix.c2005-09-06 12:00:25.0
-0700
+++ linux-2.6.13/drivers/ide/pci/piix.c 2005-09-06 13:22:49.0 -0700
@@ -231,7 +231,6 @@
pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
spin_lock_irqsave(hwgroup-lock, flags);
- spin_lock(ide_lock);
pci_read_config_word(dev, master_port, master_data);
if (is_slave) {
master_data = master_data | 0x4000;
@@ -251,7 +250,6 @@
pci_write_config_word(dev, master_port, master_data);
if (is_slave)
pci_write_config_byte(dev, slave_port, slave_data);
- spin_unlock(ide_lock);
spin_unlock_irqrestore(hwgroup-lock, flags);
}
-
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/
