On 02/20, Stephen Hemminger wrote:
>
> On Wed, 21 Feb 2007 01:19:41 +0300
> Oleg Nesterov <[EMAIL PROTECTED]> wrote:
>
> > static void release_nbp(struct kobject *kobj)
> > {
> > struct net_bridge_port *p
> > = container_of(kobj, struct net_bridge_port, kobj);
> > +
> > + dev_put(p->dev);
> > kfree(p);
> > }
> >
> > @@ -127,12 +129,6 @@ static struct kobj_type brport_ktype = {
> >
> > static void destroy_nbp(struct net_bridge_port *p)
> > {
> > - struct net_device *dev = p->dev;
> > -
> > - p->br = NULL;
> > - p->dev = NULL;
> > - dev_put(dev);
> > -
> > kobject_put(&p->kobj);
> > }
>
> Moving this around is problematic.
> The ordering here was chosen to be RCU friendly so that
> p->dev indicates the port is in process of being deleted but traffic
> may still be using old reference, but new traffic should not use it.
But it is still RCU friendly? destroy_nbp() is rcu-callback which
calls release_nbp() if we have a last reference to ->kobj. This
means that dev_put() may be done a bit later, but not earlier.
And RCU can only garantee "not before", any rcu-callback could be
delayed unpredictably.
Stephen, I know nothing about net/, and
> Probably the best thing to do is pull the whole delayed work queue
> and auto port speed stuff. When STP is moved to user space then
> it can do the ethtool op there.
I can't understand any single word in the paragraph above :)
But the bug (the stable tree has it too) is real. If this patch is
really wrong, could you please take care of it?
Oleg.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
