On 08/25/2015 02:22 PM, Jason Gunthorpe wrote:
> On Tue, Aug 25, 2015 at 01:50:05PM -0400, Doug Ledford wrote:
>> On 08/21/2015 07:34 PM, Jason Gunthorpe wrote:
>>> Even though we don't expect the group to be created by the SM we
>>> sill need to provide all the parameters to force the SM to validate
>>> they are correct.
>>
>> Why does this patch embed locking changes that, as far I can tell, are
>> not needed by the rest of the patch?
> 
> test_bit was lowered into ipoib_mcast_join, which requires pushing the
> lock unlock down as well. The set_bit side holds that lock.

I see the confusion.  The test bit of SENDONLY isn't protected by the
lock, just the setting and clearing of BUSY.  There isn't any need to
push the locking down into mcast_join just because we are checking
SENDONLY in mcast_join.

>> If the locking changes are needed for some reason, then they likely
>> need to be their own patch with their own changelog.
> 
> It doesn't make sense to move the locking without the code motion that
> motivates it, IMHO.

Sure, I agree with you on that point.  I thought you were changing the
locking for some other reason that I wasn't groking, I didn't think you
were doing that for the SENDONLY flag test.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD