On 24 March 2014 15:32, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
> On 21/03/14 14:31, Michal Kazior wrote:
[...]
>> --- a/net/wireless/core.c
>> +++ b/net/wireless/core.c
>> @@ -748,23 +748,23 @@ void cfg80211_update_iface_num(struct
>> cfg80211_registered_device *rdev,
>> rdev->num_running_monitor_ifaces += num;
>> }
>>
>> -void cfg80211_leave(struct cfg80211_registered_device *rdev,
>> - struct wireless_dev *wdev)
>> +void __cfg80211_leave(struct cfg80211_registered_device *rdev,
>> + struct wireless_dev *wdev)
>> {
>> struct net_device *dev = wdev->netdev;
>>
>> ASSERT_RTNL();
>
>
> If you have the assert check here do you need to do it from the call sites
> as well?
I just tend to propagate lockdeps asserts up. It's easier to get the
idea of locking at a quick glance and it causes lockdep to splat
sooner if you have lots of conditionals (i.e. before you hit a very
specific if-else-codepath). I'll remove this redundancy when I re-spin
unless someone objects?
>> + ASSERT_WDEV_LOCK(wdev);
>
>
> So you are changing behaviour here by requiring the wdev to be locked.
It doesn't change much in practice. Notice that prior to the patch all
switch-cases performed wdev_lock() already (either on site, or callees
did that immediately). This simply moves the locking outside the
__cfg80211_leave() so it's possible to call it from the event worker.
__cfg80211_stop_sched_scan() doesn't care about wdev lock so it's okay
I guess?
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
