[ofa-general] Re: [PATCH v3] ipoib: do not join broadcast group if interface is brought down

[Roland Dreier]

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.