Skip to content

Commit 059d0e3

Browse files
TropicaoKalle Valo
authored andcommitted
wifi: wilc1000: use SRCU instead of RCU for vif list traversal
Enabling CONFIG_PROVE_RCU_LIST raises many warnings in wilc driver, even on some places already protected by a read critical section. An example of such case is in wilc_get_available_idx: ============================= WARNING: suspicious RCU usage 6.8.0-rc1+ #32 Not tainted ----------------------------- drivers/net/wireless/microchip/wilc1000/netdev.c:944 RCU-list traversed in non-reader section!! [...] stack backtrace: CPU: 0 PID: 26 Comm: kworker/0:3 Not tainted 6.8.0-rc1+ #32 Hardware name: Atmel SAMA5 Workqueue: events_freezable mmc_rescan unwind_backtrace from show_stack+0x18/0x1c show_stack from dump_stack_lvl+0x34/0x58 dump_stack_lvl from wilc_netdev_ifc_init+0x788/0x8ec wilc_netdev_ifc_init from wilc_cfg80211_init+0x690/0x910 wilc_cfg80211_init from wilc_sdio_probe+0x168/0x490 wilc_sdio_probe from sdio_bus_probe+0x230/0x3f4 sdio_bus_probe from really_probe+0x270/0xdf4 really_probe from __driver_probe_device+0x1dc/0x580 __driver_probe_device from driver_probe_device+0x60/0x140 driver_probe_device from __device_attach_driver+0x268/0x364 __device_attach_driver from bus_for_each_drv+0x15c/0x1cc bus_for_each_drv from __device_attach+0x1ec/0x3e8 __device_attach from bus_probe_device+0x190/0x1c0 bus_probe_device from device_add+0x10dc/0x18e4 device_add from sdio_add_func+0x1c0/0x2c0 sdio_add_func from mmc_attach_sdio+0xa08/0xe1c mmc_attach_sdio from mmc_rescan+0xa00/0xfe0 mmc_rescan from process_one_work+0x8d4/0x169c process_one_work from worker_thread+0x8cc/0x1340 worker_thread from kthread+0x448/0x510 kthread from ret_from_fork+0x14/0x28 This warning is due to the section being protected by a srcu critical read section, but the list traversal being done with classic RCU API. Fix the warning by using corresponding SRCU read lock/unlock APIs. While doing so, since we always manipulate the same list (managed through a pointer embedded in struct_wilc), add a macro to reduce the corresponding boilerplate in each call site. Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> Signed-off-by: Kalle Valo <kvalo@kernel.org> Link: https://msgid.link/20240215-wilc_fix_rcu_usage-v1-2-f610e46c6f82@bootlin.com
1 parent 5d2dbcc commit 059d0e3

File tree

5 files changed

+16
-10
lines changed

5 files changed

+16
-10
lines changed

