[0/9] rtw88: prepare locking for SDIO support

Message

Martin Blumenstingl Dec. 28, 2021, 9:14 p.m. UTC

Hello rtw88 and mac80211 maintainers/contributors,

there is an ongoing effort where Jernej and I are working on adding
SDIO support to the rtw88 driver [0].
The hardware we use at the moment is RTL8822BS and RTL8822CS.
We are at a point where scanning, assoc etc. works (though it's not
fast yet, in my tests I got ~6Mbit/s in either direction).

This series contains some preparation work for adding SDIO support.
While testing our changes we found that there are some "scheduling
while atomic" errors in the kernel log. These are due to the fact
that the SDIO accessors (sdio_readb, sdio_writeb and friends) may
sleep internally.

Some background on why SDIO access (for example: sdio_writeb) cannot
be done with a spinlock held (this is a copy from my previous mail,
see [1]):
- when using for example sdio_writeb the MMC subsystem in Linux
  prepares a so-called MMC request
- this request is submitted to the MMC host controller hardware
- the host controller hardware forwards the MMC request to the card
- the card signals when it's done processing the request
- the MMC subsystem in Linux waits for the card to signal that it's
done processing the request in mmc_wait_for_req_done() -> this uses
wait_for_completion() internally, which might sleep (which is not
allowed while a spinlock is held)

Based on Ping-Ke's suggestion I came up with the code in this series.
The goal is to use non-atomic locking for all register access in the
rtw88 driver. One patch adds a new function to mac80211 which did not
have a "non-atomic" version of it's "atomic" counterpart yet.

As mentioned before I don't have any rtw88 PCIe device so I am unable
to test on that hardware.
I am sending this as an RFC series since I am new to the mac80211
subsystem as well as the rtw88 driver. So any kind of feedback is
very welcome!
The actual changes for adding SDIO support will be sent separately in
the future.


Changes since v1 at [2] (which I sent back in summer):
- patch #1: fixed kernel doc copy & paste (remove _atomic) as suggested
  by Ping-Ke and Johannes
- patch #1: added paragraph about driver authors having to be careful
  where they use this new function as suggested by Johannes
