netdev
[Top] [All Lists]

Re: modular net drivers, take 2

To: Andi Kleen <ak@xxxxxx>
Subject: Re: modular net drivers, take 2
From: Andi Kleen <ak@xxxxxx>
Date: Tue, 20 Jun 2000 12:46:13 +0200
Cc: Keith Owens <kaos@xxxxxxxxxx>, Donald Becker <becker@xxxxxxxxx>, Andrew Morton <andrewm@xxxxxxxxxx>, "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>
In-reply-to: <20000620124146.A1375@fred.muc.de>; from Andi Kleen on Tue, Jun 20, 2000 at 12:41:46PM +0200
References: <Pine.LNX.4.10.10006192011230.26261-100000@vaio.greennet> <3292.961460999@kao2.melbourne.sgi.com> <20000620124146.A1375@fred.muc.de>
Sender: owner-netdev@xxxxxxxxxxx
On Tue, Jun 20, 2000 at 12:41:46PM +0200, Andi Kleen wrote:
> On Tue, Jun 20, 2000 at 02:33:47AM +0200, Keith Owens wrote:
> > >> - 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);
> > >
> > >"Twisty logic"?  Most of the drivers have straight-forward use counts.
> > >How is this new method any simpler?  If anything, it seems to be more
> > >complex without any obvious benefit.
> > 
> > There are inherent unload races when code that lives inside a module
> > tries to adjust the use count on that module.  To the extent that the
> > code pages can be deleted underneath the code that is executing!
> > Module use counts need to be set before entering the module, not after
> > the module code has started executing.
> 
> At least for open/close() that is not true -- rtnl_lock() protects against
> that. For that there are the same rules as in 2.2 (INC before first sleep,
> DEC after last sleep) 

Ok, there is still a small race with the actual module unload. I think
the cleanest solution is to let open/close run in the big kernel lock.
They are not performance critical anyways.

Comments ? 

Looks much prefereable than changing all the network drivers at this
stage at least.


-Andi

--- linux/net/core/dev.c-devlock        Tue Jun 20 00:58:33 2000
+++ linux/net/core/dev.c        Tue Jun 20 12:38:25 2000
@@ -89,6 +89,7 @@
 #include <net/profile.h>
 #include <linux/init.h>
 #include <linux/kmod.h>
+#include <linux/smp_lock.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 */
@@ -2091,10 +2092,12 @@
                case SIOCSIFNAME:
                        if (!capable(CAP_NET_ADMIN))
                                return -EPERM;
+                       lock_kernel(); 
                        dev_load(ifr.ifr_name);
                        rtnl_lock();
                        ret = dev_ifsioc(&ifr, cmd);
                        rtnl_unlock();
+                       unlock_kernel(); 
                        return ret;
        
                case SIOCGIFMEM:

<Prev in Thread] Current Thread [Next in Thread>