|To:||Keith Owens <kaos@xxxxxxxxxx>|
|Subject:||Re: modular net drivers, take 2|
|From:||Andrew Morton <andrewm@xxxxxxxxxx>|
|Date:||Wed, 21 Jun 2000 21:06:41 +1000|
|References:||Your message of "Wed, 21 Jun 2000 04:45:19 GMT." <3950485F.AD6F58C6@uow.edu.au> <firstname.lastname@example.org>|
Keith Owens wrote: > > On Wed, 21 Jun 2000 04:45:19 +0000, > Andrew Morton <andrewm@xxxxxxxxxx> wrote: > >Plan M: > > > >sys_delete_module() doesn't do the vfree(). It schedules it for 5 > >seconds in the future. Or provides a mechanism for userland to do this. > > Does not fix the problem where module open() code is running at the > same time as module_exit() code which tears down the kernel structures > that open needs. Freeing the code pages while they are in use is only > one of the possible failure modes for this race. Aaaah. I can see it all clearly now. The fundamental misdesign here is that register_netdevice() and unregister_netdev() are called from within the module constructor and destructor functions. You see, right now a module "owns" a number of netdevices. This is wrong. It should be that a module "owns" a driver (pci_driver) and that driver "owns" a number of netdevices: module_init() registers the availability of the driver. refcount remains zero driver *open_module("eepro100") Locates the driver. Increments module refcount. No netdevices open yet. Shold not be called from within module_init! That's the current problem. device *open_driver(driver *) Creates an interface instance. Increments an 'opencount' in the driver. Critically: register_netdevice is called olny now. close_device(device *) Unregister_netdev() close_driver(driver *) Fails if any devices are open. Decrements module refcount to zero. Now, no drivers are registered, so nobody can access "eth0", or device->open or anything else. module_exit() Unregisters the driver availability. Safe. We just need to prevent module_exit() from racing against open_driver(). Just a spinlock. Too late for all of this.
|<Prev in Thread]||Current Thread||[Next in Thread>|