- patch #2 (new): keep rtw_iterate_vifs_atomic() to not undo the fix
  from commit 5b0efb4d670c8 ("rtw88: avoid circular locking between
  local->iflist_mtx and rtwdev->mutex") and instead call
  rtw_bf_cfg_csi_rate() from rtw_watch_dog_work() (outside the atomic
  section) as suggested by Ping-Ke.
- patch #3 (new): keep rtw_iterate_vifs_atomic() to prevent deadlocks
  as Johannes suggested. Keep track of all relevant stations inside
  rtw_ra_mask_info_update_iter() and the iter-data and then call
  rtw_update_sta_info() while held under rtwdev->mutex instead
- patch #7: shrink the critical section as suggested by Ping-Ke


[0] https://github.com/xdarklight/linux/tree/rtw88-sdio-locking-prep-linux-next-20211226
[1] https://lore.kernel.org/linux-wireless/[e-mail]/
[2] https://lore.kernel.org/netdev/[e-mail]/T/


Martin Blumenstingl (9):
  mac80211: Add stations iterator where the iterator function may sleep
  rtw88: Move rtw_chip_cfg_csi_rate() out of rtw_vif_watch_dog_iter()
  rtw88: Move rtw_update_sta_info() out of
    rtw_ra_mask_info_update_iter()
  rtw88: Use rtw_iterate_vifs where the iterator reads or writes
    registers
  rtw88: Use rtw_iterate_stas where the iterator reads or writes
    registers
  rtw88: Replace usage of rtw_iterate_keys_rcu() with rtw_iterate_keys()
  rtw88: Configure the registers from rtw_bf_assoc() outside the RCU
    lock
  rtw88: hci: Convert rf_lock from a spinlock to a mutex
  rtw88: fw: Convert h2c.lock from a spinlock to a mutex

 drivers/net/wireless/realtek/rtw88/bf.c       | 13 ++---
 drivers/net/wireless/realtek/rtw88/fw.c       | 14 +++---
 drivers/net/wireless/realtek/rtw88/hci.h      | 11 ++---
 drivers/net/wireless/realtek/rtw88/mac80211.c | 14 +++++-
 drivers/net/wireless/realtek/rtw88/main.c     | 47 +++++++++----------
 drivers/net/wireless/realtek/rtw88/main.h     |  4 +-
 drivers/net/wireless/realtek/rtw88/phy.c      |  4 +-
 drivers/net/wireless/realtek/rtw88/ps.c       |  2 +-
 drivers/net/wireless/realtek/rtw88/util.h     |  4 +-
 drivers/net/wireless/realtek/rtw88/wow.c      |  2 +-
 include/net/mac80211.h                        | 21 +++++++++
 net/mac80211/util.c                           | 13 +++++
 12 files changed, 94 insertions(+), 55 deletions(-)

Comments

Ping-Ke Shih Jan. 7, 2022, 9:19 a.m. UTC | #1

> -----Original Message-----
> From: Martin Blumenstingl <[e-mail]>
> Sent: Wednesday, December 29, 2021 5:15 AM
> To: [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Neo Jou <[e-mail]>; Jernej Skrabec <[e-mail]>;
> Pkshih <[e-mail]>; Martin Blumenstingl <[e-mail]>
> Subject: [PATCH 0/9] rtw88: prepare locking for SDIO support
> 
> Hello rtw88 and mac80211 maintainers/contributors,
> 
> there is an ongoing effort where Jernej and I are working on adding
> SDIO support to the rtw88 driver [0].
> The hardware we use at the moment is RTL8822BS and RTL8822CS.
> We are at a point where scanning, assoc etc. works (though it's not
> fast yet, in my tests I got ~6Mbit/s in either direction).

Could I know if you have improvement of this throughput issue?

I have done simple test of this patchset on RTL8822CE, and it works
well. But, I think I don't test all flows yet, so I will do more
test that will take a while. After that, I can give a Tested-by tag.

Thank you
Ping-Ke

Martin Blumenstingl Jan. 7, 2022, 9:49 p.m. UTC | #2

Hi Ping-Ke,

On Fri, Jan 7, 2022 at 10:19 AM Pkshih <[e-mail]> wrote:
>
>
> > -----Original Message-----
> > From: Martin Blumenstingl <[e-mail]>
> > Sent: Wednesday, December 29, 2021 5:15 AM
> > To: [e-mail]
> > Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; Neo Jou <[e-mail]>; Jernej Skrabec <[e-mail]>;
> > Pkshih <[e-mail]>; Martin Blumenstingl <[e-mail]>
> > Subject: [PATCH 0/9] rtw88: prepare locking for SDIO support
> >
> > Hello rtw88 and mac80211 maintainers/contributors,
> >
> > there is an ongoing effort where Jernej and I are working on adding
> > SDIO support to the rtw88 driver [0].
> > The hardware we use at the moment is RTL8822BS and RTL8822CS.
> > We are at a point where scanning, assoc etc. works (though it's not
> > fast yet, in my tests I got ~6Mbit/s in either direction).
>
> Could I know if you have improvement of this throughput issue?
Yes, in the meantime we have made some performance improvements.
Currently the throughput numbers are approx.:
TX: 30Mbit/s
RX: 20Mbit/s

I have seen RX and TX throughputs of up to 50Mbit/s on my RTL8822CS,
but I cannot reliably reproduce this (meaning: if I don't touch my
board and run the same iperf3 test again then in one run it may
achieve 50Mbit/s, but in the next run only 25Mbit/s).
In other words: throughput is much better than what we started with in
summer, but I think it can be improved further.

> I have done simple test of this patchset on RTL8822CE, and it works
> well. But, I think I don't test all flows yet, so I will do more
> test that will take a while. After that, I can give a Tested-by tag.
I also got feedback off-list from a user who used the patches from
this series on top of the out-of-tree rtw88-usb driver. These patches
fix one "scheduling while atomic" issue for him as well.
Maybe you can do your extensive tests after I sent v3 of this series?
Also thanks for offering to test this, I don't have any Realtek PCIe
wifi, so I am unable to verify I broke anything myself.


Best regards,
Martin