Fix initialization sequence in SunGEM driver

The patch below alters the SunGEM driver so that the net_device callback pointers are initialized before calling register_netdev(), rather than after.

I think that would always have been a bug, but its effects have become worse in recent kernels: the fact that get_stats is NULL when register_netdev() is called means that the netstat_group sysfs attributes are not registered. That in turn means that we get an oops on module remove as it attempts to deregister those attributes (dir->d_inode is NULL in sysfs_hash_and_remove()).

diff -urN linuxppc-2.5-benh/drivers/net/sungem.c linux-zax/drivers/net/sungem.c
@@ -2757,6 +2757,19 @@
 	if (gem_get_device_address(gp))
 		goto err_out_free_consistent;
 
+	dev->open = gem_open;
+	dev->stop = gem_close;
+	dev->hard_start_xmit = gem_start_xmit;
+	dev->get_stats = gem_get_stats;
+	dev->set_multicast_list = gem_set_multicast;
+	dev->do_ioctl = gem_ioctl;
+	dev->ethtool_ops = &gem_ethtool_ops;
+	dev->tx_timeout = gem_tx_timeout;
+	dev->watchdog_timeo = 5 * HZ;
+	dev->change_mtu = gem_change_mtu;
+	dev->irq = pdev->irq;
+	dev->dma = 0;
+
 	if (register_netdev(dev)) {
 		printk(KERN_ERR PFX "Cannot register net device, "
 		       "aborting.\n");

@@ -2785,19 +2798,6 @@
 
 	pci_set_drvdata(pdev, dev);
 
-	dev->open = gem_open;
-	dev->stop = gem_close;
-	dev->hard_start_xmit = gem_start_xmit;
-	dev->get_stats = gem_get_stats;
-	dev->set_multicast_list = gem_set_multicast;
-	dev->do_ioctl = gem_ioctl;
-	dev->ethtool_ops = &gem_ethtool_ops;
-	dev->tx_timeout = gem_tx_timeout;
-	dev->watchdog_timeo = 5 * HZ;
-	dev->change_mtu = gem_change_mtu;
-	dev->irq = pdev->irq;
-	dev->dma = 0;
-
 	/* GEM can do it all... */
 	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
 	if (pci_using_dac)

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Applied, thanks David.

But in the future please send net driver fixes to [email protected] with either Jeff Garzik or myself CC:'d? Thanks.

I also would absolutely not classify your patch as "trivial". It's not like a missing semi-colon or a comment typo, someone with real understanding of how this stuff works would need to review this patch.

Based just upon the kinds of networking patches Linus has forwarded to me today, I can duly say that far too much stuff is being sent as "[TRIVIAL]" which is not.

In message <[email protected]> you write:

Perhaps. Certainly I give more slack to a trusted source, and since I end up just sending it to the maintainer anyway, I don't usually see too much harm in holding it. Perhaps I need a non-trivial patch monkey 8)

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.