On Thu, Feb 12, 2009 at 11:48 PM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> 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 -- 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
