Re: [PATCH] to deal with bad dev->refcnt in unregister_netdevice()

[Thomas Hood]

Sorry, but it turns out that the bug is not completely
fixed by the change that acme made.  With the change,
ifup-ing and if-downing eth0 with the ipx module loaded
no longer reduces eth0's refcnt to an indefinitely low
(larger and larger negative) number.  However if the ipx
module is loaded first and ipx configured on eth0, and
then the network card inserted and "ifconfig eth0 up" done,
and then "ifconfig eth0 down" done, then once again the
refcnt is too low, so that when I try to "cardctl eject"
my ethernet card, "modprobe -r xirc2ps_cs" hangs up.
This whole business of refcnts needs to be thought 
through more carefully.

> This bug was fixed by "acme" in 2.4.1-ac10.  :)
> The ipx driver now increments refcnt on NETDEV_UP to
> match downing the interface on NETDEV_DOWN.
> 
> Thanks all.
> Thomas
> 
> 
> > Okay, I now know the cause of this problem.  It's a bug in
> > the ipx driver.  (This also closes pcmcia-cs bug #126563.)
> > 
> > Through copious use of printk()s I was able to track down
> > the point at which the dev->refcnt for my card was being
> > erroneously decremented.  It was being erroneously decremented
> > by "notifier_call_chain(&netdev_chain, NETDEV_DOWN, dev);"
> > near the end of the "dev_close()" function in net/core/dev.c.
> > This indicated that one of the registered notifiers
> > was doing an improper "dev_put()".  I rmmod-ed the ipx module
> > from my system and the problem magically disappeared.  The
> > refcnt is no longer inappropriately decremented and there
> > are no more inappropriate calls to netdev_finish_unregister()
> > in the absence of a prior call to unregister_netdevice()
> > (which is what resulted in the "Freeing alive device" messages).
> > 
> > Where is the bug?  It is in the ipx driver.  When I configure
> > eth0 for ipx, the device gets added to the ipx driver's linked
> > list of devices headed at "ipx_interfaces".  The ipx driver
> > registers the following function to be notified of net events.
> > It's clear that the ipx driver will do a __ipxitf_put (which
> > decrements dev-refcnt and does a dev_put() on dev) every time
> > eth0 is taken down with "ifconfig eth0 down"!  That would be
> > okay, I guess, if the opposite were done on an "ifconfig eth0 up"
> > but it isn't.  This needs to be fixed somehow.  I'll leave it
> > up to the maintainers and other gurus to figure out how.  In
> > the meantime I'll just avoid using ipx.
> > 
> > ---------------- net/ipx/af_ipx.c ---------------------------
> > static int ipxitf_device_event(struct notifier_block *notifier,
> >                                 unsigned long event, void *ptr)
> > {
> >         struct net_device *dev = ptr;
> >         ipx_interface *i, *tmp;
> > 
> >         if (event != NETDEV_DOWN)
> >                 return NOTIFY_DONE;
> > 
> >         spin_lock_bh(&ipx_interfaces_lock);
> >         for (i = ipx_interfaces; i;) {
> >                 tmp = i->if_next;
> >                 if (i->if_dev == dev)
> >                         __ipxitf_put(i);
> >                 i = tmp;
> > 
> >         }
> >         spin_unlock_bh(&ipx_interfaces_lock);
> >         return NOTIFY_DONE;
> > }
> > --------------------------------------------------------------
>