PATCH: Fix crashes with hotplug serverworks

PATCH: Fix crashes with hotplug serverworks

[Alan Cox]

You can't install the base kernel on a Stratus box because of the
overuse of __init. Affects both IDE layers identically. It isn't the
only misuser of __init so more review of other drivers (or fixing
ide_register code to know about hotplug v non-hotplug chipsets) would be
good.

Signed-off-by: Alan Cox <alan@redhat.com>
Original issue found by Stratus and their patch was the inspiration for
this trivial one.

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.12/drivers/ide/pci/serverworks.c linux-2.6.12/drivers/ide/pci/serverworks.c
--- linux.vanilla-2.6.12/drivers/ide/pci/serverworks.c	2005-06-19 11:30:47.000000000 +0100
+++ linux-2.6.12/drivers/ide/pci/serverworks.c	2005-06-20 20:45:50.000000000 +0100
@@ -442,7 +442,7 @@
 	return (dev->irq) ? dev->irq : 0;
 }
 
-static unsigned int __init ata66_svwks_svwks (ide_hwif_t *hwif)
+static unsigned int __devinit ata66_svwks_svwks (ide_hwif_t *hwif)
 {
 	return 1;
 }
@@ -454,7 +454,7 @@
  * Bit 14 clear = primary IDE channel does not have 80-pin cable.
  * Bit 14 set   = primary IDE channel has 80-pin cable.
  */
-static unsigned int __init ata66_svwks_dell (ide_hwif_t *hwif)
+static unsigned int __devinit ata66_svwks_dell (ide_hwif_t *hwif)
 {
 	struct pci_dev *dev = hwif->pci_dev;
 	if (dev->subsystem_vendor == PCI_VENDOR_ID_DELL &&
@@ -472,7 +472,7 @@
  *
  * WARNING: this only works on Alpine hardware!
  */
-static unsigned int __init ata66_svwks_cobalt (ide_hwif_t *hwif)
+static unsigned int __devinit ata66_svwks_cobalt (ide_hwif_t *hwif)
 {
 	struct pci_dev *dev = hwif->pci_dev;
 	if (dev->subsystem_vendor == PCI_VENDOR_ID_SUN &&
@@ -483,7 +483,7 @@
 	return 0;
 }
 
-static unsigned int __init ata66_svwks (ide_hwif_t *hwif)
+static unsigned int __devinit ata66_svwks (ide_hwif_t *hwif)
 {
 	struct pci_dev *dev = hwif->pci_dev;
 
@@ -573,7 +573,7 @@
 	return ide_setup_pci_device(dev, d);
 }
 
-static int __init init_setup_csb6 (struct pci_dev *dev, ide_pci_device_t *d)
+static int __devinit init_setup_csb6 (struct pci_dev *dev, ide_pci_device_t *d)
 {
 	if (!(PCI_FUNC(dev->devfn) & 1)) {
 		d->bootable = NEVER_BOARD;

Re: PATCH: Fix crashes with hotplug serverworks

[Christoph Hellwig]

On Mon, Jun 20, 2005 at 09:21:13PM +0100, Alan Cox wrote:
> You can't install the base kernel on a Stratus box because of the
> overuse of __init. Affects both IDE layers identically. It isn't the
> only misuser of __init so more review of other drivers (or fixing
> ide_register code to know about hotplug v non-hotplug chipsets) would be
> good.

Well, because of fake-hotplug we really need to mark every ->probe routine
and what's called from it __devinit.  Debian has patch to do that forever
but Bart refused to take it.

Re: PATCH: Fix crashes with hotplug serverworks

[Bartlomiej Zolnierkiewicz]

On 6/21/05, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jun 20, 2005 at 09:21:13PM +0100, Alan Cox wrote:
> > You can't install the base kernel on a Stratus box because of the
> > overuse of __init. Affects both IDE layers identically. It isn't the
> > only misuser of __init so more review of other drivers (or fixing
> > ide_register code to know about hotplug v non-hotplug chipsets) would be
> > good.
> 
> Well, because of fake-hotplug we really need to mark every ->probe routine
> and what's called from it __devinit.  Debian has patch to do that forever
> but Bart refused to take it.

If you care to respin your big patch it I'll gladly take it.
It is indeed needed for PCI hotplug.

Bartlomiej

Re: PATCH: Fix crashes with hotplug serverworks

[Alan Cox]

On Maw, 2005-06-21 at 07:52, Christoph Hellwig wrote:
> Well, because of fake-hotplug we really need to mark every ->probe routine
> and what's called from it __devinit.  Debian has patch to do that forever
> but Bart refused to take it.

I'm not surprised from our experience either.

Actually marking all the devices __devinit may be overkill. One approach
that does also seem to work is passing "hotplug yes/no" information when
registering the driver. This is then used to run the ide scan at boot
and avoid registering that driver with the PCI core for hotplug.

A move to __devinit is certainly simpler.

Re: PATCH: Fix crashes with hotplug serverworks

[Bartlomiej Zolnierkiewicz]

On 6/21/05, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Maw, 2005-06-21 at 07:52, Christoph Hellwig wrote:
> > Well, because of fake-hotplug we really need to mark every ->probe routine
> > and what's called from it __devinit.  Debian has patch to do that forever
> > but Bart refused to take it.
> 
> I'm not surprised from our experience either.

Get a life.

> Actually marking all the devices __devinit may be overkill. One approach
> that does also seem to work is passing "hotplug yes/no" information when
> registering the driver. This is then used to run the ide scan at boot
> and avoid registering that driver with the PCI core for hotplug.

* with fake-hotplug driver you always need "hotplug yes"
* these drivers have to be registered with PCI core

> A move to __devinit is certainly simpler.

This is a correct fix.

Bartlomiej

Re: PATCH: Fix crashes with hotplug serverworks

[Alan Cox]

On Maw, 2005-06-21 at 10:59, Bartlomiej Zolnierkiewicz wrote:
> Get a life.

I think that illustrates the problem with IDE management nicely...

Re: PATCH: Fix crashes with hotplug serverworks

[Bartlomiej Zolnierkiewicz]

On 6/21/05, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Maw, 2005-06-21 at 10:59, Bartlomiej Zolnierkiewicz wrote:
> > Get a life.
> 
> I think that illustrates the problem with IDE management nicely...

Taking my words out of full context is a cheap trick.

The real problem here is your PERSONAL AGENDA against me.
I try to stay focused on TECHNICAL ISSUES while you keep on
your cruciate.  You've became really "difficult" to deal with.

BTW I'm going to apply most (all?) of your patches.

Bartlomiej

Re: PATCH: Fix crashes with hotplug serverworks

[Alan Cox]

On Maw, 2005-06-21 at 13:11, Bartlomiej Zolnierkiewicz wrote:
> I try to stay focused on TECHNICAL ISSUES while you keep on
> your cruciate.  You've became really "difficult" to deal with.

I find the same true in the other direction.

> BTW I'm going to apply most (all?) of your patches.

Glad to hear it. If you make any small changes/fixes in the process
please let me know so I can propogate them back into my tree and also
our vendor release tree.

Thanks

Alan