Skip to content

Commit

Permalink
cfg80211: avoid holding the RTNL when calling the driver
Browse files Browse the repository at this point in the history
Currently, _everything_ in cfg80211 holds the RTNL, and if you
have a slow USB device (or a few) you can get some bad lock
contention on that.

Fix that by re-adding a mutex to each wiphy/rdev as we had at
some point, so we have locking for the wireless_dev lists and
all the other things in there, and also so that drivers still
don't have to worry too much about it (they still won't get
parallel calls for a single device).

Then, we can restrict the RTNL to a few cases where we add or
remove interfaces and really need the added protection. Some
of the global list management still also uses the RTNL, since
we need to have it anyway for netdev management, but we only
hold the RTNL for very short periods of time here.

Link: https://lore.kernel.org/r/20210122161942.81df9f5e047a.I4a8e1a60b18863ea8c5e6d3a0faeafb2d45b2f40@changeid
Tested-by: Marek Szyprowski <[email protected]> [marvell driver issues]
Signed-off-by: Johannes Berg <[email protected]>
  • Loading branch information
jmberg-intel committed Jan 26, 2021
1 parent 2fe8ef1 commit a05829a
Show file tree
Hide file tree
Showing 39 changed files with 880 additions and 509 deletions.
4 changes: 3 additions & 1 deletion drivers/net/wireless/ath/ath11k/reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ int ath11k_regd_update(struct ath11k *ar, bool init)
}

rtnl_lock();
ret = regulatory_set_wiphy_regd_sync_rtnl(ar->hw->wiphy, regd_copy);
wiphy_lock(ar->hw->wiphy);
ret = regulatory_set_wiphy_regd_sync(ar->hw->wiphy, regd_copy);
wiphy_unlock(ar->hw->wiphy);
rtnl_unlock();

kfree(regd_copy);
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/ath6kl/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,13 @@ int ath6kl_core_init(struct ath6kl *ar, enum ath6kl_htc_type htc_type)
ar->avail_idx_map |= BIT(i);

rtnl_lock();
wiphy_lock(ar->wiphy);

/* Add an initial station interface */
wdev = ath6kl_interface_add(ar, "wlan%d", NET_NAME_ENUM,
NL80211_IFTYPE_STATION, 0, INFRA_NETWORK);

wiphy_unlock(ar->wiphy);
rtnl_unlock();

if (!wdev) {
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/ath6kl/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -1904,7 +1904,9 @@ void ath6kl_stop_txrx(struct ath6kl *ar)
spin_unlock_bh(&ar->list_lock);
ath6kl_cfg80211_vif_stop(vif, test_bit(WMI_READY, &ar->flag));
rtnl_lock();
wiphy_lock(ar->wiphy);
ath6kl_cfg80211_vif_cleanup(vif);
wiphy_unlock(ar->wiphy);
rtnl_unlock();
spin_lock_bh(&ar->list_lock);
}
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/wil6210/cfg80211.c
Original file line number Diff line number Diff line change
Expand Up @@ -2820,7 +2820,9 @@ void wil_p2p_wdev_free(struct wil6210_priv *wil)
wil->radio_wdev = wil->main_ndev->ieee80211_ptr;
mutex_unlock(&wil->vif_mutex);
if (p2p_wdev) {
wiphy_lock(wil->wiphy);
cfg80211_unregister_wdev(p2p_wdev);
wiphy_unlock(wil->wiphy);
kfree(p2p_wdev);
}
}
Expand Down
7 changes: 6 additions & 1 deletion drivers/net/wireless/ath/wil6210/netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,9 @@ int wil_if_add(struct wil6210_priv *wil)
wil_update_net_queues_bh(wil, vif, NULL, true);

rtnl_lock();
wiphy_lock(wiphy);
rc = wil_vif_add(wil, vif);
wiphy_unlock(wiphy);
rtnl_unlock();
if (rc < 0)
goto out_wiphy;
Expand Down Expand Up @@ -543,15 +545,18 @@ void wil_if_remove(struct wil6210_priv *wil)
{
struct net_device *ndev = wil->main_ndev;
struct wireless_dev *wdev = ndev->ieee80211_ptr;
struct wiphy *wiphy = wdev->wiphy;

wil_dbg_misc(wil, "if_remove\n");

rtnl_lock();
wiphy_lock(wiphy);
wil_vif_remove(wil, 0);
wiphy_unlock(wiphy);
rtnl_unlock();

netif_napi_del(&wil->napi_tx);
netif_napi_del(&wil->napi_rx);

wiphy_unregister(wdev->wiphy);
wiphy_unregister(wiphy);
}
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/wil6210/pcie_bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,10 @@ static void wil_pcie_remove(struct pci_dev *pdev)

