diff options
author | Alexis Lothoré <alexis.lothore@bootlin.com> | 2024-05-28 16:20:28 +0200 |
---|---|---|
committer | Kalle Valo <kvalo@kernel.org> | 2024-06-01 12:59:29 +0300 |
commit | ebfb5e8fc8b45040b979f4bf0012a01d0abac8d4 (patch) | |
tree | 519b26c46349813fca89ad50d3136d1867a35471 /drivers/net/wireless/microchip/wilc1000/netdev.c | |
parent | 10bc8558b59a264b2e342363c9ed6b2fae7a060d (diff) |
Revert "wifi: wilc1000: convert list management to RCU"
This reverts commit f236464f1db7bea80075e6e31ac70dc6eb80547f
Commit f236464f1db7 ("wifi: wilc1000: convert list management to RCU")
replaced SRCU with RCU, aiming to simplify RCU usage in the driver. No
documentation or commit history hinted about why SRCU has been preferred
in original design, so it has been assumed to be safe to do this
conversion.
Unfortunately, some static analyzers raised warnings, confirmed by runtime
checker, not long after the merge. At least three different issues arose
when switching to RCU:
- wilc_wlan_txq_filter_dup_tcp_ack is executed in a RCU read critical
section yet calls wait_for_completion_timeout
- wilc_wfi_init_mon_interface calls kmalloc and register_netdevice while
manipulating a vif retrieved from vif list
- set_channel sends command to chip (and so, also waits for a completion)
while holding a vif retrieved from vif list (so, in RCU read critical
section)
Some of those issues are not trivial to fix and would need bigger driver
rework. Fix those issues by reverting the SRCU to RCU conversion commit
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-wireless/3b46ec7c-baee-49fd-b760-3bc12fb12eaf@moroto.mountain/
Fixes: f236464f1db7 ("wifi: wilc1000: convert list management to RCU")
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Signed-off-by: Kalle Valo <kvalo@kernel.org>
Link: https://msgid.link/20240528-wilc_revert_srcu_to_rcu-v1-1-bce096e0798c@bootlin.com
Diffstat (limited to 'drivers/net/wireless/microchip/wilc1000/netdev.c')
-rw-r--r-- | drivers/net/wireless/microchip/wilc1000/netdev.c | 43 |
1 files changed, 25 insertions, 18 deletions
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c index 73f56f7b002b..710e29bea560 100644 --- a/drivers/net/wireless/microchip/wilc1000/netdev.c +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c @@ -127,28 +127,30 @@ void wilc_wlan_set_bssid(struct net_device *wilc_netdev, const u8 *bssid, int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc) { + int srcu_idx; u8 ret_val = 0; struct wilc_vif *vif; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wilc->srcu); wilc_for_each_vif(wilc, vif) { if (!is_zero_ether_addr(vif->bssid)) ret_val++; } - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); return ret_val; } static void wilc_wake_tx_queues(struct wilc *wl) { + int srcu_idx; struct wilc_vif *ifc; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wl->srcu); wilc_for_each_vif(wl, ifc) { if (ifc->mac_opened && netif_queue_stopped(ifc->ndev)) netif_wake_queue(ifc->ndev); } - rcu_read_unlock(); + srcu_read_unlock(&wl->srcu, srcu_idx); } static int wilc_txq_task(void *vp) @@ -653,6 +655,7 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p) struct sockaddr *addr = (struct sockaddr *)p; unsigned char mac_addr[ETH_ALEN]; struct wilc_vif *tmp_vif; + int srcu_idx; if (!is_valid_ether_addr(addr->sa_data)) return -EADDRNOTAVAIL; @@ -664,19 +667,19 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p) /* Verify MAC Address is not already in use: */ - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wilc->srcu); wilc_for_each_vif(wilc, tmp_vif) { wilc_get_mac_address(tmp_vif, mac_addr); if (ether_addr_equal(addr->sa_data, mac_addr)) { if (vif != tmp_vif) { - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); return -EADDRNOTAVAIL; } - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); return 0; } } - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); result = wilc_set_mac_address(vif, (u8 *)addr->sa_data); if (result) @@ -764,14 +767,15 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev) wilc_tx_complete); if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) { + int srcu_idx; struct wilc_vif *vif; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wilc->srcu); wilc_for_each_vif(wilc, vif) { if (vif->mac_opened) netif_stop_queue(vif->ndev); } - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); } return NETDEV_TX_OK; @@ -815,12 +819,13 @@ void wilc_frmw_to_host(struct wilc *wilc, u8 *buff, u32 size, unsigned int frame_len = 0; struct wilc_vif *vif; struct sk_buff *skb; + int srcu_idx; int stats; if (!wilc) return; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wilc->srcu); wilc_netdev = get_if_handler(wilc, buff); if (!wilc_netdev) goto out; @@ -848,14 +853,15 @@ void wilc_frmw_to_host(struct wilc *wilc, u8 *buff, u32 size, netdev_dbg(wilc_netdev, "netif_rx ret value is: %d\n", stats); } out: - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); } void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size, bool is_auth) { + int srcu_idx; struct wilc_vif *vif; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wilc->srcu); wilc_for_each_vif(wilc, vif) { struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)buff; u16 type = le16_to_cpup((__le16 *)buff); @@ -876,7 +882,7 @@ void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size, bool is_auth) if (vif->monitor_flag) wilc_wfi_monitor_rx(wilc->monitor_dev, buff, size); } - rcu_read_unlock(); + srcu_read_unlock(&wilc->srcu, srcu_idx); } static const struct net_device_ops wilc_netdev_ops = { @@ -906,7 +912,7 @@ void wilc_netdev_cleanup(struct wilc *wilc) list_del_rcu(&vif->list); wilc->vif_num--; mutex_unlock(&wilc->vif_mutex); - synchronize_rcu(); + synchronize_srcu(&wilc->srcu); if (vif->ndev) unregister_netdev(vif->ndev); } @@ -925,15 +931,16 @@ static u8 wilc_get_available_idx(struct wilc *wl) { int idx = 0; struct wilc_vif *vif; + int srcu_idx; - rcu_read_lock(); + srcu_idx = srcu_read_lock(&wl->srcu); wilc_for_each_vif(wl, vif) { if (vif->idx == 0) idx = 1; else idx = 0; } - rcu_read_unlock(); + srcu_read_unlock(&wl->srcu, srcu_idx); return idx; } @@ -983,7 +990,7 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name, list_add_tail_rcu(&vif->list, &wl->vif_list); wl->vif_num += 1; mutex_unlock(&wl->vif_mutex); - synchronize_rcu(); + synchronize_srcu(&wl->srcu); return vif; |