Subject: Re: [3/4] bonding: the calling of bond->slave_cnt need protection

From: Nikolay Aleksandrov <[email-masked]>
Date: Sat, 20 Jul 2013 14:42:37 +0200

On 07/20/2013 12:47 PM, Veaceslav Falico wrote:
> On Sat, Jul 20, 2013 at 03:23:57PM +0800, dingtianhong wrote:
>> The bonding_store_mode has rtnl protection, so no need to get read lock
>> for bond->slave_cnt, but the bonding_store_fail_over_mac need to protect
>> the bond->slave_cnt, so add read_lock().
>>
>> Signed-off-by: Ding Tianhong <[email-masked]>
>>
<snip>
> 
> Maybe it's Saturday, but I really don't see *any* point in this locking.
> 
> I think you've meant that we need the rtnl protection while reading
> slave_cnt AND updating the .fail_over_mac, so that in between we won't add
> new slaves with outdated params.
> 
> Something like this (untested):
> 
Indeed, Veaceslav's way is the correct one (I've looked at this race
before), but IMO it's not worth it to protect fail_over_mac as the worst
that could happen is inconsistency with the MAC addresses which isn't
fatal. Anyway, I still haven't had my coffee and might be missing something :-)

Cheers,
 Nik