linux: rtw88: Three locking fixes for existing code

fixes:

[   38.800687] ------------[ cut here ]------------
[   38.800703] Voluntary context switch within RCU read-side critical section!
[   38.800715] WARNING: CPU: 3 PID: 49 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x314/0x3ec
[   38.800740] Modules linked in: bnep rtw88_8822cs hci_uart rtw88_8822c btqca rtw88_sdio rtw88_core btrtl btbcm btintel crct10dif_ce mac80211 sunxi_cir bluetooth ecdh_generic libarc4 dwmac_sun8i ecc hantro_vpu v4l2_vp9 v4l2_h264 panfrost drm_shmem_helper gpu_sched sun50i_di cfg80211 rfkill pkcs8_key_parser fuse
[   38.800814] CPU: 3 PID: 49 Comm: kworker/u8:1 Not tainted 6.1.1 #1
[   38.800821] Hardware name: Tanix TX6 (DT)
[   38.800826] Workqueue: phy0 ieee80211_iface_work [mac80211]
[   38.800944] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   38.800950] pc : rcu_note_context_switch+0x314/0x3ec
[   38.800959] lr : rcu_note_context_switch+0x314/0x3ec
[   38.800965] sp : ffff8000098d3450
[   38.800968] x29: ffff8000098d3450 x28: ffff800000d71e90 x27: ffff00000f647400
[   38.800977] x26: 0000000000000000 x25: ffff8000098d36a8 x24: 0000000000000000
[   38.800986] x23: 0000000000000000 x22: ffff00000201d100 x21: ffff0000bf9ab100
[   38.800995] x20: ffff800009251e40 x19: ffff0000bf9abe40 x18: fffffffffffe7b30
[   38.801004] x17: 7a9deabfef448dbe x16: 0000000000004664 x15: fffffffffffe7b78
[   38.801013] x14: ffff800009400d38 x13: ffff800009400d90 x12: 0000000000000579
[   38.801022] x11: 00000000000001d3 x10: ffff80000945b958 x9 : ffff800009400d90
[   38.801031] x8 : 00000000ffffefff x7 : ffff800009458d90 x6 : 00000000000001d3
[   38.801039] x5 : ffff0000bf9a48a8 x4 : 0000000000000000 x3 : 0000000000000027
[   38.801048] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000201d100
[   38.801057] Call trace:
[   38.801060]  rcu_note_context_switch+0x314/0x3ec
[   38.801068]  __schedule+0xa4/0x6b0
[   38.801075]  schedule+0x58/0xc0
[   38.801081]  schedule_timeout+0xb8/0xec
[   38.801088]  wait_for_completion+0xb0/0x150
[   38.801093]  mmc_wait_for_req_done+0x68/0x9c
[   38.801103]  mmc_wait_for_req+0xa8/0xf4
[   38.801109]  mmc_wait_for_cmd+0x60/0x9c
[   38.801116]  mmc_io_rw_direct+0x98/0x12c
[   38.801124]  sdio_writeb+0x30/0x60
[   38.801131]  rtw_sdio_write8+0x7c/0x120 [rtw88_sdio]
[   38.801144]  rtw_bf_init_bfer_entry_mu+0x3c/0xc0 [rtw88_core]
[   38.801183]  rtw_bf_enable_bfee_mu+0x78/0x12c [rtw88_core]
[   38.801213]  rtw8822c_bf_config_bfee+0x44/0x10c [rtw88_8822c]
[   38.801229]  rtw_bf_assoc+0xec/0x210 [rtw88_core]
[   38.801258]  rtw_ops_bss_info_changed+0x27c/0x280 [rtw88_core]
[   38.801287]  ieee80211_bss_info_change_notify+0x100/0x184 [mac80211]
[   38.801363]  ieee80211_rx_mgmt_assoc_resp+0x16f4/0x1794 [mac80211]
[   38.801438]  ieee80211_sta_rx_queued_mgmt+0x250/0x9b0 [mac80211]
[   38.801513]  ieee80211_iface_work+0x2b4/0x3d0 [mac80211]
[   38.801589]  process_one_work+0x1cc/0x324
[   38.801597]  worker_thread+0x68/0x41c
[   38.801603]  kthread+0x104/0x110
[   38.801610]  ret_from_fork+0x10/0x20
[   38.801617] ---[ end trace 0000000000000000 ]---
This commit is contained in:
Rudi Heitbaum 2022-12-28 11:35:22 +00:00
parent 98507deb96
commit 844dc43e86