drivers/net/wireless/microchip/wilc1000/cfg80211.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1518,7 +1518,7 @@ static struct wilc_vif *wilc_get_vif_from_type(struct wilc *wl, int type)
15181518
{
15191519
struct wilc_vif *vif;
15201520

1521-
list_for_each_entry_rcu(vif, &wl->vif_list, list) {
1521+
wilc_for_each_vif(wl, vif) {
15221522
if (vif->iftype == type)
15231523
return vif;
15241524
}

drivers/net/wireless/microchip/wilc1000/hif.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ static struct wilc_vif *wilc_get_vif_from_idx(struct wilc *wilc, int idx)
107107
if (index < 0 || index >= WILC_NUM_CONCURRENT_IFC)
108108
return NULL;
109109

110-
list_for_each_entry_rcu(vif, &wilc->vif_list, list) {
110+
wilc_for_each_vif(wilc, vif) {
111111
if (vif->idx == index)
112112
return vif;
113113
}

drivers/net/wireless/microchip/wilc1000/netdev.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ static struct net_device *get_if_handler(struct wilc *wilc, u8 *mac_header)
9696
struct wilc_vif *vif;
9797
struct ieee80211_hdr *h = (struct ieee80211_hdr *)mac_header;
9898

99-
list_for_each_entry_rcu(vif, &wilc->vif_list, list) {
99+
wilc_for_each_vif(wilc, vif) {
100100
if (vif->iftype == WILC_STATION_MODE)
101101
if (ether_addr_equal_unaligned(h->addr2, vif->bssid)) {
102102
ndev = vif->ndev;
@@ -132,7 +132,7 @@ int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc)
132132
struct wilc_vif *vif;
133133

134134
srcu_idx = srcu_read_lock(&wilc->srcu);
135-
list_for_each_entry_rcu(vif, &wilc->vif_list, list) {
135+
wilc_for_each_vif(wilc, vif) {
136136
if (!is_zero_ether_addr(vif->bssid))
137137
ret_val++;
138138
}
@@ -146,7 +146,7 @@ static void wilc_wake_tx_queues(struct wilc *wl)
146146
struct wilc_vif *ifc;
147147

148148
srcu_idx = srcu_read_lock(&wl->srcu);
149-
list_for_each_entry_rcu(ifc, &wl->vif_list, list) {
149+
wilc_for_each_vif(wl, ifc) {
150150
if (ifc->mac_opened && netif_queue_stopped(ifc->ndev))
151151
netif_wake_queue(ifc->ndev);
152152
}
@@ -668,7 +668,7 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p)
668668
/* Verify MAC Address is not already in use: */
669669

670670
srcu_idx = srcu_read_lock(&wilc->srcu);
671-
list_for_each_entry_rcu(tmp_vif, &wilc->vif_list, list) {
671+
wilc_for_each_vif(wilc, tmp_vif) {
672672
wilc_get_mac_address(tmp_vif, mac_addr);
673673
if (ether_addr_equal(addr->sa_data, mac_addr)) {
674674
if (vif != tmp_vif) {
@@ -771,7 +771,7 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
771771
struct wilc_vif *vif;
772772

773773
srcu_idx = srcu_read_lock(&wilc->srcu);
774-
list_for_each_entry_rcu(vif, &wilc->vif_list, list) {
774+
wilc_for_each_vif(wilc, vif) {
775775
if (vif->mac_opened)
776776
netif_stop_queue(vif->ndev);
777777
}
@@ -858,7 +858,7 @@ void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size, bool is_auth)
858858
struct wilc_vif *vif;
859859

860860
srcu_idx = srcu_read_lock(&wilc->srcu);
861-
list_for_each_entry_rcu(vif, &wilc->vif_list, list) {
861+
wilc_for_each_vif(wilc, vif) {
862862
struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)buff;
863863
u16 type = le16_to_cpup((__le16 *)buff);
864864
u32 type_bit = BIT(type >> 4);
@@ -930,7 +930,7 @@ static u8 wilc_get_available_idx(struct wilc *wl)
930930
int srcu_idx;
931931

932932
srcu_idx = srcu_read_lock(&wl->srcu);
933-
list_for_each_entry_rcu(vif, &wl->vif_list, list) {
933+
wilc_for_each_vif(wl, vif) {
934934
if (vif->idx == 0)
935935
idx = 1;
936936
else

drivers/net/wireless/microchip/wilc1000/netdev.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <net/ieee80211_radiotap.h>
1414
#include <linux/if_arp.h>
1515
#include <linux/gpio/consumer.h>
16+
#include <linux/rculist.h>
1617

1718
#include "hif.h"
1819
#include "wlan.h"
@@ -29,6 +30,11 @@
2930

3031
#define TX_BACKOFF_WEIGHT_MS 1
3132

33+
#define wilc_for_each_vif(w, v) \
34+
struct wilc *_w = w; \
35+
list_for_each_entry_srcu(v, &_w->vif_list, list, \
36+
srcu_read_lock_held(&_w->srcu))
37+
3238
struct wilc_wfi_stats {
3339
unsigned long rx_packets;
3440
unsigned long tx_packets;

drivers/net/wireless/microchip/wilc1000/wlan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
725725
mutex_lock(&wilc->txq_add_to_head_cs);
726726

727727
srcu_idx = srcu_read_lock(&wilc->srcu);
728-
list_for_each_entry_rcu(vif, &wilc->vif_list, list)
728+
wilc_for_each_vif(wilc, vif)
729729
wilc_wlan_txq_filter_dup_tcp_ack(vif->ndev);
730730
srcu_read_unlock(&wilc->srcu, srcu_idx);
731731

0 commit comments

Comments
 (0)