On Mon, 28 Apr 2003 17:57:19 -0700 (PDT) "David S. Miller" <davem@redhat.com> wrote: > From: Stephen Hemminger <shemminger@osdl.org> > Date: Mon, 28 Apr 2003 16:35:57 -0700 > > So unless dst.c is fixed, there needs to be a rule NEVER use > dev->destructor in network modules. > > I don't understand what the problem is. > > Look, this code in the DST cache merely puts the device (which is > legal from any context) if the device lacks a destructor. > > If it has a destructor, it keeps the reference and once the DST entry > dies the dst->dev is dev_put(). So it does in fact do the "cleanup" > contrary to what you say, this happens during dst entry garbage > collection. > > If the dst has a dst->dev attached, the device has a reference > and thus so does your module which created the device and thus > your module cannot be unloaded. > > So to reiterate, what exactly is the problem? :-) The problem is that the destructor points to code that is in the module, and it doesn't get run during unregister but later when the dst cache GC timer expores. So it is possible for the module to go away before the DST entry is released by dev_put() in the garbage collection (by timer). At that point the destructor points to dead memory. Possible workarounds: * don't use destructor in irda, vlan, bridge, ... Go back to "old way". * make the cache smarter so when it receives a NETDEV_UNREGISTER message it purges right there, rather than waiting for the GC timer. * add a function to dev.c netdev_kfreepriv(struct netdevice *dev) { kfree(dev->priv); } that would be safe since not part of the module. * Split device and private info into two structures. Rather than having: struct xxx_info { ... struct net_device dev; ... }; x = kmalloc(); x->dev.priv = x; xxx_destructor(struct net_device *dev) { kfree(dev->priv); } Do: dev = kmalloc(sizeof(struct net_device)) dev->priv = kmalloc(sizeof(struct xxx_info)); dev->destructor = kfree; unregistering... kfree(dev->priv); dev->priv = NULL; - : send the line "unsubscribe linux-net" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html