Re: [PATCH] wifi: mac80211: Serialize calls to drv_wake_tx_queue()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.03.23 12:20, Alexander Wetzel wrote:
assuming that TXQ drivers actually still call
ieee80211_txq_schedule_end() which says it's deprecated.

That even has _bh() so the tasklet can't be running anyway ...

So if the concurrency really is only TX vs. tasklet, then you could even
just keep the BHs disabled (in _start spin_unlock only and then in _end
local_bh_disable)?

Which may also be the solution for the regression in 6.2:
Do it now for ieee80211_handle_wake_tx_queue() and apply this patch
to the development tree only.

I'd argue the other way around - do it for all to fix these issues, and
then audit drivers such as iwlwifi or even make concurrency here opt-in.

Felix did see some benefits of the concurrency I think though, so he
might have a different opinion.

What about handling it that way:

Keep serializing drv_wake_tx_queue(). But use a new spinlock the drivers
can opt out from.
   1) No flag set:
      drv_wake_tx_queue() can be running only once at any time

   2) IEEE80211_HW_CONCURRENT_AC_TX
      drv_wake_tx_queue() can be running once per AC at any time

   3) IEEE80211_HW_CONCURRENT
      current behavior.

I think if you really insist on handling this through drv_wake_tx_queue locking, it really shouldn't be done through extra hw flags, because the locking requirements are too different.

I took a quick look at all the drivers that implement iTXQ themselves and what their requirements are.

Only two drivers need changes:
- ath10k: needs a per-AC lock, because it does a scheduling round in the wake_tx_queue call. - iwlwifi: needs a global lock, since it touches a common list. None of your proposed locking types are enough for this one.

The rest seem fine to me:
- ath9k has a per-hw-queue lock
- mt76 only schedules a kthread
- rtw88 uses a global lock + workqueue for scheduling
- rtw89 uses a (potentially unnecessary) ieee80211_schedule_txq call + workqueue for scheduling

If you want to address this in the least invasive way, add a per-AC lock to ath10k's wake_tx_queue handler, a global lock to iwlwifi, and a per-AC lock inside ieee80211_handle_wake_tx_queue().

That way no locking is needed around the drv_wake_tx_queue calls.

- Felix



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux