This series follows up on some notes from this thread: http://lkml.kernel.org/linux-wireless/20181130175957.167031-1-briannorris@xxxxxxxxxxxx Subject: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage" where Ganapathi suggested I send out my work. In particular, patch 1 is a step toward helping apply Ganapathi's original "mwifiex: restructure rx_reorder_tbl_lock usage" solution without regression, by logically separating the two operations (and therefore, the locking patterns) involved in that deadlock. It doesn't re-apply that change, nor does it 100% unblock such a solution, but at least it's a step in the right direction, as I understand it. Patch 2 is a change I noticed should be possible along the way. There are a number of reasons we probably shouldn't be disabling hardirqs when it's not necessary, but one funny side effect: bugs noticed in the above "revert" patch would no longer happen. This is because mwifiex_recv_packet() bases softirq decisions on in_interrupt() (see description in include/linux/preempt.h), so it will automatically skip softirq processing if we have BH disabled, but not if we only have hard IRQs disabled. In other words, if we have such an incorrect nesting bug in the future (this time with BH disabled), we will now skip softirq processing and therefore sidestep this sort of bug. [1] [Related note: softirq masking is weird: https://lwn.net/Articles/779738/] It's also possible we can improve system responsiveness and debuggability by keeping (hard) IRQs enabled more often, although I didn't measure any particular effect here, and most of these contexts should be rather quick. I've done a variety of performance and stress tests for this series, on both 8897/SDIO and 8997/PCIe, and I haven't seen any decrease in performance or stability. Or, any change in performance appears to be within the range of "noise". Regards, Brian [1] Side note: the usage of 'in_interrupt()' in mwifiex_recv_packet() is probably not really a good idea. But it does have a helpful side effect for this particular sort of bug. Changelog: v2: * fix warnings about using uninitialized "flags" variables Brian Norris (2): mwifiex: dispatch/rotate from reorder table atomically mwifiex: don't disable hardirqs; just softirqs drivers/net/wireless/marvell/mwifiex/11n.c | 53 +++----- drivers/net/wireless/marvell/mwifiex/11n.h | 5 +- .../net/wireless/marvell/mwifiex/11n_aggr.c | 26 ++-- .../net/wireless/marvell/mwifiex/11n_aggr.h | 2 +- .../wireless/marvell/mwifiex/11n_rxreorder.c | 125 ++++++++---------- .../net/wireless/marvell/mwifiex/cfg80211.c | 35 +++-- drivers/net/wireless/marvell/mwifiex/cmdevt.c | 76 +++++------ drivers/net/wireless/marvell/mwifiex/init.c | 32 ++--- drivers/net/wireless/marvell/mwifiex/main.c | 29 ++-- drivers/net/wireless/marvell/mwifiex/scan.c | 58 ++++---- .../wireless/marvell/mwifiex/sta_cmdresp.c | 5 +- .../net/wireless/marvell/mwifiex/sta_event.c | 10 +- drivers/net/wireless/marvell/mwifiex/tdls.c | 68 ++++------ drivers/net/wireless/marvell/mwifiex/txrx.c | 5 +- .../net/wireless/marvell/mwifiex/uap_txrx.c | 10 +- drivers/net/wireless/marvell/mwifiex/usb.c | 10 +- drivers/net/wireless/marvell/mwifiex/util.c | 15 +-- drivers/net/wireless/marvell/mwifiex/wmm.c | 109 ++++++--------- 18 files changed, 278 insertions(+), 395 deletions(-) -- 2.22.0.410.gd8fdbe21b5-goog
