[rt2x00-users] [RFC 1/3] rt2x00: Simplify locking in rt2x00mac_bss_info_changed

[Ivo Van Doorn]

Hi,

>> Locking the bssid should not be needed since it is only accessed from
>> within rt2x00mac_bss_info_changed.
>>
>> Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
>> ---
>>
>> Ivo, do you remember why this locking was needed? At least it looks to me as if
>> it is not required anymore.
>>
>> Could somebody please review my assumption?

By the looks of it, it protected the BSSID and the changed_flags.
The locking looks a bit as overkill at the moment, but I think it was
used more in the past and over time the code was restructured to
require less locking.

>>  drivers/net/wireless/rt2x00/rt2x00mac.c |   18 +++---------------
>>  1 files changed, 3 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
>> index b49170d..70c60f2 100644
>> --- a/drivers/net/wireless/rt2x00/rt2x00mac.c
>> +++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
>> @@ -614,26 +614,14 @@ void rt2x00mac_bss_info_changed(struct ieee80211_hw *hw,
>>        if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
>>                return;
>>
>> -       spin_lock(&intf->lock);
>> -
>>        /*
>> -        * conf->bssid can be NULL if coming from the internal
>> -        * beacon update routine.
>> +        * Update bssid.
>>         */
>> -       if (changes & BSS_CHANGED_BSSID)
>> +       if (changes & BSS_CHANGED_BSSID) {
>>                memcpy(&intf->bssid, bss_conf->bssid, ETH_ALEN);
>> -
>> -       spin_unlock(&intf->lock);
>> -
>> -       /*
>> -        * Call rt2x00_config_intf() outside of the spinlock context since
>> -        * the call will sleep for USB drivers. By using the ieee80211_if_conf
>> -        * values as arguments we make keep access to rt2x00_intf thread safe
>> -        * even without the lock.
>> -        */
>> -       if (changes & BSS_CHANGED_BSSID)
>>                rt2x00lib_config_intf(rt2x00dev, intf, vif->type, NULL,
>>                                      bss_conf->bssid);
>> +       }


Could there be any problems in the multi-BSS case where
add_interface/remove_interface is called concurrently with
rt2x00mac_bss_info_changed() ?
Because then there could be a race condition with the calls to
rt2x00lib_config_intf

Ivo