wil6210_debugfs_remove(wil);
rtnl_lock();
wiphy_lock(wil->wiphy);
wil_p2p_wdev_free(wil);
wil_remove_all_additional_vifs(wil);
wiphy_unlock(wil->wiphy);
rtnl_unlock();
wil_if_remove(wil);
wil_if_pcie_disable(wil);
Expand Down
18 changes: 9 additions & 9 deletions drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ static const struct net_device_ops brcmf_netdev_ops_pri = {
.ndo_set_rx_mode = brcmf_netdev_set_multicast_list
};

int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked)
int brcmf_net_attach(struct brcmf_if *ifp, bool locked)
{
struct brcmf_pub *drvr = ifp->drvr;
struct net_device *ndev;
Expand All @@ -656,7 +656,7 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked)
INIT_WORK(&ifp->multicast_work, _brcmf_set_multicast_list);
INIT_WORK(&ifp->ndoffload_work, _brcmf_update_ndtable);

if (rtnl_locked)
if (locked)
err = cfg80211_register_netdevice(ndev);
else
err = register_netdev(ndev);
Expand All @@ -677,10 +677,10 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked)
return -EBADE;
}

void brcmf_net_detach(struct net_device *ndev, bool rtnl_locked)
void brcmf_net_detach(struct net_device *ndev, bool locked)
{
if (ndev->reg_state == NETREG_REGISTERED) {
if (rtnl_locked)
if (locked)
cfg80211_unregister_netdevice(ndev);
else
unregister_netdev(ndev);
Expand Down Expand Up @@ -909,7 +909,7 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
}

static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
bool rtnl_locked)
bool locked)
{
struct brcmf_if *ifp;
int ifidx;
Expand Down Expand Up @@ -938,7 +938,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
cancel_work_sync(&ifp->multicast_work);
cancel_work_sync(&ifp->ndoffload_work);
}
brcmf_net_detach(ifp->ndev, rtnl_locked);
brcmf_net_detach(ifp->ndev, locked);
} else {
/* Only p2p device interfaces which get dynamically created
* end up here. In this case the p2p module should be informed
Expand All @@ -947,7 +947,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
* serious troublesome side effects. The p2p module will clean
* up the ifp if needed.
*/
brcmf_p2p_ifp_removed(ifp, rtnl_locked);
brcmf_p2p_ifp_removed(ifp, locked);
kfree(ifp);
}

Expand All @@ -956,14 +956,14 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
drvr->if2bss[ifidx] = BRCMF_BSSIDX_INVALID;
}

void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked)
void brcmf_remove_interface(struct brcmf_if *ifp, bool locked)
{
if (!ifp || WARN_ON(ifp->drvr->iflist[ifp->bsscfgidx] != ifp))
return;
brcmf_dbg(TRACE, "Enter, bsscfgidx=%d, ifidx=%d\n", ifp->bsscfgidx,
ifp->ifidx);
brcmf_proto_del_if(ifp->drvr, ifp);
brcmf_del_if(ifp->drvr, ifp->bsscfgidx, rtnl_locked);
brcmf_del_if(ifp->drvr, ifp->bsscfgidx, locked);
}

static int brcmf_psm_watchdog_notify(struct brcmf_if *ifp,
Expand Down
6 changes: 3 additions & 3 deletions drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,16 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp);
char *brcmf_ifname(struct brcmf_if *ifp);
struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
void brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable);
int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked);
int brcmf_net_attach(struct brcmf_if *ifp, bool locked);
struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
bool is_p2pdev, const char *name, u8 *mac_addr);
void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked);
void brcmf_remove_interface(struct brcmf_if *ifp, bool locked);
void brcmf_txflowblock_if(struct brcmf_if *ifp,
enum brcmf_netif_stop_reason reason, bool state);
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb, bool inirq);
void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb);
void brcmf_net_detach(struct net_device *ndev, bool rtnl_locked);
void brcmf_net_detach(struct net_device *ndev, bool locked);
int brcmf_net_mon_attach(struct brcmf_if *ifp);
void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on);
int __init brcmf_core_init(void);
Expand Down
12 changes: 8 additions & 4 deletions drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
Original file line number Diff line number Diff line change
Expand Up @@ -2430,7 +2430,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev)
return err;
}

