[openib-general] Re: ipoib_mcast_send.patch

[Michael S. Tsirkin]

Hi!
Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: ipoib_mcast_send.patch
> 
> I started looking at IPoIB patches again.  In ipoib_mcast_send.patch,
> we have:
> 
>  > --- linux-2.6.15.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2006-01-23 21:24:10.000000000 +0200
>  > +++ linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2006-01-23 21:25:19.000000000 +0200
>  > @@ -600,6 +600,10 @@ int ipoib_mcast_start_thread(struct net_
>  >  		queue_work(ipoib_workqueue, &priv->mcast_task);
>  >  	mutex_unlock(&mcast_mutex);
>  >  
>  > +	spin_lock_irq(&priv->lock);
>  > +	set_bit(IPOIB_MCAST_STARTED, &priv->flags);
>  > +	spin_unlock_irq(&priv->lock);
> 
> This seems to leave a window where we set the IPOIB_MCAST_STARTED flag
> but the multicast work hasn't run yet.  Then it seems we're still
> susceptible to the issue you described here:
> 
>  > Further, there's an additional issue that I saw in testing:
>  > ipoib_mcast_send may get called when priv->broadcast is NULL
>  > (e.g. if the device was downed and then upped internally because
>  > of a port event).
>  > If this happends and the sendonly join request gets completed before
>  > priv->broadcast is set, we get an oops

Maybe the description is not clear enough.
There are two issues here with two separate fixes.

1. IPOIB_MCAST_STARTED - solves the first issue
2. Checking priv->broadcast in ipoib_mcast_send here:
+       if (!test_bit(IPOIB_MCAST_STARTED, &priv->flags) || !priv->broadcast) {
	- solves the second issue

They just got rolled into one patch because they touch the same code lines.

Do you want me to split them up?

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies