Re: [PATCH 6/6] cfg80211: move regulatory hints to workqueue

["Luis R. Rodriguez" <lrodriguez@atheros.com>]

On Thu, Feb 12, 2009 at 11:48 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2009-02-12 at 23:35 -0800, Luis R. Rodriguez wrote:
>
>> > Well, regardless of regulatory, we now have two paths:
>> >
>> > nl80211: cfg80211_mutex  -->  drv->mutex  -->  rtnl
>> > wext:    rtnl  -->  cfg80211_mutex  -->  drv->mutex
>> >
>> > which is clearly not a good plan.
>> >
>> > Therefore, I'll probably have to implement solution (1) from what I sent
>> > you, and invert the locking in nl80211. That would fix your the
>> > immediate problem with regulatory as well, because that was:
>> >
>> > rtnl  --->  (regulatory_hint) --->  cfg80211_mutex  -->  drv->mutex
>>
>> Yup I see. So as it stands in this patch user reg hint now is:
>>
>> reg_mutex --> cfg80211_mutex
>
> I don't really care about reg_mutex much, and it probably doesn't matter
> as long as its use is limited. Question is whether we need it, but
> that's a different question.

To answer that question -- reg_mutex just protects the
reg_requests_list linked list. That's it. Without that you could
potentially have two userspace reg hints while the workqueue runs.
Remember that the hints no longer lock except to just add a new
regulatory_request into the reg_requests_list.

>> Driver hint also does the same so it seems your fix on nl80211 would
>> probably just tap this series on the nl80211_req_set_reg().
>
> Yeah, I think I'll work on top of your series anyway so I don't clash
> with the cleanups.

Any other comments on the series?

>> I also think using a workqueue would be good here anyway, the only
>> negative thing I've seen is loading takes a bit longer.
>
> Indeed, it's a good thing because a path like this:
>
> driver_open
>  -> regulatory_hint
>  -> cfg80211 stuff
>   -> driver_reg_notifier
>
> is always deadlock prone since you call into the driver from itself.

I see -- good point.

  Luis