[PATCH] Prevent PCI driver registration failure oopsing

[PATCH] Prevent PCI driver registration failure oopsing

[Russell King]

Greg,

As discussed about six or so months ago, we agreed to hold off this
patch until fairly late, due to its ability to catch duplicate PCI
driver names.  Please note that I haven't attempted to reproduce the
problem with recent kernels, and that all ARM kernel patches released
since then have had this patch in.

I'm guessing this will actually be 2.6.1 material since it probably
doesn't show for PCI drivers which are part of the kernel tree.


If pci_register_driver fails, the register the PCI driver structure
will not be registered with the driver model.  pci_register_driver
returns with negative value, and we then attempt to unregister the
driver structure.  This leads to an oops in the driver model.

The driver model does not return the number of devices it successfully
bound the driver to, and neither does pci_register_driver() return
this information.

Therefore, all of the code below is redundant.

(There's a little redundancy left in drivers/pci/pci-driver.c but it
is harmless unlike this block.)

--- orig/include/linux/pci.h	Thu Mar 13 14:24:56 2003
+++ linux/include/linux/pci.h	Wed Mar 12 19:37:41 2003
@@ -768,26 +768,7 @@
 {
 	int rc = pci_register_driver (drv);
 
-	if (rc > 0)
-		return 0;
-
-	/* iff CONFIG_HOTPLUG and built into kernel, we should
-	 * leave the driver around for future hotplug events.
-	 * For the module case, a hotplug daemon of some sort
-	 * should load a module in response to an insert event. */
-#if defined(CONFIG_HOTPLUG) && !defined(MODULE)
-	if (rc == 0)
-		return 0;
-#else
-	if (rc == 0)
-		rc = -ENODEV;		
-#endif
-
-	/* if we get here, we need to clean up pci driver instance
-	 * and return some sort of error */
-	pci_unregister_driver (drv);
-	
-	return rc;
+	return rc < 0 ? : 0;
 }
 
 /*


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Prevent PCI driver registration failure oopsing

[Andreas Schwab]

Russell King <rmk+lkml@arm.linux.org.uk> writes:

> --- orig/include/linux/pci.h	Thu Mar 13 14:24:56 2003
> +++ linux/include/linux/pci.h	Wed Mar 12 19:37:41 2003
> @@ -768,26 +768,7 @@
>  {
>  	int rc = pci_register_driver (drv);
>  
> -	if (rc > 0)
> -		return 0;
> -
> -	/* iff CONFIG_HOTPLUG and built into kernel, we should
> -	 * leave the driver around for future hotplug events.
> -	 * For the module case, a hotplug daemon of some sort
> -	 * should load a module in response to an insert event. */
> -#if defined(CONFIG_HOTPLUG) && !defined(MODULE)
> -	if (rc == 0)
> -		return 0;
> -#else
> -	if (rc == 0)
> -		rc = -ENODEV;		
> -#endif
> -
> -	/* if we get here, we need to clean up pci driver instance
> -	 * and return some sort of error */
> -	pci_unregister_driver (drv);
> -	
> -	return rc;
> +	return rc < 0 ? : 0;

Are you sure you want to return 1 if rc < 0?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Prevent PCI driver registration failure oopsing

[Russell King]

On Tue, Oct 28, 2003 at 11:48:05AM +0100, Andreas Schwab wrote:
> Russell King <rmk+lkml@arm.linux.org.uk> writes:
> > +	return rc < 0 ? : 0;
> 
> Are you sure you want to return 1 if rc < 0?

Argh.  Definitely not.  Thanks for spotting that.

--- orig/include/linux/pci.h	Thu Mar 13 14:24:56 2003
+++ linux/include/linux/pci.h	Wed Mar 12 19:37:41 2003
@@ -768,26 +768,7 @@
 {
 	int rc = pci_register_driver (drv);
 
-	if (rc > 0)
-		return 0;
-
-	/* iff CONFIG_HOTPLUG and built into kernel, we should
-	 * leave the driver around for future hotplug events.
-	 * For the module case, a hotplug daemon of some sort
-	 * should load a module in response to an insert event. */
-#if defined(CONFIG_HOTPLUG) && !defined(MODULE)
-	if (rc == 0)
-		return 0;
-#else
-	if (rc == 0)
-		rc = -ENODEV;		
-#endif
-
-	/* if we get here, we need to clean up pci driver instance
-	 * and return some sort of error */
-	pci_unregister_driver (drv);
-	
-	return rc;
+	return rc < 0 ? rc : 0;
 }
 
 /*



-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Prevent PCI driver registration failure oopsing

[Greg KH]

On Tue, Oct 28, 2003 at 10:51:26AM +0000, Russell King wrote:
> On Tue, Oct 28, 2003 at 11:48:05AM +0100, Andreas Schwab wrote:
> > Russell King <rmk+lkml@arm.linux.org.uk> writes:
> > > +	return rc < 0 ? : 0;
> > 
> > Are you sure you want to return 1 if rc < 0?
> 
> Argh.  Definitely not.  Thanks for spotting that.
> 
> --- orig/include/linux/pci.h	Thu Mar 13 14:24:56 2003
> +++ linux/include/linux/pci.h	Wed Mar 12 19:37:41 2003

Thanks, I've applied this and will send it to Linus when 2.6.0 is out.

greg k-h