This series consists of three patches which are fixing existing
behavior (meaning: it either affects PCIe or USB or both) in the rtw88
driver.
The first change adds the packed attribute to the eFuse structs. This
was spotted by Ping-Ke while reviewing the SDIO support patches from
[0].
The remaining three changes relate to locking (barrier hold) problems.
We previously had discussed patches for this for 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).
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
[0] https://lore.kernel.org/linux-wireless/[email protected]/
[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/[email protected]/
Martin Blumenstingl (4):
rtw88: Add packed attribute to the eFuse structs
rtw88: Configure the registers from rtw_bf_assoc() outside the RCU
lock
rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter()
rtw88: Use non-atomic rtw_iterate_stas() 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 ++++--
drivers/net/wireless/realtek/rtw88/main.h | 6 +++---
drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 +++---
drivers/net/wireless/realtek/rtw88/rtw8821c.h | 9 +++++----
drivers/net/wireless/realtek/rtw88/rtw8822b.h | 9 +++++----
drivers/net/wireless/realtek/rtw88/rtw8822c.h | 9 +++++----
8 files changed, 35 insertions(+), 27 deletions(-)
--
2.39.0
The eFuse definitions in the rtw88 are using structs to describe the
eFuse contents. Add the packed attribute to all structs used for the
eFuse description so the compiler doesn't add gaps or re-order
attributes.
Also split the res2 eFuse field (which for some reason has parts of it's
data in two separate u8 fields) to avoid the following warning, now that
their surrounding struct has the packed attribute:
note: offset of packed bit-field 'res2' has changed in GCC 4.4
Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Fixes: ab0a031ecf29 ("rtw88: 8723d: Add read_efuse to recognize efuse info from map")
Fixes: 769a29ce2af4 ("rtw88: 8821c: add basic functions")
Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")
Fixes: aff5ffd718de ("wifi: rtw88: Add rtw8821cu chipset support")
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/wireless/realtek/rtw88/main.h | 6 +++---
drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 +++---
drivers/net/wireless/realtek/rtw88/rtw8821c.h | 9 +++++----
drivers/net/wireless/realtek/rtw88/rtw8822b.h | 9 +++++----
drivers/net/wireless/realtek/rtw88/rtw8822c.h | 9 +++++----
5 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 165f299e8e1f..8441c26680ad 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -438,7 +438,7 @@ struct rtw_2g_txpwr_idx {
struct rtw_2g_ns_pwr_idx_diff ht_2s_diff;
struct rtw_2g_ns_pwr_idx_diff ht_3s_diff;
struct rtw_2g_ns_pwr_idx_diff ht_4s_diff;
-};
+} __packed;
struct rtw_5g_ht_1s_pwr_idx_diff {
#ifdef __LITTLE_ENDIAN
@@ -495,12 +495,12 @@ struct rtw_5g_txpwr_idx {
struct rtw_5g_vht_ns_pwr_idx_diff vht_2s_diff;
struct rtw_5g_vht_ns_pwr_idx_diff vht_3s_diff;
struct rtw_5g_vht_ns_pwr_idx_diff vht_4s_diff;
-};
+} __packed;
struct rtw_txpwr_idx {
struct rtw_2g_txpwr_idx pwr_idx_2g;
struct rtw_5g_txpwr_idx pwr_idx_5g;
-};
+} __packed;
struct rtw_timer_list {
struct timer_list timer;
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.h b/drivers/net/wireless/realtek/rtw88/rtw8723d.h
index a356318a5c15..8160c4782457 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8723d.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.h
@@ -39,7 +39,7 @@ struct rtw8723de_efuse {
u8 device_id[2];
u8 sub_vender_id[2];
u8 sub_device_id[2];
-};
+} __packed;
struct rtw8723du_efuse {
u8 res4[48]; /* 0xd0 */
@@ -47,7 +47,7 @@ struct rtw8723du_efuse {
u8 product_id[2]; /* 0x102 */
u8 usb_option; /* 0x104 */
u8 mac_addr[ETH_ALEN]; /* 0x107 */
-};
+} __packed;
struct rtw8723d_efuse {
__le16 rtl_id;
@@ -81,7 +81,7 @@ struct rtw8723d_efuse {
struct rtw8723de_efuse e;
struct rtw8723du_efuse u;
};
-};
+} __packed;
extern const struct rtw_chip_info rtw8723d_hw_spec;
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.h b/drivers/net/wireless/realtek/rtw88/rtw8821c.h
index 1c81260f3a54..6ab16f56b5cd 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.h
@@ -27,7 +27,7 @@ struct rtw8821cu_efuse {
u8 res11[0xcf];
u8 package_type; /* 0x1fb */
u8 res12[0x4];
-};
+} __packed;
struct rtw8821ce_efuse {
u8 mac_addr[ETH_ALEN]; /* 0xd0 */
@@ -47,7 +47,8 @@ struct rtw8821ce_efuse {
u8 ltr_en:1;
u8 res1:2;
u8 obff:2;
- u8 res2:3;
+ u8 res2_1:1;
+ u8 res2_2:2;
u8 obff_cap:2;
u8 res3:4;
u8 res4[3];
@@ -63,7 +64,7 @@ struct rtw8821ce_efuse {
u8 res6:1;
u8 port_t_power_on_value:5;
u8 res7;
-};
+} __packed;
struct rtw8821c_efuse {
__le16 rtl_id;
@@ -95,7 +96,7 @@ struct rtw8821c_efuse {
struct rtw8821ce_efuse e;
struct rtw8821cu_efuse u;
};
-};
+} __packed;
static inline void
_rtw_write32s_mask(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32 data)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822b.h b/drivers/net/wireless/realtek/rtw88/rtw8822b.h
index 01d3644e0c94..35a937fc5162 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822b.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822b.h
@@ -27,7 +27,7 @@ struct rtw8822bu_efuse {
u8 res11[0xcf];
u8 package_type; /* 0x1fb */
u8 res12[0x4];
-};
+} __packed;
struct rtw8822be_efuse {
u8 mac_addr[ETH_ALEN]; /* 0xd0 */
@@ -47,7 +47,8 @@ struct rtw8822be_efuse {
u8 ltr_en:1;
u8 res1:2;
u8 obff:2;
- u8 res2:3;
+ u8 res2_1:1;
+ u8 res2_2:2;
u8 obff_cap:2;
u8 res3:4;
u8 res4[3];
@@ -63,7 +64,7 @@ struct rtw8822be_efuse {
u8 res6:1;
u8 port_t_power_on_value:5;
u8 res7;
-};
+} __packed;
struct rtw8822b_efuse {
__le16 rtl_id;
@@ -95,7 +96,7 @@ struct rtw8822b_efuse {
struct rtw8822bu_efuse u;
struct rtw8822be_efuse e;
};
-};
+} __packed;
static inline void
_rtw_write32s_mask(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32 data)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.h b/drivers/net/wireless/realtek/rtw88/rtw8822c.h
index 479d5d769c52..868c0e6825e5 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822c.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.h
@@ -14,7 +14,7 @@ struct rtw8822cu_efuse {
u8 res1[3];
u8 mac_addr[ETH_ALEN]; /* 0x157 */
u8 res2[0x3d];
-};
+} __packed;
struct rtw8822ce_efuse {
u8 mac_addr[ETH_ALEN]; /* 0x120 */
@@ -34,7 +34,8 @@ struct rtw8822ce_efuse {
u8 ltr_en:1;
u8 res1:2;
u8 obff:2;
- u8 res2:3;
+ u8 res2_1:1;
+ u8 res2_2:2;
u8 obff_cap:2;
u8 res3:4;
u8 class_code[3];
@@ -50,7 +51,7 @@ struct rtw8822ce_efuse {
u8 res6:1;
u8 port_t_power_on_value:5;
u8 res7;
-};
+} __packed;
struct rtw8822c_efuse {
__le16 rtl_id;
@@ -94,7 +95,7 @@ struct rtw8822c_efuse {
struct rtw8822cu_efuse u;
struct rtw8822ce_efuse e;
};
-};
+} __packed;
enum rtw8822c_dpk_agc_phase {
RTW_DPK_GAIN_CHECK,
--
2.39.0
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 <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
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
On 12/29/22 06:48, Martin Blumenstingl wrote:
> This series consists of three patches which are fixing existing
> behavior (meaning: it either affects PCIe or USB or both) in the rtw88
> driver.
>
> The first change adds the packed attribute to the eFuse structs. This
> was spotted by Ping-Ke while reviewing the SDIO support patches from
> [0].
>
> The remaining three changes relate to locking (barrier hold) problems.
> We previously had discussed patches for this for 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).
>
> 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
>
>
> [0] https://lore.kernel.org/linux-wireless/[email protected]/
> [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/[email protected]/
>
>
> Martin Blumenstingl (4):
> rtw88: Add packed attribute to the eFuse structs
> rtw88: Configure the registers from rtw_bf_assoc() outside the RCU
> lock
> rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter()
> rtw88: Use non-atomic rtw_iterate_stas() 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 ++++--
> drivers/net/wireless/realtek/rtw88/main.h | 6 +++---
> drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 +++---
> drivers/net/wireless/realtek/rtw88/rtw8821c.h | 9 +++++----
> drivers/net/wireless/realtek/rtw88/rtw8822b.h | 9 +++++----
> drivers/net/wireless/realtek/rtw88/rtw8822c.h | 9 +++++----
> 8 files changed, 35 insertions(+), 27 deletions(-)
>
Martin,
I do not feel qualified to review these contributions, but I have some suggestions.
The first is that the subject should start with wifi: rtw88: .... That is a
fairly recent change that you likely did not catch.
My second comment is that changed patches should have a version number to
identify that they are new patches. You can generate the correct form using the
"-v N" option in 'git format-email'. Once you have generated the patches, you
should then edit them to indicate what change was made to each patch in the
various versions. Such explanations should go below the --- following the
Signed-off-by line, and end with another ---. With these additions, the
community, and more importantly Kalle, can keep track of the various versions,
and know what reviewer's comments have been addressed.
I know of several people that have asked about SDIO versions of these drivers.
They will be pleased to see them become available.
Larry
Hello Larry,
On Thu, Dec 29, 2022 at 6:41 PM Larry Finger <[email protected]> wrote:
[...]
> I do not feel qualified to review these contributions, but I have some suggestions.
>
> The first is that the subject should start with wifi: rtw88: .... That is a
> fairly recent change that you likely did not catch.
Oh, this is something that I missed. I'll wait until tomorrow to see
if I can get Ping-Ke's Reviewed-by on patch 1 and then re-send the
whole series with fixed subjects.
> My second comment is that changed patches should have a version number to
> identify that they are new patches.
This series had four patches from the beginning. So no patches were
added/removed during the lifecycle of this patchset.
I think the cover-letter subject is a bit misleading as it contains
the words "SDIO support". In fact the issues (which are fixed by this
series) were found while working on SDIO support, but they also apply
to existing PCIe/USB support.
> [...] Once you have generated the patches, you
> should then edit them to indicate what change was made to each patch in the
> various versions. Such explanations should go below the --- following the
> Signed-off-by line, and end with another ---. With these additions, the
> community, and more importantly Kalle, can keep track of the various versions,
> and know what reviewer's comments have been addressed.
Noted. I will take care of this in v3 along with the updated subjects.
> I know of several people that have asked about SDIO versions of these drivers.
> They will be pleased to see them become available.
Thanks for the motivating words :-)
Best regards,
Martin
On Thu, 2022-12-29 at 13:48 +0100, Martin Blumenstingl wrote:
> The eFuse definitions in the rtw88 are using structs to describe the
> eFuse contents. Add the packed attribute to all structs used for the
> eFuse description so the compiler doesn't add gaps or re-order
> attributes.
>
> Also split the res2 eFuse field (which for some reason has parts of it's
> data in two separate u8 fields) to avoid the following warning, now that
> their surrounding struct has the packed attribute:
> note: offset of packed bit-field 'res2' has changed in GCC 4.4
>
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Fixes: ab0a031ecf29 ("rtw88: 8723d: Add read_efuse to recognize efuse info from map")
> Fixes: 769a29ce2af4 ("rtw88: 8821c: add basic functions")
> Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")
> Fixes: aff5ffd718de ("wifi: rtw88: Add rtw8821cu chipset support")
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
Thanks for your work!
On Thu, 2022-12-29 at 13:48 +0100, Martin Blumenstingl wrote:
> 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 <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
I think my reviewed-by should behind your signed-off-by.
> ---
> 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,
On Thu, Dec 29, 2022 at 01:48:41PM +0100, Martin Blumenstingl wrote:
> This series consists of three patches which are fixing existing
> behavior (meaning: it either affects PCIe or USB or both) in the rtw88
> driver.
>
> The first change adds the packed attribute to the eFuse structs. This
> was spotted by Ping-Ke while reviewing the SDIO support patches from
> [0].
>
> The remaining three changes relate to locking (barrier hold) problems.
> We previously had discussed patches for this for 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).
Indeed I haven't stumbled over any of the issues you fix in this series.
I briefly looked into it and it might be that at least some of the code
paths are only used with beam forming support. Could it be that my
cheapo USB sticks do not support that?
For what it's worth, I gave this series a quick test and didn't see any
regressions, so:
Tested-by: Sascha Hauer <[email protected]>
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Ping-Ke,
On Fri, Dec 30, 2022 at 12:48 AM Ping-Ke Shih <[email protected]> wrote:
[...]
> > Reviewed-by: Ping-Ke Shih <[email protected]>
> > Signed-off-by: Martin Blumenstingl <[email protected]>
>
> I think my reviewed-by should behind your signed-off-by.
My understanding is that I have to put your Reviewed-by above my
Signed-off-by since I added the Reviewed-by to the description.
If the maintainer adds your Reviewed-by while applying the patch to
the tree they will put your Reviewed-by between my Signed-off-by and
the maintainer's Signed-off-by.
If this is incorrect then please let me know and I'll change it for v3.
Best regards,
Martin
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Wednesday, January 4, 2023 11:44 PM
> To: Ping-Ke Shih <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 2/4] rtw88: Configure the registers from rtw_bf_assoc() outside the RCU lock
>
> Hi Ping-Ke,
>
> On Fri, Dec 30, 2022 at 12:48 AM Ping-Ke Shih <[email protected]> wrote:
> [...]
> > > Reviewed-by: Ping-Ke Shih <[email protected]>
> > > Signed-off-by: Martin Blumenstingl <[email protected]>
> >
> > I think my reviewed-by should behind your signed-off-by.
> My understanding is that I have to put your Reviewed-by above my
> Signed-off-by since I added the Reviewed-by to the description.
> If the maintainer adds your Reviewed-by while applying the patch to
> the tree they will put your Reviewed-by between my Signed-off-by and
> the maintainer's Signed-off-by.
>
> If this is incorrect then please let me know and I'll change it for v3.
>
My original thought is to add my reviewed-by in the order like maintainer
applies the patch, but your understanding looks reasonable. Sorry for the noise.
Ping-Ke
In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times.
I'm using the Realtek RTL8822CE PCIe module in access point mode.
Below a dump of the first lock up. There is no log from the other two lockups, possibly because of
'options rtw88_pci disable_aspm=1' in rtw88_pci.conf
I hope this is of any use to you.
Best regards,
Gert Erkelens
Dec 29 22:24:29 shuttle kernel: [98328.813880] BUG: scheduling while atomic:
kworker/u4:0/7592/0x00000700
Dec 29 22:24:29 shuttle kernel: [98328.813889] Modules linked in: tls ccm xt_nat xt_state
xt_MASQUERADE nft_reject_ipv4 nft_reject nft_ct nft_masq nft_chain_nat nf_nat_h323 nf_conntrack_h323
nf_nat_pptp nf_conntrack_pptp nf_nat_tftp nf_conntrack_tftp nf_nat_sip nf_conntrack_sip nf_nat_irc
nf_conntrack_irc nf_nat_ftp nf_conntrack_ftp iptable_nat nf_nat ip6t_REJECT nf_reject_ipv6 xt_hl
ip6_tables ip6t_rt ipt_REJECT nf_reject_ipv4 xt_LOG nf_log_syslog nft_limit binfmt_misc xt_limit
xt_addrtype xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nft_compat nft_counter
nf_tables nfnetlink nls_iso8859_1 snd_sof_pci_intel_cnl snd_sof_intel_hda_common soundwire_intel
rtw88_8822ce soundwire_generic_allocation rtw88_8822c soundwire_cadence snd_sof_intel_hda rtw88_pci
snd_sof_pci intel_tcc_cooling snd_sof_xtensa_dsp x86_pkg_temp_thermal intel_powerclamp rtw88_core
coretemp snd_sof snd_soc_hdac_hda snd_hda_ext_core mac80211 snd_soc_acpi_intel_match kvm_intel
snd_soc_acpi mei_hdcp intel_rapl_msr soundwire_bus kvm
Dec 29 22:24:29 shuttle kernel: [98328.813949] snd_hda_codec_hdmi snd_hda_codec_realtek
snd_hda_codec_generic ledtrig_audio snd_soc_core snd_compress ac97_bus snd_pcm_dmaengine
snd_hda_intel snd_intel_dspcfg btusb rapl intel_cstate snd_intel_sdw_acpi btrtl cfg80211
snd_hda_codec btbcm snd_hda_core btintel snd_hwdep snd_pcm wmi_bmof
processor_thermal_device_pci_legacy snd_timer processor_thermal_device bluetooth libarc4
processor_thermal_rfim snd ee1004 processor_thermal_mbox ecdh_generic soundcore
processor_thermal_rapl mei_me ecc intel_rapl_common mei int340x_thermal_zone intel_pch_thermal
intel_soc_dts_iosf mac_hid acpi_pad acpi_tad ramoops pstore_blk reed_solomon pstore_zone efi_pstore
dm_multipath scsi_dh_rdac scsi_dh_emc sch_fq_codel scsi_dh_alua msr ip_tables x_tables autofs4 btrfs
blake2b_generic zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor
async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear i915 ttm drm_kms_helper syscopyarea
sysfillrect sysimgblt fb_sys_fops
Dec 29 22:24:29 shuttle kernel: [98328.814014] crct10dif_pclmul cec crc32_pclmul
ghash_clmulni_intel igb rc_core aesni_intel i2c_i801 drm crypto_simd cryptd dca i2c_smbus
i2c_algo_bit e1000e xhci_pci xhci_pci_renesas ahci libahci wmi video pinctrl_cannonlake
Dec 29 22:24:29 shuttle kernel: [98328.814032] CPU: 0 PID: 7592 Comm: kworker/u4:0 Not tainted
5.15.0-56-generic #62-Ubuntu
Dec 29 22:24:29 shuttle kernel: [98328.814037] Hardware name: Shuttle Inc. DS10U/DS10U, BIOS 1.07
11/18/2022
Dec 29 22:24:29 shuttle kernel: [98328.814038] Workqueue: phy0 rtw_watch_dog_work [rtw88_core]
Dec 29 22:24:29 shuttle kernel: [98328.814054] Call Trace:
Dec 29 22:24:29 shuttle kernel: [98328.814056] <IRQ>
Dec 29 22:24:29 shuttle kernel: [98328.814059] show_stack+0x52/0x5c
Dec 29 22:24:29 shuttle kernel: [98328.814064] dump_stack_lvl+0x4a/0x63
Dec 29 22:24:29 shuttle kernel: [98328.814070] dump_stack+0x10/0x16
Dec 29 22:24:29 shuttle kernel: [98328.814074] __schedule_bug.cold+0x4e/0x60
Dec 29 22:24:29 shuttle kernel: [98328.814077] __schedule+0x449/0x590
Dec 29 22:24:29 shuttle kernel: [98328.814081] schedule+0x69/0x110
Dec 29 22:24:29 shuttle kernel: [98328.814084] schedule_preempt_disabled+0xe/0x20
Dec 29 22:24:29 shuttle kernel: [98328.814088] __mutex_lock.constprop.0+0x484/0x490
Dec 29 22:24:29 shuttle kernel: [98328.814091] ? __qdisc_run+0x96/0xe0
Dec 29 22:24:29 shuttle kernel: [98328.814097] __mutex_lock_slowpath+0x13/0x20
Dec 29 22:24:29 shuttle kernel: [98328.814100] mutex_lock+0x38/0x50
Dec 29 22:24:29 shuttle kernel: [98328.814104] rtw_ops_set_tim+0x20/0x40 [rtw88_core]
Dec 29 22:24:29 shuttle kernel: [98328.814117] __sta_info_recalc_tim+0x1a1/0x3d0 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814172] sta_info_recalc_tim+0x10/0x20 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814221] ieee80211_tx_h_unicast_ps_buf+0x225/0x310 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814283] invoke_tx_handlers_early+0xbf/0x360 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814341] ieee80211_tx+0x9c/0x160 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814398] ieee80211_xmit+0xc0/0x100 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814454] __ieee80211_subif_start_xmit+0x1f6/0x360 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814509] ? consume_skb+0x43/0xb0
Dec 29 22:24:29 shuttle kernel: [98328.814515] ieee80211_subif_start_xmit+0x47/0x190 [mac80211]
Dec 29 22:24:29 shuttle kernel: [98328.814570] ? dev_queue_xmit_nit+0x275/0x2b0
Dec 29 22:24:29 shuttle kernel: [98328.814574] xmit_one.constprop.0+0x99/0x160
Dec 29 22:24:29 shuttle kernel: [98328.814576] dev_hard_start_xmit+0x45/0x90
Dec 29 22:24:29 shuttle kernel: [98328.814580] __dev_queue_xmit+0x46d/0x510
Dec 29 22:24:29 shuttle kernel: [98328.814583] ? neigh_add_timer+0x37/0x60
Dec 29 22:24:29 shuttle kernel: [98328.814588] dev_queue_xmit+0x10/0x20
Dec 29 22:24:29 shuttle kernel: [98328.814591] neigh_resolve_output+0x114/0x1b0
Dec 29 22:24:29 shuttle kernel: [98328.814594] ip_finish_output2+0x17a/0x460
Dec 29 22:24:29 shuttle kernel: [98328.814597] __ip_finish_output+0xb7/0x180
Dec 29 22:24:29 shuttle kernel: [98328.814599] ip_finish_output+0x2e/0xc0
Dec 29 22:24:29 shuttle kernel: [98328.814602] ip_output+0x78/0x100
Dec 29 22:24:29 shuttle kernel: [98328.814604] ? __ip_finish_output+0x180/0x180
Dec 29 22:24:29 shuttle kernel: [98328.814606] ip_forward_finish+0x8b/0xb0
Dec 29 22:24:29 shuttle kernel: [98328.814612] ip_forward+0x3fc/0x550
Dec 29 22:24:29 shuttle kernel: [98328.814616] ? __build_flow_key.constprop.0+0x92/0xf0
Dec 29 22:24:29 shuttle kernel: [98328.814620] ? ip_expire+0x1a0/0x1a0
Dec 29 22:24:29 shuttle kernel: [98328.814624] ip_sublist_rcv_finish+0x6f/0x80
Dec 29 22:24:29 shuttle kernel: [98328.814628] ip_sublist_rcv+0x17c/0x200
Dec 29 22:24:29 shuttle kernel: [98328.814633] ? ip_sublist_rcv+0x200/0x200
Dec 29 22:24:29 shuttle kernel: [98328.814637] ip_list_rcv+0xf9/0x120
Dec 29 22:24:29 shuttle kernel: [98328.814641] __netif_receive_skb_list_core+0x218/0x240
Dec 29 22:24:29 shuttle kernel: [98328.814645] netif_receive_skb_list_internal+0x18e/0x2a0
Dec 29 22:24:29 shuttle kernel: [98328.814649] ? e1000_clean_rx_irq+0x34d/0x3a0 [e1000e]
Dec 29 22:24:29 shuttle kernel: [98328.814669] napi_complete_done+0x7a/0x1c0
Dec 29 22:24:29 shuttle kernel: [98328.814672] e1000e_poll+0xcf/0x2d0 [e1000e]
Dec 29 22:24:29 shuttle kernel: [98328.814690] __napi_poll+0x30/0x180
Dec 29 22:24:29 shuttle kernel: [98328.814693] net_rx_action+0x126/0x280
Dec 29 22:24:29 shuttle kernel: [98328.814697] __do_softirq+0xd6/0x2e7
Dec 29 22:24:29 shuttle kernel: [98328.814701] irq_exit_rcu+0x94/0xc0
Dec 29 22:24:29 shuttle kernel: [98328.814706] common_interrupt+0x8e/0xa0
Dec 29 22:24:29 shuttle kernel: [98328.814710] </IRQ>
Dec 29 22:24:29 shuttle kernel: [98328.814712] <TASK>
Dec 29 22:24:29 shuttle kernel: [98328.814713] asm_common_interrupt+0x27/0x40
Dec 29 22:24:29 shuttle kernel: [98328.814715] RIP: 0010:rtw_pci_read32+0x14/0x20 [rtw88_pci]
Dec 29 22:24:29 shuttle kernel: [98328.814721] Code: b7 30 7a 00 00 48 89 e5 66 8b 06 5d c3 cc cc cc
cc 0f 1f 44 00 00 0f 1f 44 00 00 55 89 f6 48 03 b7 30 7a 00 00 48 89 e5 8b 06 <5d> c3 cc cc cc cc 66
0f 1f 44 00 00 0f 1f 44 00 00 55 89 f6 48 03
Dec 29 22:24:29 shuttle kernel: [98328.814724] RSP: 0018:ffffbff902427da0 EFLAGS: 00000282
Dec 29 22:24:29 shuttle kernel: [98328.814727] RAX: 00000000c0000000 RBX: 0000000000000002 RCX:
000000000000000c
Dec 29 22:24:29 shuttle kernel: [98328.814729] RDX: 0000000012baa000 RSI: ffffbff900a11d2c RDI:
ffff9b370b9b2020
Dec 29 22:24:29 shuttle kernel: [98328.814731] RBP: ffffbff902427da0 R08: 0000000000000000 R09:
0000000000000000
Dec 29 22:24:29 shuttle kernel: [98328.814732] R10: 0000000000000005 R11: 0000000000000000 R12:
ffff9b370b9b2020
Dec 29 22:24:29 shuttle kernel: [98328.814734] R13: 0000000000000000 R14: 0000000000000000 R15:
0000000000000000
Dec 29 22:24:29 shuttle kernel: [98328.814737] rtw8822c_false_alarm_statistics+0x26d/0x350 [rtw88_8822c]
Dec 29 22:24:29 shuttle kernel: [98328.814744] rtw_phy_dynamic_mechanism+0x71/0x320 [rtw88_core]
Dec 29 22:24:29 shuttle kernel: [98328.814761] rtw_watch_dog_work+0x1cd/0x260 [rtw88_core]
Dec 29 22:24:29 shuttle kernel: [98328.814773] process_one_work+0x228/0x3d0
Dec 29 22:24:29 shuttle kernel: [98328.814776] worker_thread+0x53/0x420
Dec 29 22:24:29 shuttle kernel: [98328.814779] ? process_one_work+0x3d0/0x3d0
Dec 29 22:24:29 shuttle kernel: [98328.814781] kthread+0x127/0x150
Dec 29 22:24:29 shuttle kernel: [98328.814785] ? set_kthread_struct+0x50/0x50
Dec 29 22:24:29 shuttle kernel: [98328.814789] ret_from_fork+0x1f/0x30
Dec 29 22:24:29 shuttle kernel: [98328.814796] </TASK>
Dec 29 22:24:29 shuttle kernel: [98328.814811] NOHZ tick-stop error: Non-RCU local softirq work is
pending, handler #88!!!
Dec 29 22:24:35 shuttle kernel: [98333.925935] Dead loop on virtual device wlp2s0, fix it urgently!
Dec 29 22:24:36 shuttle kernel: [98334.949866] Dead loop on virtual device wlp2s0, fix it urgently!
Dec 29 22:24:37 shuttle kernel: [98335.974049] Dead loop on virtual device wlp2s0, fix it urgently!
Hi Gert,
On Sat, Jan 7, 2023 at 7:23 PM gert erkelens <[email protected]> wrote:
>
>
> In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times.
> I'm using the Realtek RTL8822CE PCIe module in access point mode.
> Below a dump of the first lock up. There is no log from the other two lockups, possibly because of
> 'options rtw88_pci disable_aspm=1' in rtw88_pci.conf
>
> I hope this is of any use to you.
Did you try patches 2-4 from this series? Patch 3 should solve this
specific issue.
Best regards,
Martin
> -----Original Message-----
> From: gert erkelens <[email protected]>
> Sent: Sunday, January 8, 2023 2:23 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Ping-Ke Shih <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH 0/4] rtw88: Four fixes found while working on SDIO support
>
> In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times.
> I'm using the Realtek RTL8822CE PCIe module in access point mode.
> Below a dump of the first lock up. There is no log from the other two lockups, possibly because of
> 'options rtw88_pci disable_aspm=1' in rtw88_pci.conf
>
> I hope this is of any use to you.
>
> Best regards,
> Gert Erkelens
>
>
> Dec 29 22:24:29 shuttle kernel: [98328.813880] BUG: scheduling while atomic:
> kworker/u4:0/7592/0x00000700
[...]
> Dec 29 22:24:29 shuttle kernel: [98328.814032] CPU: 0 PID: 7592 Comm: kworker/u4:0 Not tainted
> 5.15.0-56-generic #62-Ubuntu
The trace below is very similar to this fix
7711fe713a49 ("wifi: rtw88: add a work to correct atomic scheduling warning of ::set_tim")
Please check if the driver you are using includes it.
By the way, ::set_time is added after 5.19, but your kernel is 5.15.0-56-generic
Ping-Ke