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.
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.
_______________________________________________
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
