All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [BUG] drivers/net/ioc3_eth.c in 2.5
  2003-06-09 17:10   ` [BUG] drivers/net/ioc3_eth.c in 2.5 Stephen Hemminger
@ 2003-06-09 17:09     ` David S. Miller
  2003-06-09 17:12     ` Jeff Garzik
  1 sibling, 0 replies; 5+ messages in thread
From: David S. Miller @ 2003-06-09 17:09 UTC (permalink / raw)
  To: shemminger; +Cc: ralf, jgarzik, netdev

   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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] drivers/net/ioc3_eth.c in 2.5
       [not found] ` <20030607.013010.116359540.davem@redhat.com>
@ 2003-06-09 17:10   ` Stephen Hemminger
  2003-06-09 17:09     ` David S. Miller
  2003-06-09 17:12     ` Jeff Garzik
  0 siblings, 2 replies; 5+ messages in thread
From: Stephen Hemminger @ 2003-06-09 17:10 UTC (permalink / raw)
  To: David S. Miller; +Cc: ralf, jgarzik, netdev

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);
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] drivers/net/ioc3_eth.c in 2.5
  2003-06-09 17:10   ` [BUG] drivers/net/ioc3_eth.c in 2.5 Stephen Hemminger
  2003-06-09 17:09     ` David S. Miller
@ 2003-06-09 17:12     ` Jeff Garzik
  2003-06-09 18:08       ` Stephen Hemminger
  1 sibling, 1 reply; 5+ messages in thread
From: Jeff Garzik @ 2003-06-09 17:12 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, ralf, netdev

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] drivers/net/ioc3_eth.c in 2.5
  2003-06-09 17:12     ` Jeff Garzik
@ 2003-06-09 18:08       ` Stephen Hemminger
  2003-06-11 11:50         ` Ralf Baechle
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2003-06-09 18:08 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: davem, ralf, netdev

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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] drivers/net/ioc3_eth.c in 2.5
  2003-06-09 18:08       ` Stephen Hemminger
@ 2003-06-11 11:50         ` Ralf Baechle
  0 siblings, 0 replies; 5+ messages in thread
From: Ralf Baechle @ 2003-06-11 11:50 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jeff Garzik, davem, netdev

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2003-06-11 11:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20030606161658.1f01b8f9.shemminger@osdl.org>
     [not found] ` <20030607.013010.116359540.davem@redhat.com>
2003-06-09 17:10   ` [BUG] drivers/net/ioc3_eth.c in 2.5 Stephen Hemminger
2003-06-09 17:09     ` David S. Miller
2003-06-09 17:12     ` Jeff Garzik
2003-06-09 18:08       ` Stephen Hemminger
2003-06-11 11:50         ` Ralf Baechle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.