Re: [BUG] drivers/net/ioc3_eth.c in 2.5

Re: [BUG] drivers/net/ioc3_eth.c in 2.5

[Stephen Hemminger]

On Sat, 07 Jun 2003 01:30:10 -0700 (PDT)
"David S. Miller" <davem@redhat.com> wrote:

>    From: Stephen Hemminger <shemminger@osdl.org>
>    Date: Fri, 6 Jun 2003 16:16:58 -0700
> 
>    This driver never calls unregister in it's module exit function:
>    
>    static void __exit ioc3_cleanup_module(void)
>    {
>    	pci_unregister_driver(&ioc3_driver);
>    }
>    
> pci_unregister_driver() invokes, for each PCI driver instance
> registered, the ->remove() method for that driver.
> 
> What is the problem?  tg3.c and many other drivers work exactly
> like this, using the PCI registry mechanism as a helper to do
> all the grunge work or device iteration.

pci_unregister_driver does the iteration but not the net device
cleanup.


The problem is the driver never calls unregister_netdev, it just free's
the device structure.  If this ever happens, the net device list would be corrupt.
Don't have the hardware to actually do this though.

Looks like the right fix is:
--- ioc3-eth.c.orig	2003-06-09 10:05:45.000000000 -0700
+++ ioc3-eth.c	2003-06-09 10:04:45.000000000 -0700
@@ -1614,6 +1614,7 @@ static void __devexit ioc3_remove_one (s
 	struct ioc3 *ioc3 = ip->regs;
 
 	iounmap(ioc3);
+	unregister_netdev(dev);
 	pci_release_regions(pdev);
 	kfree(dev);
 }

Re: [BUG] drivers/net/ioc3_eth.c in 2.5

[David S. Miller]

   From: Stephen Hemminger <shemminger@osdl.org>
   Date: Mon, 9 Jun 2003 10:10:18 -0700
   
   pci_unregister_driver does the iteration but not the net device
   cleanup.
   
You're absolutely right.

Re: [BUG] drivers/net/ioc3_eth.c in 2.5

[Jeff Garzik]

On Mon, Jun 09, 2003 at 10:10:18AM -0700, Stephen Hemminger wrote:
> Looks like the right fix is:
> --- ioc3-eth.c.orig	2003-06-09 10:05:45.000000000 -0700
> +++ ioc3-eth.c	2003-06-09 10:04:45.000000000 -0700
> @@ -1614,6 +1614,7 @@ static void __devexit ioc3_remove_one (s
>  	struct ioc3 *ioc3 = ip->regs;
>  
>  	iounmap(ioc3);
> +	unregister_netdev(dev);
>  	pci_release_regions(pdev);
>  	kfree(dev);
>  }

You want to unregister before iounmap.

	Jeff

Re: [BUG] drivers/net/ioc3_eth.c in 2.5

[Stephen Hemminger]

On Mon, 9 Jun 2003 13:12:24 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:

> On Mon, Jun 09, 2003 at 10:10:18AM -0700, Stephen Hemminger wrote:
> > Looks like the right fix is:
> > --- ioc3-eth.c.orig	2003-06-09 10:05:45.000000000 -0700
> > +++ ioc3-eth.c	2003-06-09 10:04:45.000000000 -0700
> > @@ -1614,6 +1614,7 @@ static void __devexit ioc3_remove_one (s
> >  	struct ioc3 *ioc3 = ip->regs;
> >  
> >  	iounmap(ioc3);
> > +	unregister_netdev(dev);
> >  	pci_release_regions(pdev);
> >  	kfree(dev);
> >  }
> 
> You want to unregister before iounmap.
> 
> 	Jeff
> 
> 
Okay:
--- ioc3-eth.c.orig	2003-06-09 10:05:45.000000000 -0700
+++ ioc3-eth.c	2003-06-09 11:08:01.000000000 -0700
@@ -1613,6 +1613,7 @@ static void __devexit ioc3_remove_one (s
 	struct ioc3_private *ip = dev->priv;
 	struct ioc3 *ioc3 = ip->regs;
 
+	unregister_netdev(dev);
 	iounmap(ioc3);
 	pci_release_regions(pdev);
 	kfree(dev);

Re: [BUG] drivers/net/ioc3_eth.c in 2.5

[Ralf Baechle]

On Mon, Jun 09, 2003 at 11:08:55AM -0700, Stephen Hemminger wrote:

> Okay:
> --- ioc3-eth.c.orig	2003-06-09 10:05:45.000000000 -0700
> +++ ioc3-eth.c	2003-06-09 11:08:01.000000000 -0700
> @@ -1613,6 +1613,7 @@ static void __devexit ioc3_remove_one (s
>  	struct ioc3_private *ip = dev->priv;
>  	struct ioc3 *ioc3 = ip->regs;
>  
> +	unregister_netdev(dev);
>  	iounmap(ioc3);
>  	pci_release_regions(pdev);
>  	kfree(dev);

Thanks, applied.  Same is also needed for 2.4.

  Ralf