Re: [patch] drivers/ide/pci/alim15x3.c SMP fix

[Ingo Molnar]

* Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:

> On Iau, 2005-09-01 at 09:24 +0200, Ingo Molnar wrote:
> > is this the right way to fix the UP assumption below?
> 
> Probably not. The ide_lock may already be held (randomly depending on 
> the code path) at the point we retune a drive on error. Actually you 
> probably crash before this anyway.
> 
> The ALi code in question was written knowing the system would be
> uniprocessor and making various related assumptions. You also have to
> get this locking right just to make it more fun - loading the timings
> for one channel while another is doing I/O corrupts your data silently
> in some cases. Fixing the ide_lock to be consistent in usage when the
> tuning calls are made (ie fix the reset path and other offenders) might
> be possible and would make using ide_lock ok, but it would still be
> wrong with pre-emption and/or SMP.

so perhaps part of the solution would be to do the initialization under 
the IDE lock, via the patch below? It boots fine on my box so the basic 
codepaths seem to be OK. Then the retuning codepaths need to be checked 
to make sure they are holding the IDE lock.

	Ingo

Index: linux/drivers/ide/setup-pci.c
===================================================================
--- linux.orig/drivers/ide/setup-pci.c
+++ linux/drivers/ide/setup-pci.c
@@ -665,8 +665,11 @@ static int do_ide_setup_pci_device(struc
 {
 	static ata_index_t ata_index = { .b = { .low = 0xff, .high = 0xff } };
 	int tried_config = 0;
+	unsigned long flags;
 	int pciirq, ret;
 
+	spin_lock_irqsave(&ide_lock, flags);
+
 	ret = ide_setup_pci_controller(dev, d, noisy, &tried_config);
 	if (ret < 0)
 		goto out;
@@ -721,6 +724,8 @@ static int do_ide_setup_pci_device(struc
 	*index = ata_index;
 	ide_pci_setup_ports(dev, d, pciirq, index);
 out:
+	spin_unlock_irqrestore(&ide_lock, flags);
+
 	return ret;
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/