lkml.org 
[lkml]   [2022]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 05/10] rtw88: iterate over vif/sta list non-atomically
On Wed, Jun 08, 2022 at 01:25:34PM +0000, Ping-Ke Shih wrote:
> Hi Martin and Sascha,
>
> Thank you both.
>
> On Wed, 2022-06-08 at 00:06 +0200, Martin Blumenstingl wrote:
> > Hi Sascha,
> >
> > thanks for this patch!
> >
> > On Mon, May 30, 2022 at 3:55 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > [...]
> > > drivers/net/wireless/realtek/rtw88/phy.c | 6 +-
> > > drivers/net/wireless/realtek/rtw88/ps.c | 2 +-
> > > drivers/net/wireless/realtek/rtw88/util.c | 103 ++++++++++++++++++++++
> > > drivers/net/wireless/realtek/rtw88/util.h | 12 ++-
> > > 4 files changed, 116 insertions(+), 7 deletions(-)
> > I compared the changes from this patch with my earlier work. I would
> > like to highlight a few functions to understand if they were left out
> > on purpose or by accident.
> >
> > __fw_recovery_work() in main.c (unfortunately I am not sure how to
> > trigger/test this "firmware recovery" logic):
>
> This can be triggered by 'echo 1 > /sys/kernel/debug/ieee80211/rtw88/fw_crash',
> but only the latest firmware of 8822c can support this.
>
> > - this is already called with &rtwdev->mutex held
> > - it uses rtw_iterate_keys_rcu() (which internally uses rtw_write32
> > from rtw_sec_clear_cam). feel free to either add [0] to your series or
> > even squash it into an existing patch
>
> ieee80211_iter_keys() check lockdep_assert_wiphy(hw->wiphy), but we don't
> hold the lock in this work; it also do mutex_lock(&local->key_mtx) that
> I'm afraid it could cause deadlock.
>
> So, I think we can use _rcu version to collect key list like sta and vif.
>
> > - it uses rtw_iterate_stas_atomic() (which internally uses
> > rtw_fw_send_h2c_command from rtw_fw_media_status_report)
> > - it uses rtw_iterate_vifs_atomic() (which internally can read/write
> > registers from rtw_chip_config_bfee)
> > - in my previous series I simply replaced the
> > rtw_iterate_stas_atomic() and rtw_iterate_vifs_atomic() calls with the
> > non-atomic variants (for the rtw_iterate_keys_rcu() call I did some
> > extra cleanup, see [0])
> >
> > rtw_wow_fw_media_status() in wow.c (unfortunately I am also not sure
> > how to test WoWLAN):
> > - I am not sure if &rtwdev->mutex is held when this function is called
> > - it uses rtw_iterate_stas_atomic() (which internally uses
> > rtw_fw_send_h2c_command from rtw_fw_media_status_report)
> > - in my previous series I simply replaced rtw_iterate_stas_atomic()
> > with it's non-atomic variant
> >
> > Additionally I rebased my SDIO work on top of your USB series.
> > This makes SDIO support a lot easier - so thank you for your work!
> > I found that three of my previous patches (in addition to [0] which I
> > already mentioned earlier) are still needed to get rid of some
> > warnings when using the SDIO interface (the same warnings may or may
> > not be there with the USB interface - it just depends on whether your
> > AP makes rtw88 hit that specific code-path):
> > - [1]: rtw88: Configure the registers from rtw_bf_assoc() outside the RCU lock
>
> I think we need this.
>
> > - [2]: rtw88: Move rtw_chip_cfg_csi_rate() out of rtw_vif_watch_dog_iter()
>
> I think we don't need this, but just use rtw_iterate_vifs() to
> iterate rtw_vif_watch_dog_iter.
>
> > - [3]: rtw88: Move rtw_update_sta_info() out of rtw_ra_mask_info_update_iter()
>
> Need partial -- hold rtwdev->mutex before entering rtw_ra_mask_info_update().
>
> Then, use rtw_iterate_stas() to iterate rtw_ra_mask_info_update_iter.
> No need others.
>
>
> Sascha, could you squash Martin's patches into your patchset?
> And, then I can do more tests on PCI cards.

Yes, will do and send a v3 with it.

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 |

\
 
 \ /
  Last update: 2022-06-08 15:44    [W:0.043 / U:0.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site