> Quoting Moni Shoua <[EMAIL PROTECTED]>:
> Subject: Re: [ofa-general] Re: [RFC] [PATCH v3] IB/ipoib: Add bonding support
> to?IPoIB
>
> Michael S. Tsirkin wrote:
> > Another question.
> >
> >> @@ -769,32 +768,32 @@ static void ipoib_set_mcast_list(struct
> >> static void ipoib_neigh_destructor(struct neighbour *n)
> >> {
> >> struct ipoib_neigh *neigh;
> >> - struct ipoib_dev_priv *priv = netdev_priv(n->dev);
> >> + struct ipoib_dev_priv *priv;
> >> unsigned long flags;
> >> struct ipoib_ah *ah = NULL;
> >>
> >> - ipoib_dbg(priv,
> >> - "neigh_destructor for %06x " IPOIB_GID_FMT "\n",
> >> - IPOIB_QPN(n->ha),
> >> - IPOIB_GID_RAW_ARG(n->ha + 4));
> >> -
> >> - spin_lock_irqsave(&priv->lock, flags);
> >>
> >> neigh = *to_ipoib_neigh(n);
> >> if (neigh) {
> >> + priv = netdev_priv(neigh->dev);
> >> + ipoib_dbg(priv,
> >> + "neigh_destructor for %06x " IPOIB_GID_FMT "\n",
> >> + IPOIB_QPN(n->ha),
> >> + IPOIB_GID_RAW_ARG(n->ha + 4));
> >> +
> >> + spin_lock_irqsave(&priv->lock, flags);
> >> if (neigh->ah)
> >> ah = neigh->ah;
> >> list_del(&neigh->list);
> >> ipoib_neigh_free(n->dev, neigh);
> >> + spin_unlock_irqrestore(&priv->lock, flags);
> >> }
> >> -
> >> - spin_unlock_irqrestore(&priv->lock, flags);
> >> -
> >> if (ah)
> >> ipoib_put_ah(ah);
> >> }
> >
> > Using to_ipoib_neigh outside priv->lock looks problematic.
> > Can you convince me this does not introduce new races?
> >
> >
> I can try...
> ipoib_neigh_destructor is called from neigh_destroy() and this is when the
> kernel neighbour is under destruction itself and no one holds a reference to
> it.
OK but we might have references to ipoib_neigh. Specifically path and mcast
group all might have it - that's what neigh_list is.
> My opinion is that if I can't assume that no one is touching ipoib_neigh when
> kernel
> neighbour is being destroyed then we have a bigger problem.
That's what locks you remove seem to be there for.
--
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
