Message-ID: <394E2616.C25F8376@uow.edu.au>
Date:   Mon, 19 Jun 2000 23:54:30 +1000
From:   Andrew Morton <andrewm@uow.edu.au>
MIME-Version: 1.0
To:     "netdev@oss.sgi.com" <netdev@oss.sgi.com>
Subject: modular net drivers
Content-Type: multipart/mixed; boundary="------------41373E28E27635F9DC73A463"
Sender: owner-netdev@oss.sgi.com
Precedence: bulk

This is a multi-part message in MIME format.
--------------41373E28E27635F9DC73A463
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

As you may have noticed, Al Viro is running around the kernel getting
rid of MOD_INC_USE_COUNT and MOD_DEC_USE_COUNT.  His long-term plan
is to remove MOD_INC_USE_COUNT and MOD_DEC_USE_COUNT completely.
I'm looking into the changes required for the net drivers.

I have attached here a patch which implements this.  It works with
3c59x.c, tested as a module and statically linked.  Also compiles
and works with CONFIG_MODULES turned off.

The implications of this are:

- All 2.4-only netdrivers can have all their MOD_DEC and MOD_INC calls
  removed.  All that twisty logic to keep track of the counts can be
  tossed.  A single

	SET_NETDEVICE_OWNER(dev);

  will be needed when the netdevice fields are being filled in.

- Drivers which support 2.2 will need to retain their MOD_INC/MOD_DEC
  macros and they will need to be given a SET_NETDEVICE_OWNER(). They
  will need to do this:

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
#define SET_NETDEVICE_OWNER(d)
#else
#define MOD_INC_USE_COUNT
#define MOD_DEC_USE_COUNT
#endif


A couple of sticky drivers which Al has identified are com20020_cs.c
and hysdn_net.c.  These both call dev->stop() in bizarre ways and
should be repaired anyway - they're either trying to bypass
notification and dev_clear_fastroute() or they're trying hard to
leak module refcounts and crash the machine.  They can continue
to use the legacy MOD_DEC and MOD_INC calls, but only until
these are tossed out altogether.

Al suggested that the SET_NETDEVICE_OWNER(dev) functionality
could be embedded within ether_setup() but I think it's better
open-coded in this manner - a few drivers (eg, shaper.c) don't
use ether_setup.

If/when this patch goes in it will NOT require that all netdevice
drivers be edited at the same time.  We can migrate them gradually.

Comments?  Gnashing of teeth?
--------------41373E28E27635F9DC73A463
Content-Type: text/plain; charset=us-ascii;
 name="netdevice-modules.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="netdevice-modules.patch"

--- linux-2.4.0-test1-ac21/include/linux/netdevice.h	Mon Jun 19 16:47:53 2000
+++ linux-akpm/include/linux/netdevice.h	Mon Jun 19 22:10:09 2000
@@ -136,6 +136,11 @@
 struct neigh_parms;
 struct sk_buff;
 
+/* Centralised module refcounting for netdevices */
+struct module;
+#define SET_NETDEVICE_OWNER(dev)	\
+	do { dev->owner = THIS_MODULE; } while (0)
+
 struct netif_rx_stats
 {
 	unsigned total;
@@ -372,6 +377,9 @@
 						     unsigned char *haddr);
 	int			(*neigh_setup)(struct net_device *dev, struct neigh_parms *);
 	int			(*accept_fastpath)(struct net_device *, struct dst_entry*);
+
+	/* open/release and usage marking */
+	struct module *owner;
 
 	/* bridge stuff */
 	struct net_bridge_port	*br_port;
--- linux-2.4.0-test1-ac21/net/core/dev.c	Mon Jun 19 16:47:54 2000
+++ linux-akpm/net/core/dev.c	Mon Jun 19 21:28:14 2000
@@ -89,6 +89,7 @@
 #include <net/profile.h>
 #include <linux/init.h>
 #include <linux/kmod.h>
+#include <linux/module.h>
 #if defined(CONFIG_NET_RADIO) || defined(CONFIG_NET_PCMCIA_RADIO)
 #include <linux/wireless.h>		/* Note : will define WIRELESS_EXT */
 #endif	/* CONFIG_NET_RADIO || CONFIG_NET_PCMCIA_RADIO */
@@ -664,8 +665,13 @@
 	 *	Call device private open method
 	 */
 	 
-	if (dev->open) 
+	if (dev->open) {
+		if (dev->owner)
+			__MOD_INC_USE_COUNT(dev->owner);
   		ret = dev->open(dev);
+		if (ret != 0 && dev->owner)
+			__MOD_DEC_USE_COUNT(dev->owner);
+	}
 
 	/*
 	 *	If it went open OK then:
@@ -780,6 +786,13 @@
 	 *	Tell people we are down
 	 */
 	notifier_call_chain(&netdev_chain, NETDEV_DOWN, dev);
+
+	/*
+	 * Drop the module refcount
+	 */
+	if (dev->owner) {
+		__MOD_DEC_USE_COUNT(dev->owner);
+	}
 
 	return(0);
 }

--------------41373E28E27635F9DC73A463--