[openib-general] Re: ipoib_multicast_ah.patch

[Michael S. Tsirkin]

Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: ipoib_multicast_ah.patch
> 
>     >> who else might change the value here?  Answering this would
>     >> help justify the patch and save me from having to write up a
>     >> changelog entry.
> 
>     Michael> I has a quick look and couldnt see any other place -
>     Michael> except when the mcast entry is freed. Am I missing
>     Michael> something?
> 
> I don't know if you're missing anything -- you posted the patch, and
> I'm just trying to understand the race it protects against.  If
> nothing else could change the value in between the tests, then what's
> the harm in testing it twice?

I guess there's a misunderstanding here. Its pretty simple:
ipoib_mcast_send tests mcast->ah twice under priv->lock.
ipoib_mcast_join_finish modifies the mcast->ah without taking a lock.
No *other* place modifies the mcast->ah.

As a solution, take priv->lock around assignment to mcast->ah
thus making sure ipoib_mcast_send is not in flight.

OK?

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies