Actually, priv->broadcast was never checked for being non-NULL
in mcast_join_task before this patch.
The only change in the patch is that priv->broadcast might stay NULL
if it was NULL when the function started. So, if anyone could set
priv->broadcast to NULL while join_task is running, wouldn't it
happen without the patch as well?
Roland Dreier wrote:
So what protects priv->broadcast? It seems that the only lock taken
when setting broadcast to NULL is priv->lock. But eg here:
- priv->mcast_mtu =
IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu));
+ if (priv->broadcast)
+ priv->mcast_mtu =
IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu));
what prevents broadcast from becoming NULL right after it's tested?
Also
+ spin_lock_irq(&priv->lock);
+ if (priv->broadcast &&
+ !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &priv->broadcast->flags)) {
if (!test_bit(IPOIB_MCAST_FLAG_BUSY, &priv->broadcast->flags))
ipoib_mcast_join(dev, priv->broadcast, 0);
doesn't ipoib_mcast_join() do GFP_KERNEL stuff, which would be a problem
inside a spinlock? (Have you tested this with lockdep turned on?)
- R.
--
--Yossi
_______________________________________________
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