void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked)
void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool locked)
{
struct brcmf_cfg80211_info *cfg;
struct brcmf_cfg80211_vif *vif;
Expand All @@ -2439,11 +2439,15 @@ void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked)
vif = ifp->vif;
cfg = wdev_to_cfg(&vif->wdev);
cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif = NULL;
if (!rtnl_locked)
if (locked) {
rtnl_lock();
cfg80211_unregister_wdev(&vif->wdev);
if (!rtnl_locked)
wiphy_lock(cfg->wiphy);
cfg80211_unregister_wdev(&vif->wdev);
wiphy_unlock(cfg->wiphy);
rtnl_unlock();
} else {
cfg80211_unregister_wdev(&vif->wdev);
}
brcmf_free_vif(vif);
}

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/wireless/intel/iwlwifi/mvm/d3.c
Original file line number Diff line number Diff line change
Expand Up @@ -2143,7 +2143,7 @@ static int __iwl_mvm_resume(struct iwl_mvm *mvm, bool test)

out_iterate:
if (!test)
ieee80211_iterate_active_interfaces_rtnl(mvm->hw,
ieee80211_iterate_active_interfaces_mtx(mvm->hw,
IEEE80211_IFACE_ITER_NORMAL,
iwl_mvm_d3_disconnect_iter, keep ? vif : NULL);

Expand Down
4 changes: 2 additions & 2 deletions drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ int iwl_mvm_init_fw_regd(struct iwl_mvm *mvm)
int ret;
bool changed;
const struct ieee80211_regdomain *r =
rtnl_dereference(mvm->hw->wiphy->regd);
wiphy_dereference(mvm->hw->wiphy, mvm->hw->wiphy->regd);

if (!r)
return -ENOENT;
Expand All @@ -282,7 +282,7 @@ int iwl_mvm_init_fw_regd(struct iwl_mvm *mvm)

/* update cfg80211 if the regdomain was changed */
if (changed)
ret = regulatory_set_wiphy_regd_sync_rtnl(mvm->hw->wiphy, regd);
ret = regulatory_set_wiphy_regd_sync(mvm->hw->wiphy, regd);
else
ret = 0;

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ int iwl_mvm_init_mcc(struct iwl_mvm *mvm)
return -EIO;
}

retval = regulatory_set_wiphy_regd_sync_rtnl(mvm->hw->wiphy, regd);
retval = regulatory_set_wiphy_regd_sync(mvm->hw->wiphy, regd);
kfree(regd);
return retval;
}
Expand Down
6 changes: 3 additions & 3 deletions drivers/net/wireless/marvell/mwifiex/cfg80211.c
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ mwifiex_cfg80211_disconnect(struct wiphy *wiphy, struct net_device *dev,
struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);

if (!mwifiex_stop_bg_scan(priv))
cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
cfg80211_sched_scan_stopped_locked(priv->wdev.wiphy, 0);

if (mwifiex_deauthenticate(priv, NULL))
return -EFAULT;
Expand Down Expand Up @@ -2366,7 +2366,7 @@ mwifiex_cfg80211_connect(struct wiphy *wiphy, struct net_device *dev,
(int)sme->ssid_len, (char *)sme->ssid, sme->bssid);

if (!mwifiex_stop_bg_scan(priv))
cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
cfg80211_sched_scan_stopped_locked(priv->wdev.wiphy, 0);

ret = mwifiex_cfg80211_assoc(priv, sme->ssid_len, sme->ssid, sme->bssid,
priv->bss_mode, sme->channel, sme, 0);
Expand Down Expand Up @@ -2576,7 +2576,7 @@ mwifiex_cfg80211_scan(struct wiphy *wiphy,
priv->scan_block = false;

if (!mwifiex_stop_bg_scan(priv))
cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
cfg80211_sched_scan_stopped_locked(priv->wdev.wiphy, 0);

user_scan_cfg = kzalloc(sizeof(*user_scan_cfg), GFP_KERNEL);
if (!user_scan_cfg)
Expand Down
7 changes: 7 additions & 0 deletions drivers/net/wireless/marvell/mwifiex/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,14 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
}