View File

@ -0,0 +1,303 @@
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: linux-wireless@vger.kernel.org
Cc: tony0620emma@gmail.com, kvalo@kernel.org, pkshih@realtek.com,
s.hauer@pengutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: [PATCH v3 0/3] wifi: rtw88: Three locking fixes for existing code
Date: Sun, 8 Jan 2023 22:13:21 +0100
Message-Id: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
X-Mailer: git-send-email 2.39.0
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org
This series consists of three patches which are fixing existing
behavior (meaning: it either affects PCIe or USB or both) in the rtw88
driver.
We previously had discussed patches for these locking issues while
working on SDIO support, but the problem never ocurred while testing
USB cards. It turns out that these are still needed and I think that
they also fix the same problems for USB users (it's not clear how often
it happens there though) - and possibly even PCIe card users.
The issue fixed by the second and third patches have been spotted by a
user who tested rtw88 SDIO support. Everything is working fine for him
but there are warnings [1] and [2] in the kernel log stating "Voluntary
context switch within RCU read-side critical section!".
The solution in the third and fourth patch was actually suggested by
Ping-Ke in [3]. Thanks again!
These fixes are indepdent of my other series adding SDIO support to the
rtw88 driver, meaning they can be added to the wireless driver tree on
top of Linux 6.2-rc1 or linux-next.
Changes since v1 at [4]:
- Keep the u8 bitfields in patch 1 but split the res2 field into res2_1
and res2_2 as suggested by Ping-Ke
- Added Ping-Ke's reviewed-by to patches 2-4 - thank you!
- Added a paragraph in the cover-letter to avoid confusion whether
these patches depend on the rtw88 SDIO support series
Changes since v2 at [5]:
- Added Ping-Ke's Reviewed-by and Sascha's Tested-by (thanks to both of
you!)
- Dropped patch 1/4 "rtw88: Add packed attribute to the eFuse structs"
This requires more discussion. I'll send a separate patch for this.
- Updated cover letter title so it's clear that this is independent of
SDIO support code
[0] https://lore.kernel.org/linux-wireless/695c976e02ed44a2b2345a3ceb226fc4@realtek.com/
[1] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366421445
[2] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366610249
[3] https://lore.kernel.org/lkml/e0aa1ba4336ab130712e1fcb425e6fd0adca4145.camel@realtek.com/
[4] https://lore.kernel.org/linux-wireless/20221228133547.633797-1-martin.blumenstingl@googlemail.com/
[5] https://lore.kernel.org/linux-wireless/20221229124845.1155429-1-martin.blumenstingl@googlemail.com/
Martin Blumenstingl (3):
wifi: rtw88: Move register access from rtw_bf_assoc() outside the RCU
wifi: rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter()
wifi: rtw88: Use non-atomic sta iterator in rtw_ra_mask_info_update()
drivers/net/wireless/realtek/rtw88/bf.c | 13 +++++++------
drivers/net/wireless/realtek/rtw88/mac80211.c | 4 +++-
drivers/net/wireless/realtek/rtw88/main.c | 6 ++++--
3 files changed, 14 insertions(+), 9 deletions(-)
--
2.39.0
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: linux-wireless@vger.kernel.org
Cc: tony0620emma@gmail.com, kvalo@kernel.org, pkshih@realtek.com,
s.hauer@pengutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: [PATCH v3 1/3] wifi: rtw88: Move register access from rtw_bf_assoc() outside the RCU
Date: Sun, 8 Jan 2023 22:13:22 +0100
Message-Id: <20230108211324.442823-2-martin.blumenstingl@googlemail.com>
X-Mailer: git-send-email 2.39.0
In-Reply-To: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
References: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org
USB and (upcoming) SDIO support may sleep in the read/write handlers.
Shrink the RCU critical section so it only cover the call to
ieee80211_find_sta() and finding the ic_vht_cap/vht_cap based on the
found station. This moves the chip's BFEE configuration outside the
rcu_read_lock section and thus prevent "scheduling while atomic" or
"Voluntary context switch within RCU read-side critical section!"
warnings when accessing the registers using an SDIO card (which is
where this issue has been spotted in the real world - but it also
affects USB cards).
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v1 -> v2:
- Added Ping-Ke's Reviewed-by (thank you!)
v2 -> v3:
- Added Sascha's Tested-by (thank you!)
- added "wifi" prefix to the subject and reworded the title accordingly
drivers/net/wireless/realtek/rtw88/bf.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/bf.c b/drivers/net/wireless/realtek/rtw88/bf.c
index 038a30b170ef..c827c4a2814b 100644
--- a/drivers/net/wireless/realtek/rtw88/bf.c
+++ b/drivers/net/wireless/realtek/rtw88/bf.c
@@ -49,19 +49,23 @@ void rtw_bf_assoc(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
sta = ieee80211_find_sta(vif, bssid);
if (!sta) {
+ rcu_read_unlock();
+
rtw_warn(rtwdev, "failed to find station entry for bss %pM\n",
bssid);
- goto out_unlock;
+ return;
}
ic_vht_cap = &hw->wiphy->bands[NL80211_BAND_5GHZ]->vht_cap;
vht_cap = &sta->deflink.vht_cap;
+ rcu_read_unlock();
+
if ((ic_vht_cap->cap & IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE) &&
(vht_cap->cap & IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE)) {
if (bfinfo->bfer_mu_cnt >= chip->bfer_mu_max_num) {
rtw_dbg(rtwdev, RTW_DBG_BF, "mu bfer number over limit\n");
- goto out_unlock;
+ return;
}
ether_addr_copy(bfee->mac_addr, bssid);
@@ -75,7 +79,7 @@ void rtw_bf_assoc(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
(vht_cap->cap & IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE)) {
if (bfinfo->bfer_su_cnt >= chip->bfer_su_max_num) {
rtw_dbg(rtwdev, RTW_DBG_BF, "su bfer number over limit\n");
- goto out_unlock;
+ return;
}
sound_dim = vht_cap->cap &
@@ -98,9 +102,6 @@ void rtw_bf_assoc(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
rtw_chip_config_bfee(rtwdev, rtwvif, bfee, true);
}
-
-out_unlock:
- rcu_read_unlock();
}
void rtw_bf_init_bfer_entry_mu(struct rtw_dev *rtwdev,
--
2.39.0
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: linux-wireless@vger.kernel.org
Cc: tony0620emma@gmail.com, kvalo@kernel.org, pkshih@realtek.com,
s.hauer@pengutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: [PATCH v3 2/3] wifi: rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter()
Date: Sun, 8 Jan 2023 22:13:23 +0100
Message-Id: <20230108211324.442823-3-martin.blumenstingl@googlemail.com>
X-Mailer: git-send-email 2.39.0
In-Reply-To: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
References: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org
USB and (upcoming) SDIO support may sleep in the read/write handlers.
Make rtw_watch_dog_work() use rtw_iterate_vifs() to prevent "scheduling
while atomic" or "Voluntary context switch within RCU read-side
critical section!" warnings when accessing the registers using an SDIO
card (which is where this issue has been spotted in the real world but
it also affects USB cards).
Fixes: 78d5bf925f30 ("wifi: rtw88: iterate over vif/sta list non-atomically")
Suggested-by: Ping-Ke Shih <pkshih@realtek.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v1 -> v2:
- no change
v2 -> v3:
- Added Ping-Ke's Reviewed-by (thank you!)
- Added Sascha's Tested-by (thank you!)
- added "wifi" prefix to the subject and reworded the title accordingly
drivers/net/wireless/realtek/rtw88/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 888427cf3bdf..b2e78737bd5d 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -241,8 +241,10 @@ static void rtw_watch_dog_work(struct work_struct *work)
rtw_phy_dynamic_mechanism(rtwdev);
data.rtwdev = rtwdev;
- /* use atomic version to avoid taking local->iflist_mtx mutex */
- rtw_iterate_vifs_atomic(rtwdev, rtw_vif_watch_dog_iter, &data);
+ /* rtw_iterate_vifs internally uses an atomic iterator which is needed
+ * to avoid taking local->iflist_mtx mutex
+ */
+ rtw_iterate_vifs(rtwdev, rtw_vif_watch_dog_iter, &data);
/* fw supports only one station associated to enter lps, if there are
* more than two stations associated to the AP, then we can not enter
--
2.39.0
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: linux-wireless@vger.kernel.org
Cc: tony0620emma@gmail.com, kvalo@kernel.org, pkshih@realtek.com,
s.hauer@pengutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: [PATCH v3 3/3] wifi: rtw88: Use non-atomic sta iterator in rtw_ra_mask_info_update()
Date: Sun, 8 Jan 2023 22:13:24 +0100
Message-Id: <20230108211324.442823-4-martin.blumenstingl@googlemail.com>
X-Mailer: git-send-email 2.39.0
In-Reply-To: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
References: <20230108211324.442823-1-martin.blumenstingl@googlemail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org
USB and (upcoming) SDIO support may sleep in the read/write handlers.
Use non-atomic rtw_iterate_stas() in rtw_ra_mask_info_update() because
the iterator function rtw_ra_mask_info_update_iter() needs to read and
write registers from within rtw_update_sta_info(). Using the non-atomic
iterator ensures that we can sleep during USB and SDIO register reads
and writes. This fixes "scheduling while atomic" or "Voluntary context
switch within RCU read-side critical section!" warnings as seen by SDIO
card users (but it also affects USB cards).
Fixes: 78d5bf925f30 ("wifi: rtw88: iterate over vif/sta list non-atomically")
Suggested-by: Ping-Ke Shih <pkshih@realtek.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v1 -> v2:
- Added Ping-Ke's Reviewed-by (thank you!)
v2 -> v3:
- Added Sascha's Tested-by (thank you!)
- added "wifi" prefix to the subject and reworded the title accordingly
drivers/net/wireless/realtek/rtw88/mac80211.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 776a9a9884b5..3b92ac611d3f 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -737,7 +737,7 @@ static void rtw_ra_mask_info_update(struct rtw_dev *rtwdev,
br_data.rtwdev = rtwdev;
br_data.vif = vif;
br_data.mask = mask;
- rtw_iterate_stas_atomic(rtwdev, rtw_ra_mask_info_update_iter, &br_data);
+ rtw_iterate_stas(rtwdev, rtw_ra_mask_info_update_iter, &br_data);
}
static int rtw_ops_set_bitrate_mask(struct ieee80211_hw *hw,
@@ -746,7 +746,9 @@ static int rtw_ops_set_bitrate_mask(struct ieee80211_hw *hw,
{
struct rtw_dev *rtwdev = hw->priv;
+ mutex_lock(&rtwdev->mutex);
rtw_ra_mask_info_update(rtwdev, vif, mask);
+ mutex_unlock(&rtwdev->mutex);
return 0;
}
--
2.39.0