Hi Arend, > I stumbled upon locking in wiphy_apply_custom_regulatory() and the > history of commits make it a bit unclear to me if these locks are all > really needed. Oh how I like the regulatory code ... ;) > beee246951571 added the rcu assignments without any locking. Well that was just plain wrong, right? You have to have some kind of writer-side locking with RCU. I think this probably was assuming we'd be under some locks already? In regulatory_set_wiphy_regd() vs. regulatory_set_wiphy_regd_sync() we later formalized this. > 51d62f2f2c501 added the rtnl locking. That was the bugfix. > a05829a7222e9 added the wiphy lock. Yes that's because now get_wiphy_regdom() can be called as documented: /* * Returns the regulatory domain associated with the wiphy. * * Requires any of RTNL, wiphy mutex or RCU protection. */ > My understanding of RCU operations was that locking is only needed for > readers, > No, readers are lockless (just "rcu_read_lock()" which enters a critical section, but no locking). > but here we use not one but two mutex locks which makes me > wonder. Especially, as this function is called before registering the wiphy. > > Could you clarify? > Like I said above, that's because we want to dereference the pointer in three different contexts: * RCU * RTNL * wiphy mutex Maybe we can get rid of some of those readers, but e.g. iwlwifi is using wiphy_dereference(mvm->hw->wiphy, mvm->hw->wiphy->regd); which really should use get_wiphy_regdom(), but I'm not sure is holding the RTNL in all paths? Looking at them, it seems like it is, so perhaps we can get rid of the wiphy mutex locking. Why are you asking? :) johannes