rtnl_lock();
wiphy_lock(adapter->wiphy);
/* Create station interface by default */
wdev = mwifiex_add_virtual_intf(adapter->wiphy, "mlan%d", NET_NAME_ENUM,
NL80211_IFTYPE_STATION, NULL);
if (IS_ERR(wdev)) {
mwifiex_dbg(adapter, ERROR,
"cannot create default STA interface\n");
wiphy_unlock(adapter->wiphy);
rtnl_unlock();
goto err_add_intf;
}
Expand All @@ -614,6 +616,7 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
if (IS_ERR(wdev)) {
mwifiex_dbg(adapter, ERROR,
"cannot create AP interface\n");
wiphy_unlock(adapter->wiphy);
rtnl_unlock();
goto err_add_intf;
}
Expand All @@ -625,10 +628,12 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
if (IS_ERR(wdev)) {
mwifiex_dbg(adapter, ERROR,
"cannot create p2p client interface\n");
wiphy_unlock(adapter->wiphy);
rtnl_unlock();
goto err_add_intf;
}
}
wiphy_unlock(adapter->wiphy);
rtnl_unlock();

mwifiex_drv_get_driver_version(adapter, fmt, sizeof(fmt) - 1);
Expand Down Expand Up @@ -1440,9 +1445,11 @@ static void mwifiex_uninit_sw(struct mwifiex_adapter *adapter)
if (!priv)
continue;
rtnl_lock();
wiphy_lock(adapter->wiphy);
if (priv->netdev &&
priv->wdev.iftype != NL80211_IFTYPE_UNSPECIFIED)
mwifiex_del_virtual_intf(adapter->wiphy, &priv->wdev);
wiphy_unlock(adapter->wiphy);
rtnl_unlock();
}

Expand Down
3 changes: 2 additions & 1 deletion drivers/net/wireless/quantenna/qtnfmac/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,9 @@ static int qtnf_core_mac_attach(struct qtnf_bus *bus, unsigned int macid)
mac->wiphy_registered = 1;

rtnl_lock();

wiphy_lock(priv_to_wiphy(mac));
ret = qtnf_core_net_attach(mac, vif, "wlan%d", NET_NAME_ENUM);
wiphy_unlock(priv_to_wiphy(mac));
rtnl_unlock();

if (ret) {
Expand Down
8 changes: 8 additions & 0 deletions drivers/net/wireless/virt_wifi.c
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,9 @@ static int virt_wifi_newlink(struct net *src_net, struct net_device *dev,
dev->ieee80211_ptr->iftype = NL80211_IFTYPE_STATION;
dev->ieee80211_ptr->wiphy = common_wiphy;

wiphy_lock(common_wiphy);
err = register_netdevice(dev);
wiphy_unlock(common_wiphy);
if (err) {
dev_err(&priv->lowerdev->dev, "can't register_netdevice: %d\n",
err);
Expand All @@ -560,7 +562,9 @@ static int virt_wifi_newlink(struct net *src_net, struct net_device *dev,

return 0;
unregister_netdev:
wiphy_lock(common_wiphy);
unregister_netdevice(dev);
wiphy_unlock(common_wiphy);
free_wireless_dev:
kfree(dev->ieee80211_ptr);
dev->ieee80211_ptr = NULL;
Expand All @@ -586,7 +590,9 @@ static void virt_wifi_dellink(struct net_device *dev,
netdev_rx_handler_unregister(priv->lowerdev);
netdev_upper_dev_unlink(priv->lowerdev, dev);

wiphy_lock(common_wiphy);
unregister_netdevice_queue(dev, head);
wiphy_unlock(common_wiphy);
module_put(THIS_MODULE);

/* Deleting the wiphy is handled in the module destructor. */
Expand Down Expand Up @@ -625,7 +631,9 @@ static int virt_wifi_event(struct notifier_block *this, unsigned long event,
upper_dev = priv->upperdev;

upper_dev->rtnl_link_ops->dellink(upper_dev, &list_kill);
wiphy_lock(common_wiphy);
unregister_netdevice_many(&list_kill);
wiphy_unlock(common_wiphy);
break;
}

Expand Down
Loading

0 comments on commit a05829a

Please sign in to comment.