> In ipoib_flush_paths(), we take the netif_tx_lock to remove a path
> My question is - what data does this lock protect?
> It isn't path->list and path->rb_node, because priv->lock is enough to
> protect them.
>
> It might be neigh and neigh->ah, to avoid freeing the neighbour and its
> address
> handle while ipoib_start_xmit() is using it, but this particular part is
> done *outside*
> the tx lock.
You probably already saw this from the history of the file, but this
locking was added in 9217b27b:
IB/ipoib: Fix flush/start xmit race (from code review)
Prevent flush task from freeing the ipoib_neigh pointer, while
ipoib_start_xmit() is accessing the ipoib_neigh through the pointer it
has loaded from the skb's hardware address.
> Unless I'm missing something - shouldn't the code:
>
> spin_unlock_irqrestore(&priv->lock, flags);
> netif_tx_unlock_bh(dev);
> wait_for_completion(&path->done);
> release >> path_free(dev, path);
> lock >> netif_tx_lock_bh(dev);
> spin_lock_irqsave(&priv->lock, flags);
>
> Be like this:
>
> spin_unlock_irqrestore(&priv->lock, flags);
> netif_tx_unlock_bh(dev);
> wait_for_completion(&path->done);
> lock >> netif_tx_lock_bh(dev);
> release >> path_free(dev, path);
> spin_lock_irqsave(&priv->lock, flags);
I don't think it matters -- the path has been removed from every
externally visible list/rbtree already so there's no way anyone could
grab it after we drop the tx lock.
- R.
_______________________________________________
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
