diff mbox

[1/1] mac80211: ieee80211_rx_napi: remove warning

Message ID 1496581915-4412-2-git-send-email-erik.stromdahl@gmail.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Erik Stromdahl June 4, 2017, 1:11 p.m. UTC
The softirq count is not always incremented during driver
operation. This is the case for usb and sdio network
drivers.

The below warning occurs on the first RX frame pushed to
mac80211 (for usb and sdio):

[   27.414995] ------------[ cut here ]------------
[   27.416444] WARNING: CPU: 0 PID: 16 at net/mac80211/rx.c:4254 ieee80211_rx_napi+0x598/0xa30
[   27.419161] Modules linked in: i8042 serio ehci_pci ehci_hcd ath10k_usb ath10k_core ath
[   27.421660] CPU: 0 PID: 16 Comm: kworker/0:1 Not tainted 4.12.0-rc2-wt-ath-ARCH-QEMU+ #5
[   27.424323] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
[   27.425205] Workqueue: events ath10k_usb_io_comp_work [ath10k_usb]
[   27.425760] task: ffff88003a730d00 task.stack: ffffc90000210000
[   27.426275] RIP: 0010:ieee80211_rx_napi+0x598/0xa30
[   27.426700] RSP: 0018:ffffc90000213c68 EFLAGS: 00010346
[   27.427155] RAX: 0000000080000000 RBX: ffff880039d08500 RCX: 0000000000000000
[   27.427764] RDX: ffff880039d08500 RSI: 0000000000000000 RDI: ffff880039ff0780
[   27.428371] RBP: ffffc90000213d38 R08: 0000000000000000 R09: 00000000fffffff0
[   27.429015] R10: ffffea0000e7e200 R11: 0000000000000080 R12: 0000000000000000
[   27.429633] R13: ffff880039ff1560 R14: 0000000000000080 R15: ffff880039ff0780
[   27.430240] FS:  0000000000000000(0000) GS:ffff88003be00000(0000) knlGS:0000000000000000
[   27.430914] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   27.431392] CR2: 00000000007fe518 CR3: 000000003f8c0000 CR4: 00000000000006f0
[   27.431982] Call Trace:
[   27.432195]  ? ieee80211_rx_napi+0x18/0xa30
[   27.432553]  ath10k_wmi_event_mgmt_rx+0x233/0x430 [ath10k_core]
[   27.433046]  ath10k_wmi_tlv_op_rx+0x2fb/0x7b0 [ath10k_core]
[   27.433567]  ath10k_wmi_process_rx+0x1a/0x40 [ath10k_core]
[   27.434039]  ath10k_usb_io_comp_work+0x13e/0x1a0 [ath10k_usb]
[   27.434527]  ? __schedule+0x2e3/0x840
[   27.434858]  process_one_work+0x1e0/0x420
[   27.435203]  worker_thread+0x48/0x3f0
[   27.435514]  kthread+0x109/0x140
[   27.435846]  ? process_one_work+0x420/0x420
[   27.436231]  ? kthread_create_on_node+0x70/0x70
[   27.436644]  ret_from_fork+0x2c/0x40
[   27.437046] Code: 70 4c 8b ab d8 00 00 00 44 8b 83 80 00 00 00 41 0f b7 55 00 4d 89 ee 41 89 d4 41 83 e4 0c e9 88 fc ff ff 4d 89 ec e9 c1 fd ff ff <0f> ff 0f b6 43 4b 3c 02 0f 86 b2 fa ff ff 0f ff e9 30 fb ff ff
[   27.439114] ---[ end trace 89f286e9814e824a ]---

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 net/mac80211/rx.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Johannes Berg June 7, 2017, 9:57 p.m. UTC | #1
On Sun, 2017-06-04 at 15:11 +0200, Erik Stromdahl wrote:
> The softirq count is not always incremented during driver
> operation. This is the case for usb and sdio network
> drivers.

I'm pretty sure the warning is correct, and we do rely on having
local_bh_disable(), otherwise we may end up taking a soft-IRQ and I
believe there are some things that could get messed up in that case.

So - I think the warning is there for a reason, and drivers should just
local_bh_disable() before calling into that. What's wrong with that?

johannes
Erik Stromdahl June 8, 2017, 5:10 p.m. UTC | #2
On 2017-06-07 23:57, Johannes Berg wrote:
> On Sun, 2017-06-04 at 15:11 +0200, Erik Stromdahl wrote:
>> The softirq count is not always incremented during driver
>> operation. This is the case for usb and sdio network
>> drivers.
> 
> I'm pretty sure the warning is correct, and we do rely on having
> local_bh_disable(), otherwise we may end up taking a soft-IRQ and I
> believe there are some things that could get messed up in that case.
> 
Ok, I will make sure to increment the softirq counter before calling
ieee80211_rx then.

> So - I think the warning is there for a reason, and drivers should just
> local_bh_disable() before calling into that. What's wrong with that?
I guess there is nothing wrong with that, it's just that ath10k does not
call local_bh_disable anywhere in the code.
I guess it is relying on lower layers (pcie?) to do that.
When introducing sdio and usb support these calls will have to be added
explicitly in ath10k.

> 
> johannes
>
diff mbox

Patch

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1f75280ba26c..2ec54232817d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -4251,8 +4251,6 @@  void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 
-	WARN_ON_ONCE(softirq_count() == 0);
-
 	if (WARN_ON(status->band >= NUM_NL80211_BANDS))
 		goto drop;