iwl_pcie_gen2_enqueue_hcmd needs same fix as iwl_pcie_enqueue_hcmd

iwl_pcie_gen2_enqueue_hcmd needs same fix as iwl_pcie_enqueue_hcmd

[Heiner Kallweit]

Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
I get the same lockdep warning on AX210.

[PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Jiri Kosina]

On Wed, 7 Apr 2021, Heiner Kallweit wrote:

> Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
> iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
> I get the same lockdep warning on AX210.

Makes sense, it's being called from exactly the same contexts.



From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

Analogically to what we did in 2800aadc18a6 ("iwlwifi: Fix 
softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()"), we must apply the 
same fix to iwl_pcie_gen2_enqueue_hcmd(), as it's being called from 
exactly the same contexts.

Reported-by: Heiner Kallweit <hkallweit1@gmail.com
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
--- 

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
index 4456abb9a074..34bde8c87324 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
@@ -40,6 +40,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
 	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
 	struct iwl_tfh_tfd *tfd;
+	unsigned long flags;
 
 	copy_size = sizeof(struct iwl_cmd_header_wide);
 	cmd_size = sizeof(struct iwl_cmd_header_wide);
@@ -108,14 +109,14 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 		goto free_dup_buf;
 	}
 
-	spin_lock_bh(&txq->lock);
+	spin_lock_irqsave(&txq->lock, flags);
 
 	idx = iwl_txq_get_cmd_index(txq, txq->write_ptr);
 	tfd = iwl_txq_get_tfd(trans, txq, txq->write_ptr);
 	memset(tfd, 0, sizeof(*tfd));
 
 	if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
-		spin_unlock_bh(&txq->lock);
+		spin_unlock_irqrestore(&txq->lock, flags);
 
 		IWL_ERR(trans, "No space in command queue\n");
 		iwl_op_mode_cmd_queue_full(trans->op_mode);
@@ -250,7 +251,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	spin_unlock(&trans_pcie->reg_lock);
 
 out:
-	spin_unlock_bh(&txq->lock);
+	spin_unlock_irqrestore(&txq->lock, flags);
 free_dup_buf:
 	if (idx < 0)
 		kfree(dup_buf);

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Johannes Berg]

On Wed, 2021-04-07 at 09:51 +0200, Jiri Kosina wrote:
> On Wed, 7 Apr 2021, Heiner Kallweit wrote:
> 
> > Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
> > iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
> > I get the same lockdep warning on AX210.
> 
> Makes sense, it's being called from exactly the same contexts.

I'm guessing nobody saw this before because the LEDs stuff is not
supported/used on newer devices :)

Still makes sense though.

Btw, I had a fix for your patch for some devices, see below. Already in
our tree, so Luca will send it on the way, just FYI/review here.

Thanks,
johannes


From: Johannes Berg <johannes.berg@intel.com>
Date: Tue, 6 Apr 2021 16:50:23 +0200
Subject: [PATCH] [BUGFIX] iwlwifi: pcie: don't enable BHs with IRQs disabled

After the fix from Jiri that disabled local IRQs instead of
just BHs (necessary to fix an issue with submitting a command
with IRQs already disabled), there was still a situation in
which we could deep in there enable BHs, if the device config
sets the apmg_wake_up_wa configuration, which is true on all
7000 series devices.

To fix that, but not require reverting commit 1ed08f6fb5ae
("iwlwifi: remove flags argument for nic_access"), split up
nic access into a version with BH manipulation to use most
of the time, and without it for this specific case where the
local IRQs are already disabled.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 .../wireless/intel/iwlwifi/pcie/internal.h    |  5 ++++
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 24 ++++++++++++++++---
 drivers/net/wireless/intel/iwlwifi/pcie/tx.c  |  4 ++--
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
index 2c2389feb5e1..0b99f0c34111 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
@@ -453,6 +453,11 @@ struct iwl_trans
 		      const struct iwl_cfg_trans_params *cfg_trans);
 void iwl_trans_pcie_free(struct iwl_trans *trans);
 
+bool __iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans);
+#define _iwl_trans_pcie_grab_nic_access(trans)			\
+	__cond_lock(nic_access_nobh,				\
+		    likely(__iwl_trans_pcie_grab_nic_access(trans)))
+
 /*****************************************************
 * RX
 ******************************************************/
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index f2869fc343e3..bf36fa72f22e 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -2047,12 +2047,16 @@ static void iwl_trans_pcie_removal_wk(struct work_struct *wk)
 	module_put(THIS_MODULE);
 }
 
-static bool iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans)
+/*
+ * This version doesn't disable BHs but rather assumes they're
+ * already disabled.
+ */
+bool __iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans)
 {
 	int ret;
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 
-	spin_lock_bh(&trans_pcie->reg_lock);
+	spin_lock(&trans_pcie->reg_lock);
 
 	if (trans_pcie->cmd_hold_nic_awake)
 		goto out;
@@ -2137,7 +2141,7 @@ static bool iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans)
 		}
 
 err:
-		spin_unlock_bh(&trans_pcie->reg_lock);
+		spin_unlock(&trans_pcie->reg_lock);
 		return false;
 	}
 
@@ -2150,6 +2154,20 @@ out:
 	return true;
 }
 
+static bool iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans)
+{
+	bool ret;
+
+	local_bh_disable();
+	ret = __iwl_trans_pcie_grab_nic_access(trans);
+	if (ret) {
+		/* keep BHs disabled until iwl_trans_pcie_release_nic_access */
+		return ret;
+	}
+	local_bh_enable();
+	return false;
+}
+
 static void iwl_trans_pcie_release_nic_access(struct iwl_trans *trans)
 {
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
index 0346505351f5..4f6c187eed69 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
@@ -643,7 +643,7 @@ static int iwl_pcie_set_cmd_in_flight(struct iwl_trans *trans,
 	 * returned. This needs to be done only on NICs that have
 	 * apmg_wake_up_wa set (see above.)
 	 */
-	if (!iwl_trans_grab_nic_access(trans))
+	if (!_iwl_trans_pcie_grab_nic_access(trans))
 		return -EIO;
 
 	/*
@@ -652,7 +652,7 @@ static int iwl_pcie_set_cmd_in_flight(struct iwl_trans *trans,
 	 * already true, so it's OK to unconditionally set it to true.
 	 */
 	trans_pcie->cmd_hold_nic_awake = true;
-	spin_unlock_bh(&trans_pcie->reg_lock);
+	spin_unlock(&trans_pcie->reg_lock);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Johannes Berg]

On Wed, 2021-04-07 at 09:55 +0200, Johannes Berg wrote:
> On Wed, 2021-04-07 at 09:51 +0200, Jiri Kosina wrote:
> > On Wed, 7 Apr 2021, Heiner Kallweit wrote:
> > 
> > > Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
> > > iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
> > > I get the same lockdep warning on AX210.
> > 
> > Makes sense, it's being called from exactly the same contexts.
> 
> I'm guessing nobody saw this before because the LEDs stuff is not
> supported/used on newer devices :)

Eh, wait, Heiner said he sees the same warning? With LEDs? I was pretty
sure that's not supported by the later firmware, but maybe some other
path? Anyway this fix seems right.

johannes

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Heiner Kallweit]

On 07.04.2021 09:56, Johannes Berg wrote:
> On Wed, 2021-04-07 at 09:55 +0200, Johannes Berg wrote:
>> On Wed, 2021-04-07 at 09:51 +0200, Jiri Kosina wrote:
>>> On Wed, 7 Apr 2021, Heiner Kallweit wrote:
>>>
>>>> Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
>>>> iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
>>>> I get the same lockdep warning on AX210.
>>>
>>> Makes sense, it's being called from exactly the same contexts.
>>
>> I'm guessing nobody saw this before because the LEDs stuff is not
>> supported/used on newer devices :)
> 
> Eh, wait, Heiner said he sees the same warning? With LEDs? I was pretty
> sure that's not supported by the later firmware, but maybe some other
> path? Anyway this fix seems right.
> 
> johannes
> 

For your reference here the warning I got.

[   27.645349] ------------[ cut here ]------------
[   27.645453] WARNING: CPU: 1 PID: 2323 at kernel/softirq.c:364 __local_bh_enable_ip+0x85/0xc0
[   27.645516] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio cmac bnep iwlmvm led_class vfat fat mac80211 libarc4 iwlwifi x86_pkg_temp_thermal coretemp i915 btusb btintel bluetooth snd_hda_intel snd_intel_dspcfg aesni_intel r8169 cfg80211 realtek ecdh_generic snd_hda_codec mdio_devres crypto_simd i2c_i801 rfkill intel_gtt ecc mei_me cryptd i2c_smbus snd_hda_core libphy mei i2c_algo_bit snd_pcm drm_kms_helper syscopyarea sysfillrect sysimgblt snd_timer fb_sys_fops snd sch_fq_codel crypto_user drm efivarfs ext4 mbcache jbd2 ums_realtek crc32c_intel ahci libahci libata
[   27.645991] CPU: 1 PID: 2323 Comm: ip Not tainted 5.12.0-rc6-next-20210406+ #4
[   27.646040] Hardware name: NA ZBOX-CI327NANO-GS-01/ZBOX-CI327NANO-GS-01, BIOS 5.12 04/28/2020
[   27.646094] RIP: 0010:__local_bh_enable_ip+0x85/0xc0
[   27.646132] Code: 8b 05 af c6 ba 47 a9 00 ff ff 00 74 32 65 ff 0d a1 c6 ba 47 e8 7c b4 0e 00 fb 5b 41 5c 5d c3 65 8b 05 6b cc ba 47 85 c0 75 ae <0f> 0b eb aa e8 92 b3 0e 00 eb ab 4c 89 e7 e8 18 ff 05 00 eb b4 65
[   27.646275] RSP: 0018:ffffbb574062b1b8 EFLAGS: 00010046
[   27.646311] RAX: 0000000000000000 RBX: 0000000000000201 RCX: 0000000000000000
[   27.646355] RDX: 0000000000000003 RSI: 0000000000000201 RDI: ffffffffc051192b
[   27.646401] RBP: ffffbb574062b1c8 R08: 0000000000000000 R09: 0000000000000001
[   27.646444] R10: ffff9eec03243e00 R11: 0000000000000000 R12: ffffffffc051192b
[   27.646488] R13: ffff9eec07e72b40 R14: 0000000000000001 R15: 0000000000000000
[   27.646533] FS:  00007fce73f11740(0000) GS:ffff9eec7bc80000(0000) knlGS:0000000000000000
[   27.646584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   27.646622] CR2: 00005581ebf10cc0 CR3: 0000000102924000 CR4: 00000000003506e0
[   27.646668] Call Trace:
[   27.646687]  _raw_spin_unlock_bh+0x2c/0x40
[   27.646718]  iwl_pcie_gen2_enqueue_hcmd+0x50b/0x890 [iwlwifi]
[   27.646781]  iwl_trans_txq_send_hcmd+0x68/0x3b0 [iwlwifi]
[   27.646836]  iwl_trans_send_cmd+0x7d/0x170 [iwlwifi]
[   27.646888]  iwl_mvm_send_cmd+0x29/0x80 [iwlmvm]
[   27.646942]  iwl_mvm_led_set+0xa5/0xd0 [iwlmvm]
[   27.646995]  iwl_led_brightness_set+0x1a/0x20 [iwlmvm]
[   27.647050]  led_set_brightness_nosleep+0x24/0x50
[   27.647085]  led_set_brightness+0x41/0x50
[   27.647115]  led_trigger_event+0x46/0x70
[   27.647143]  ieee80211_led_radio+0x24/0x30 [mac80211]
[   27.647288]  ieee80211_do_open+0x4c4/0x9a0 [mac80211]
[   27.647379]  ieee80211_open+0x69/0x90 [mac80211]
[   27.647487]  __dev_open+0xd6/0x190
[   27.647517]  __dev_change_flags+0x1b2/0x230
[   27.647548]  dev_change_flags+0x21/0x60
[   27.647577]  do_setlink+0x289/0x1190
[   27.647607]  ? __nla_validate_parse+0x60/0xbd0
[   27.647644]  __rtnl_newlink+0x61c/0xa10
[   27.647677]  ? mark_lock+0x25/0x1110
[   27.647709]  ? rcu_read_lock_sched_held+0x41/0x80
[   27.647744]  ? kmem_cache_alloc_trace+0x21e/0x250
[   27.647781]  rtnl_newlink+0x44/0x70
[   27.647808]  rtnetlink_rcv_msg+0x17f/0x4b0
[   27.647838]  ? netlink_deliver_tap+0x10e/0x3f0
[   27.647873]  ? find_held_lock+0x31/0x90
[   27.647902]  ? netlink_deliver_tap+0x10e/0x3f0
[   27.647934]  ? rtnetlink_put_metrics+0x1d0/0x1d0
[   27.647967]  netlink_rcv_skb+0x50/0xf0
[   27.648000]  rtnetlink_rcv+0x10/0x20
[   27.648027]  netlink_unicast+0x1a3/0x270
[   27.648056]  netlink_sendmsg+0x22c/0x430
[   27.648085]  ? netlink_unicast+0x270/0x270
[   27.648116]  ____sys_sendmsg+0x2a6/0x2e0
[   27.648147]  ? import_iovec+0x1b/0x20
[   27.648179]  ? sendmsg_copy_msghdr+0x79/0xa0
[   27.648211]  ___sys_sendmsg+0x82/0xc0
[   27.648240]  __sys_sendmsg+0x62/0xb0
[   27.648268]  ? syscall_enter_from_user_mode+0x21/0x60
[   27.648307]  __x64_sys_sendmsg+0x1a/0x20
[   27.648337]  do_syscall_64+0x38/0x50
[   27.648366]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   27.648403] RIP: 0033:0x7fce74030737
[   27.648433] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
[   27.648547] RSP: 002b:00007ffdd61d1498 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[   27.648600] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fce74030737
[   27.648645] RDX: 0000000000000000 RSI: 00007ffdd61d1500 RDI: 0000000000000003
[   27.648691] RBP: 00000000606d6d4f R08: 0000000000000001 R09: 00007fce740f1a60
[   27.648736] R10: 0000000000000230 R11: 0000000000000246 R12: 0000000000000001
[   27.648782] R13: 00007ffdd61d15d0 R14: 0000000000000000 R15: 00005581ebec6020
[   27.648830] irq event stamp: 94495
[   27.648855] hardirqs last  enabled at (94493): [<ffffffffb846a864>] __local_bh_enable_ip+0x74/0xc0
[   27.648917] hardirqs last disabled at (94494): [<ffffffffb8bf198f>] _raw_read_lock_irqsave+0x7f/0x90
[   27.648977] softirqs last  enabled at (94492): [<ffffffffc051192b>] iwl_pcie_gen2_enqueue_hcmd+0x50b/0x890 [iwlwifi]
[   27.649064] softirqs last disabled at (94495): [<ffffffffc0511516>] iwl_pcie_gen2_enqueue_hcmd+0xf6/0x890 [iwlwifi]
[   27.649151] ---[ end trace da6e11a6f99dba37 ]---
[   27.649242] ------------[ cut here ]------------
[   27.649326] raw_local_irq_restore() called with IRQs enabled
[   27.649367] WARNING: CPU: 1 PID: 2323 at kernel/locking/irqflag-debug.c:10 warn_bogus_irq_restore+0x22/0x30
[   27.649433] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio cmac bnep iwlmvm led_class vfat fat mac80211 libarc4 iwlwifi x86_pkg_temp_thermal coretemp i915 btusb btintel bluetooth snd_hda_intel snd_intel_dspcfg aesni_intel r8169 cfg80211 realtek ecdh_generic snd_hda_codec mdio_devres crypto_simd i2c_i801 rfkill intel_gtt ecc mei_me cryptd i2c_smbus snd_hda_core libphy mei i2c_algo_bit snd_pcm drm_kms_helper syscopyarea sysfillrect sysimgblt snd_timer fb_sys_fops snd sch_fq_codel crypto_user drm efivarfs ext4 mbcache jbd2 ums_realtek crc32c_intel ahci libahci libata
[   27.649900] CPU: 1 PID: 2323 Comm: ip Tainted: G        W         5.12.0-rc6-next-20210406+ #4
[   27.649956] Hardware name: NA ZBOX-CI327NANO-GS-01/ZBOX-CI327NANO-GS-01, BIOS 5.12 04/28/2020
[   27.650010] RIP: 0010:warn_bogus_irq_restore+0x22/0x30
[   27.650047] Code: cc cc cc cc cc cc cc cc 55 48 89 e5 80 3d ca 51 9f 00 00 74 02 5d c3 48 c7 c7 18 07 21 b9 c6 05 b8 51 9f 00 01 e8 12 f8 fc ff <0f> 0b 5d c3 cc cc cc cc cc cc cc cc cc cc 55 48 89 e5 41 54 53 44
[   27.650177] RSP: 0018:ffffbb574062b3d0 EFLAGS: 00010286
[   27.650216] RAX: 0000000000000000 RBX: 0000000000000200 RCX: ffff9eec7bc976f8
[   27.650303] RDX: 00000000ffffffd8 RSI: 0000000000000027 RDI: ffff9eec7bc976f0
[   27.650423] RBP: ffffbb574062b3d0 R08: 0000000000000001 R09: 0000000000000001
[   27.650542] R10: 00000000ffffe000 R11: 3fffffffffffffff R12: ffff9eec08e9ac78
[   27.650661] R13: 00000000000000ff R14: 0000000000000286 R15: ffff9eec08e9ac78
[   27.650781] FS:  00007fce73f11740(0000) GS:ffff9eec7bc80000(0000) knlGS:0000000000000000
[   27.650919] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   27.651070] CR2: 00005581ebf10cc0 CR3: 0000000102924000 CR4: 00000000003506e0
[   27.651243] Call Trace:
[   27.651298]  _raw_read_unlock_irqrestore+0x4f/0x60
[   27.651392]  led_trigger_event+0x64/0x70
[   27.651470]  ieee80211_led_radio+0x24/0x30 [mac80211]
[   27.651758]  ieee80211_do_open+0x4c4/0x9a0 [mac80211]
[   27.652006]  ieee80211_open+0x69/0x90 [mac80211]
[   27.652334]  __dev_open+0xd6/0x190
[   27.652407]  __dev_change_flags+0x1b2/0x230
[   27.652490]  dev_change_flags+0x21/0x60
[   27.652566]  do_setlink+0x289/0x1190
[   27.652641]  ? __nla_validate_parse+0x60/0xbd0
[   27.652729]  __rtnl_newlink+0x61c/0xa10
[   27.652809]  ? mark_lock+0x25/0x1110
[   27.652886]  ? rcu_read_lock_sched_held+0x41/0x80
[   27.652977]  ? kmem_cache_alloc_trace+0x21e/0x250
[   27.653068]  rtnl_newlink+0x44/0x70
[   27.653139]  rtnetlink_rcv_msg+0x17f/0x4b0
[   27.653257]  ? netlink_deliver_tap+0x10e/0x3f0
[   27.653343]  ? find_held_lock+0x31/0x90
[   27.653419]  ? netlink_deliver_tap+0x10e/0x3f0
[   27.653504]  ? rtnetlink_put_metrics+0x1d0/0x1d0
[   27.653591]  netlink_rcv_skb+0x50/0xf0
[   27.653667]  rtnetlink_rcv+0x10/0x20
[   27.653738]  netlink_unicast+0x1a3/0x270
[   27.653816]  netlink_sendmsg+0x22c/0x430
[   27.653893]  ? netlink_unicast+0x270/0x270
[   27.653973]  ____sys_sendmsg+0x2a6/0x2e0
[   27.654052]  ? import_iovec+0x1b/0x20
[   27.654127]  ? sendmsg_copy_msghdr+0x79/0xa0
[   27.654250]  ___sys_sendmsg+0x82/0xc0
[   27.654332]  __sys_sendmsg+0x62/0xb0
[   27.654407]  ? syscall_enter_from_user_mode+0x21/0x60
[   27.654502]  __x64_sys_sendmsg+0x1a/0x20
[   27.654581]  do_syscall_64+0x38/0x50
[   27.654654]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   27.654748] RIP: 0033:0x7fce74030737
[   27.654819] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
[   27.655111] RSP: 002b:00007ffdd61d1498 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[   27.655284] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fce74030737
[   27.655405] RDX: 0000000000000000 RSI: 00007ffdd61d1500 RDI: 0000000000000003
[   27.655523] RBP: 00000000606d6d4f R08: 0000000000000001 R09: 00007fce740f1a60
[   27.655641] R10: 0000000000000230 R11: 0000000000000246 R12: 0000000000000001
[   27.655759] R13: 00007ffdd61d15d0 R14: 0000000000000000 R15: 00005581ebec6020
[   27.655879] irq event stamp: 95385
[   27.655944] hardirqs last  enabled at (95393): [<ffffffffb84d2270>] console_unlock+0x4a0/0x5c0
[   27.656093] hardirqs last disabled at (95400): [<ffffffffb84d21e3>] console_unlock+0x413/0x5c0
[   27.656275] softirqs last  enabled at (95022): [<ffffffffb846a9ec>] irq_exit_rcu+0xac/0xe0
[   27.656420] softirqs last disabled at (95017): [<ffffffffb846a9ec>] irq_exit_rcu+0xac/0xe0
[   27.656561] ---[ end trace da6e11a6f99dba38 ]---

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Kalle Valo]

Jiri Kosina <jikos@kernel.org> wrote:

> On Wed, 7 Apr 2021, Heiner Kallweit wrote:
> 
> > Same fix as in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq disabling in
> > iwl_pcie_enqueue_hcmd()") is needed for iwl_pcie_gen2_enqueue_hcmd.
> > I get the same lockdep warning on AX210.
> 
> Makes sense, it's being called from exactly the same contexts.
> 
> 
> 
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()
> 
> Analogically to what we did in 2800aadc18a6 ("iwlwifi: Fix 
> softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()"), we must apply the 
> same fix to iwl_pcie_gen2_enqueue_hcmd(), as it's being called from 
> exactly the same contexts.
> 
> Reported-by: Heiner Kallweit <hkallweit1@gmail.com
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> index 4456abb9a074..34bde8c87324 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> @@ -40,6 +40,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
>  	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
>  	struct iwl_tfh_tfd *tfd;
> +	unsigned long flags;
>  
>  	copy_size = sizeof(struct iwl_cmd_header_wide);
>  	cmd_size = sizeof(struct iwl_cmd_header_wide);
> @@ -108,14 +109,14 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  		goto free_dup_buf;
>  	}
>  
> -	spin_lock_bh(&txq->lock);
> +	spin_lock_irqsave(&txq->lock, flags);
>  
>  	idx = iwl_txq_get_cmd_index(txq, txq->write_ptr);
>  	tfd = iwl_txq_get_tfd(trans, txq, txq->write_ptr);
>  	memset(tfd, 0, sizeof(*tfd));
>  
>  	if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
> -		spin_unlock_bh(&txq->lock);
> +		spin_unlock_irqrestore(&txq->lock, flags);
>  
>  		IWL_ERR(trans, "No space in command queue\n");
>  		iwl_op_mode_cmd_queue_full(trans->op_mode);
> @@ -250,7 +251,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  	spin_unlock(&trans_pcie->reg_lock);
>  
>  out:
> -	spin_unlock_bh(&txq->lock);
> +	spin_unlock_irqrestore(&txq->lock, flags);
>  free_dup_buf:
>  	if (idx < 0)
>  		kfree(dup_buf);

This is malformed in patchwork, check the link below. Please resend, and
I strongly recommend to use git send-email to avoid any format issues.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Jiri Kosina]

On Sat, 17 Apr 2021, Kalle Valo wrote:

> This is malformed in patchwork, check the link below. Please resend, and
> I strongly recommend to use git send-email to avoid any format issues.

Honestly I have no idea what you are talking about, there is no whitespace 
damage nor anything else that I'd see to be broken. I just took the patch 
from the mail I sent, applied with git-am, and it worked flawlessly.

Anyway, I'll send a patch as a followup to this mail so that it could 
hopefully be picked up by your tooling.

Thanks,

-- 
Jiri Kosina
SUSE Labs

[PATCH v2] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Analogically to what we did in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq 
disabling in iwl_pcie_enqueue_hcmd()"), we must apply the same fix to 
iwl_pcie_gen2_enqueue_hcmd(), as it's being called from exactly the same 
contexts.

Reported-by: Heiner Kallweit <hkallweit1@gmail.com
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
--- 

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
index 4456abb9a074..34bde8c87324 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
@@ -40,6 +40,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
 	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
 	struct iwl_tfh_tfd *tfd;
+	unsigned long flags;
 
 	copy_size = sizeof(struct iwl_cmd_header_wide);
 	cmd_size = sizeof(struct iwl_cmd_header_wide);
@@ -108,14 +109,14 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 		goto free_dup_buf;
 	}
 
-	spin_lock_bh(&txq->lock);
+	spin_lock_irqsave(&txq->lock, flags);
 
 	idx = iwl_txq_get_cmd_index(txq, txq->write_ptr);
 	tfd = iwl_txq_get_tfd(trans, txq, txq->write_ptr);
 	memset(tfd, 0, sizeof(*tfd));
 
 	if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
-		spin_unlock_bh(&txq->lock);
+		spin_unlock_irqrestore(&txq->lock, flags);
 
 		IWL_ERR(trans, "No space in command queue\n");
 		iwl_op_mode_cmd_queue_full(trans->op_mode);
@@ -250,7 +251,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	spin_unlock(&trans_pcie->reg_lock);
 
 out:
-	spin_unlock_bh(&txq->lock);
+	spin_unlock_irqrestore(&txq->lock, flags);
 free_dup_buf:
 	if (idx < 0)
 		kfree(dup_buf);

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Jiri Kosina]

On Sat, 17 Apr 2021, Jiri Kosina wrote:

> > This is malformed in patchwork, check the link below. Please resend, and
> > I strongly recommend to use git send-email to avoid any format issues.
> 
> Honestly I have no idea what you are talking about, there is no whitespace 
> damage nor anything else that I'd see to be broken. I just took the patch 
> from the mail I sent, applied with git-am, and it worked flawlessly.
> 
> Anyway, I'll send a patch as a followup to this mail so that it could 
> hopefully be picked up by your tooling.

And it seems to have appeared here:

https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Sedat Dilek]

On Sat, Apr 17, 2021 at 11:25 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Sat, 17 Apr 2021, Jiri Kosina wrote:
>
> > > This is malformed in patchwork, check the link below. Please resend, and
> > > I strongly recommend to use git send-email to avoid any format issues.
> >
> > Honestly I have no idea what you are talking about, there is no whitespace
> > damage nor anything else that I'd see to be broken. I just took the patch
> > from the mail I sent, applied with git-am, and it worked flawlessly.
> >
> > Anyway, I'll send a patch as a followup to this mail so that it could
> > hopefully be picked up by your tooling.
>
> And it seems to have appeared here:
>
> https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/
>
> --
> Jiri Kosina
> SUSE Labs
>

Thanks for the patch Jiri.

This is on top of Linux v5.12-rc7+...

link="https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm"

$ b4 -d am $link

$ LC_ALL=C git apply --check --verbose
../v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
Checking patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c...

$ LC_ALL=C git apply --verbose
../v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
Checking patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c...
Applied patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c cleanly.

- Sedat -

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Kalle Valo]

Jiri Kosina <jikos@kernel.org> writes:

> On Sat, 17 Apr 2021, Kalle Valo wrote:
>
>> This is malformed in patchwork, check the link below. Please resend, and
>> I strongly recommend to use git send-email to avoid any format issues.
>
> Honestly I have no idea what you are talking about, there is no whitespace 
> damage nor anything else that I'd see to be broken. I just took the patch 
> from the mail I sent, applied with git-am, and it worked flawlessly.

Compare these two links:

https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/

https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/

In v1 there's email discussion in the commit log which shouldn't be
there.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Sedat Dilek]

On Sat, Apr 17, 2021 at 2:06 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sat, Apr 17, 2021 at 11:25 AM Jiri Kosina <jikos@kernel.org> wrote:
> >
> > On Sat, 17 Apr 2021, Jiri Kosina wrote:
> >
> > > > This is malformed in patchwork, check the link below. Please resend, and
> > > > I strongly recommend to use git send-email to avoid any format issues.
> > >
> > > Honestly I have no idea what you are talking about, there is no whitespace
> > > damage nor anything else that I'd see to be broken. I just took the patch
> > > from the mail I sent, applied with git-am, and it worked flawlessly.
> > >
> > > Anyway, I'll send a patch as a followup to this mail so that it could
> > > hopefully be picked up by your tooling.
> >
> > And it seems to have appeared here:
> >
> > https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/
> >
> > --
> > Jiri Kosina
> > SUSE Labs
> >
>
> Thanks for the patch Jiri.
>
> This is on top of Linux v5.12-rc7+...
>
> link="https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm"
>
> $ b4 -d am $link
>
> $ LC_ALL=C git apply --check --verbose
> ../v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
> Checking patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c...
>
> $ LC_ALL=C git apply --verbose
> ../v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
> Checking patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c...
> Applied patch drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c cleanly.
>


Small nit:

Reported-by: Heiner Kallweit <hkallweit1@gmail.com

Here misses closing ">" in the email-address of Heiner.

Feel free to add my:

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0 (x86-64)

- Sedat -

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Sedat Dilek]

On Sun, Apr 18, 2021 at 8:47 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Jiri Kosina <jikos@kernel.org> writes:
>
> > On Sat, 17 Apr 2021, Kalle Valo wrote:
> >
> >> This is malformed in patchwork, check the link below. Please resend, and
> >> I strongly recommend to use git send-email to avoid any format issues.
> >
> > Honestly I have no idea what you are talking about, there is no whitespace
> > damage nor anything else that I'd see to be broken. I just took the patch
> > from the mail I sent, applied with git-am, and it worked flawlessly.
>
> Compare these two links:
>
> https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
>
> https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/
>

v2 should have this diff:

$ git diff v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
v2_20210417_jikos_iwlwifi_fix_softi
rq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd-dileks.mbx
diff --git a/v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
b/v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl
_pcie_gen2_enqueue_hcmd-dileks.mbx
index 6d250b75305e..63695ce63065 100644
--- a/v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd.mbx
+++ b/v2_20210417_jikos_iwlwifi_fix_softirq_hardirq_disabling_in_iwl_pcie_gen2_enqueue_hcmd-dileks.mbx
@@ -20,9 +20,7 @@ disabling in iwl_pcie_enqueue_hcmd()"), we must
apply the same fix to
iwl_pcie_gen2_enqueue_hcmd(), as it's being called from exactly the same
contexts.

----
-
-Reported-by: Heiner Kallweit <hkallweit1@gmail.com
+Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c

Otherwise Reported-by and S-o-b is dropped when applying to my local
Git because of "---" in v2.
Closing ">" misses in Heiners Reported-by.

Jiri, can you resend a v3?

- Sedat -

> In v1 there's email discussion in the commit log which shouldn't be
> there.
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Kalle Valo]

Jiri Kosina <jikos@kernel.org> wrote:

> From: Jiri Kosina <jkosina@suse.cz>
> 
> Analogically to what we did in 2800aadc18a6 ("iwlwifi: Fix softirq/hardirq 
> disabling in iwl_pcie_enqueue_hcmd()"), we must apply the same fix to 
> iwl_pcie_gen2_enqueue_hcmd(), as it's being called from exactly the same 
> contexts.
> 
> Reported-by: Heiner Kallweit <hkallweit1@gmail.com
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> index 4456abb9a074..34bde8c87324 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
> @@ -40,6 +40,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
>  	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
>  	struct iwl_tfh_tfd *tfd;
> +	unsigned long flags;
>  
>  	copy_size = sizeof(struct iwl_cmd_header_wide);
>  	cmd_size = sizeof(struct iwl_cmd_header_wide);
> @@ -108,14 +109,14 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  		goto free_dup_buf;
>  	}
>  
> -	spin_lock_bh(&txq->lock);
> +	spin_lock_irqsave(&txq->lock, flags);
>  
>  	idx = iwl_txq_get_cmd_index(txq, txq->write_ptr);
>  	tfd = iwl_txq_get_tfd(trans, txq, txq->write_ptr);
>  	memset(tfd, 0, sizeof(*tfd));
>  
>  	if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
> -		spin_unlock_bh(&txq->lock);
> +		spin_unlock_irqrestore(&txq->lock, flags);
>  
>  		IWL_ERR(trans, "No space in command queue\n");
>  		iwl_op_mode_cmd_queue_full(trans->op_mode);
> @@ -250,7 +251,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
>  	spin_unlock(&trans_pcie->reg_lock);
>  
>  out:
> -	spin_unlock_bh(&txq->lock);
> +	spin_unlock_irqrestore(&txq->lock, flags);
>  free_dup_buf:
>  	if (idx < 0)
>  		kfree(dup_buf);

Patch applied to wireless-drivers.git, thanks.

e7020bb068d8 iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2104171112390.18270@cbobk.fhfr.pm/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Hans de Goede]

Hi,

On 4/15/21 2:21 PM, Johannes Berg wrote:
> On Thu, 2021-04-15 at 14:04 +0200, Hans de Goede wrote:
>> This fixes the same locking problem fixed by commit 2800aadc18a6 ("iwlwifi:
>> Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()") but then for
>> the gen2 variant of enqueue_hcmd().
>>
>> It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs
>> disabled (e.g. from LED core). We can't enable BHs in such a situation.
>>
>> Turn the unconditional BH-enable/BH-disable code into
>> hardirq-disable/conditional-enable.
>>
>> This fixes the warning below.
> 
> I believe Jiri posted the same patch:
> 
> https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/

Ah yes that is the same patch. I did reference Jiri's patch
for fixing the same issue in iwl_pcie_enqueue_hcmd() in the commit
message, without knowing that Jiri had send a later patch which also fixes
this in  iwl_pcie_gen2_enqueue_hcmd().

Going with Jiri's patch is fine then, but it would be nice if we
can get a fix for this in place soon-ish, which I see has
already been discussed further down in this thread :)

Regards,

Hans

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Coelho, Luciano]

On Thu, 2021-04-15 at 14:44 +0200, Jiri Kosina wrote:
> On Thu, 15 Apr 2021, Coelho, Luciano wrote:
> 
> > > I believe Jiri posted the same patch:
> > > 
> > > https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
> > > 
> > > Not sure where it is now though, I guess Luca can comment.
> > 
> > Jiri's patch is in 5.12-rc7.
> 
> The iwl_pcie_enqueue_hcmd() is. The one referenced above 
> (iwl_pcie_gen2_enqueue_hcmd()) is nowhere as far as I can tell.

Sorry, I missed that one somehow.  Kalle, I have assigned it to you. 
Can you please take it for -fixes (i.e. still for v5.12)? It's a
serious fix.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Johannes Berg]

On Thu, 2021-04-15 at 13:04 +0000, Coelho, Luciano wrote:
> On Thu, 2021-04-15 at 14:44 +0200, Jiri Kosina wrote:
> > On Thu, 15 Apr 2021, Coelho, Luciano wrote:
> > 
> > > > I believe Jiri posted the same patch:
> > > > 
> > > > https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
> > > > 
> > > > Not sure where it is now though, I guess Luca can comment.
> > > 
> > > Jiri's patch is in 5.12-rc7.
> > 
> > The iwl_pcie_enqueue_hcmd() is. The one referenced above 
> > (iwl_pcie_gen2_enqueue_hcmd()) is nowhere as far as I can tell.
> 
> Right, sorry, I confused the two.
> 
> In that same thread Johannes sent a third patch (which we have in our
> internal tree).  Johannes, with your patch, the gen2 version is also
> needed right?

Yes, they're independent.

johannes

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Coelho, Luciano]

On Thu, 2021-04-15 at 14:44 +0200, Jiri Kosina wrote:
> On Thu, 15 Apr 2021, Coelho, Luciano wrote:
> 
> > > I believe Jiri posted the same patch:
> > > 
> > > https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
> > > 
> > > Not sure where it is now though, I guess Luca can comment.
> > 
> > Jiri's patch is in 5.12-rc7.
> 
> The iwl_pcie_enqueue_hcmd() is. The one referenced above 
> (iwl_pcie_gen2_enqueue_hcmd()) is nowhere as far as I can tell.

Right, sorry, I confused the two.

In that same thread Johannes sent a third patch (which we have in our
internal tree).  Johannes, with your patch, the gen2 version is also
needed right?

Kalle, can you take that patch directly to your tree? I'll assign it to
you in patchwork.

And I'll send Johannes' patch out now too.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Jiri Kosina]

On Thu, 15 Apr 2021, Coelho, Luciano wrote:

> > I believe Jiri posted the same patch:
> > 
> > https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
> > 
> > Not sure where it is now though, I guess Luca can comment.
> 
> Jiri's patch is in 5.12-rc7.

The iwl_pcie_enqueue_hcmd() is. The one referenced above 
(iwl_pcie_gen2_enqueue_hcmd()) is nowhere as far as I can tell.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Coelho, Luciano]

On Thu, 2021-04-15 at 14:37 +0200, Johannes Berg wrote:
> On Thu, 2021-04-15 at 12:37 +0000, Coelho, Luciano wrote:
> > 
> > > I also had another fix in this area too.
> > 
> > Your patch was not sent out yet.  Is this serious enough to justify
> > trying to get it into 5.12 so late in the series? Maybe it makes more
> > sense to wait for stable...
> 
> It *is* pretty serious, and given that Linus is contemplating rc8 I'd
> probably say we could try?

Okay, I'll send it out now and we can try to take it forward.  Kalle,
is that okay with you? As usual, this is for fixes and should go
directly to your tree.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Johannes Berg]

On Thu, 2021-04-15 at 12:37 +0000, Coelho, Luciano wrote:
> 
> > I also had another fix in this area too.
> 
> Your patch was not sent out yet.  Is this serious enough to justify
> trying to get it into 5.12 so late in the series? Maybe it makes more
> sense to wait for stable...

It *is* pretty serious, and given that Linus is contemplating rc8 I'd
probably say we could try?

johannes

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Coelho, Luciano]

On Thu, 2021-04-15 at 14:21 +0200, Johannes Berg wrote:
> On Thu, 2021-04-15 at 14:04 +0200, Hans de Goede wrote:
> > This fixes the same locking problem fixed by commit 2800aadc18a6 ("iwlwifi:
> > Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()") but then for
> > the gen2 variant of enqueue_hcmd().
> > 
> > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs
> > disabled (e.g. from LED core). We can't enable BHs in such a situation.
> > 
> > Turn the unconditional BH-enable/BH-disable code into
> > hardirq-disable/conditional-enable.
> > 
> > This fixes the warning below.
> 
> I believe Jiri posted the same patch:
> 
> https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/
> 
> Not sure where it is now though, I guess Luca can comment.

Jiri's patch is in 5.12-rc7.


> I also had another fix in this area too.

Your patch was not sent out yet.  Is this serious enough to justify
trying to get it into 5.12 so late in the series? Maybe it makes more
sense to wait for stable...

--
Luca.

Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Johannes Berg]

On Thu, 2021-04-15 at 14:04 +0200, Hans de Goede wrote:
> This fixes the same locking problem fixed by commit 2800aadc18a6 ("iwlwifi:
> Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()") but then for
> the gen2 variant of enqueue_hcmd().
> 
> It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs
> disabled (e.g. from LED core). We can't enable BHs in such a situation.
> 
> Turn the unconditional BH-enable/BH-disable code into
> hardirq-disable/conditional-enable.
> 
> This fixes the warning below.

I believe Jiri posted the same patch:

https://lore.kernel.org/linux-wireless/nycvar.YFH.7.76.2104070918090.12405@cbobk.fhfr.pm/

Not sure where it is now though, I guess Luca can comment.

I also had another fix in this area too.

johannes

[PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_gen2_enqueue_hcmd()

[Hans de Goede]

This fixes the same locking problem fixed by commit 2800aadc18a6 ("iwlwifi:
Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()") but then for
the gen2 variant of enqueue_hcmd().

It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs
disabled (e.g. from LED core). We can't enable BHs in such a situation.

Turn the unconditional BH-enable/BH-disable code into
hardirq-disable/conditional-enable.

This fixes the warning below.

[   36.763543] WARNING: CPU: 6 PID: 1582 at kernel/softirq.c:178 __local_bh_enable_ip+0x97/0xd0
[   36.763550] Modules linked in: cmac bnep vfat fat snd_ctl_led snd_soc_skl_hda_dsp snd_soc_hdac_hdmi snd_soc_dmic snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_sof_pci_intel_cnl snd_sof_intel_hda_common soundwire_intel soundwire_generic_allocation soundwire_cadence snd_sof_intel_hda iTCO_wdt mei_wdt snd_sof_pci mei_hdcp intel_pmc_bxt iTCO_vendor_support snd_sof snd_sof_xtensa_dsp snd_soc_skl intel_rapl_msr snd_soc_hdac_hda snd_hda_ext_core snd_soc_sst_ipc x86_pkg_temp_thermal snd_soc_sst_dsp snd_soc_acpi_intel_match intel_powerclamp snd_soc_acpi coretemp snd_soc_core snd_compress ac97_bus cdc_ether kvm_intel usbnet snd_pcm_dmaengine r8152 snd_hda_intel mii snd_intel_dspcfg snd_usb_audio snd_intel_sdw_acpi snd_usbmidi_lib kvm snd_rawmidi snd_hda_codec irqbypass snd_hda_core snd_hwdep rapl intel_cstate snd_seq intel_uncore iwlmvm snd_seq_device pcspkr snd_pcm wmi_bmof intel_wmi_thunderbolt e1000e mac80211 uvcvideo btusb i2c_i801 videobuf2_vmalloc videobuf2_memops btrtl
[   36.763661]  snd_timer i2c_smbus videobuf2_v4l2 btbcm btintel libarc4 thunderbolt videobuf2_common bluetooth videodev mei_me iwlwifi mei ecdh_generic mc ecc nxp_nci_i2c nxp_nci joydev processor_thermal_device ucsi_acpi nci processor_thermal_rfim processor_thermal_mbox cfg80211 typec_ucsi processor_thermal_rapl intel_pch_thermal intel_rapl_common idma64 intel_soc_dts_iosf typec nfc int3403_thermal int340x_thermal_zone soc_button_array intel_hid sparse_keymap acpi_pad int3400_thermal acpi_thermal_rel binfmt_misc zram ip_tables dm_crypt trusted hid_logitech_hidpp hid_logitech_dj uas usb_storage crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel i915 serio_raw nvme nvme_core i2c_algo_bit drm_kms_helper cec hid_multitouch drm wmi i2c_hid_acpi i2c_hid thinkpad_acpi ledtrig_audio platform_profile snd soundcore rfkill drm_privacy_screen_helper video pinctrl_cannonlake i2c_dev fuse
[   36.763775] CPU: 6 PID: 1582 Comm: NetworkManager Not tainted 5.12.0-rc7+ #303
[   36.763778] Hardware name: LENOVO 20U90SIT19/20U90SIT19, BIOS N2WET24W (1.14 ) 10/15/2020
[   36.763780] RIP: 0010:__local_bh_enable_ip+0x97/0xd0
[   36.763783] Code: f3 48 a9 00 ff ff 00 74 36 65 ff 0d b3 02 f3 48 e8 de ee 12 00 fb 66 0f 1f 44 00 00 5b 5d c3 65 8b 05 d9 0a f3 48 85 c0 75 9c <0f> 0b eb 98 e8 00 ee 12 00 eb a7 48 89 ef e8 16 14 07 00 eb b0 65
[   36.763786] RSP: 0018:ffffbc2e016b72d0 EFLAGS: 00010046
[   36.763790] RAX: 0000000000000000 RBX: 0000000000000201 RCX: 0000000000000000
[   36.763792] RDX: 0000000000000003 RSI: 0000000000000201 RDI: ffffffffc0a8527b
[   36.763794] RBP: ffffffffc0a8527b R08: 0000000000000000 R09: 0000000000000001
[   36.763796] R10: ffffbc2e016b71c0 R11: 0000000000000001 R12: ffffa0570c902c10
[   36.763798] R13: 0000000000000000 R14: 0000000080000000 R15: 0000000000000000
[   36.763801] FS:  00007f44b889bbc0(0000) GS:ffffa05a5a580000(0000) knlGS:0000000000000000
[   36.763803] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   36.763805] CR2: 00007fde6d88b8f0 CR3: 0000000142b08005 CR4: 00000000003706e0
[   36.763808] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   36.763810] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   36.763812] Call Trace:
[   36.763815]  iwl_pcie_gen2_enqueue_hcmd+0x56b/0x8c0 [iwlwifi]
[   36.763848]  iwl_trans_txq_send_hcmd+0x59/0x440 [iwlwifi]
[   36.763872]  iwl_trans_send_cmd+0x81/0x180 [iwlwifi]
[   36.763890]  iwl_mvm_send_cmd+0x28/0x80 [iwlmvm]
[   36.763910]  iwl_mvm_led_set+0x9c/0xc0 [iwlmvm]
[   36.763933]  ? _raw_read_lock_irqsave+0x88/0xa0
[   36.763938]  led_trigger_event+0x46/0x70
[   36.763945]  ieee80211_do_open+0x4c2/0xa70 [mac80211]
[   36.764006]  ieee80211_open+0x69/0x90 [mac80211]
[   36.764057]  __dev_open+0xd4/0x1a0
[   36.764063]  __dev_change_flags+0x1c8/0x240
[   36.764072]  dev_change_flags+0x21/0x60
[   36.764077]  do_setlink+0x238/0x1110
[   36.764084]  ? cpumask_next+0x17/0x20
[   36.764087]  ? __snmp6_fill_stats64.constprop.0+0x53/0xe0
[   36.764093]  ? __nla_validate_parse+0x4f/0xbf0
[   36.764109]  __rtnl_newlink+0x601/0x9b0
[   36.764116]  ? __lock_acquire+0x389/0x1e10
[   36.764130]  ? lock_acquire+0xb5/0x380
[   36.764134]  ? sock_def_readable+0x5/0x2a0
[   36.764136]  ? lock_is_held_type+0xa5/0x120
[   36.764141]  ? find_held_lock+0x2b/0x80
[   36.764146]  ? sock_def_readable+0xb0/0x2a0
[   36.764148]  ? lock_release+0xba/0x2a0
[   36.764156]  ? netlink_unicast+0x1f7/0x230
[   36.764163]  ? rtnl_getlink+0x364/0x3e0
[   36.764207]  ? rcu_read_lock_sched_held+0x3f/0x80
[   36.764211]  ? kmem_cache_alloc_trace+0x29a/0x2c0
[   36.764220]  rtnl_newlink+0x44/0x70
[   36.764225]  rtnetlink_rcv_msg+0x16e/0x480
[   36.764229]  ? netlink_deliver_tap+0x95/0x3d0
[   36.764236]  ? rtnetlink_put_metrics+0x1c0/0x1c0
[   36.764241]  netlink_rcv_skb+0x50/0xf0
[   36.764251]  netlink_unicast+0x16d/0x230
[   36.764258]  netlink_sendmsg+0x24d/0x480
[   36.764270]  sock_sendmsg+0x5e/0x60
[   36.764274]  ____sys_sendmsg+0x22f/0x270
[   36.764278]  ? import_iovec+0x17/0x20
[   36.764282]  ? sendmsg_copy_msghdr+0x59/0x90
[   36.764290]  ___sys_sendmsg+0x81/0xc0
[   36.764303]  ? lock_is_held_type+0xa5/0x120
[   36.764307]  ? find_held_lock+0x2b/0x80
[   36.764313]  ? __fget_files+0xd0/0x1a0
[   36.764316]  ? lock_release+0xba/0x2a0
[   36.764324]  ? __fget_files+0xef/0x1a0
[   36.764333]  __sys_sendmsg+0x49/0x80
[   36.764342]  ? syscall_enter_from_user_mode+0x27/0x80
[   36.764348]  do_syscall_64+0x33/0x40
[   36.764352]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   36.764355] RIP: 0033:0x7f44b99876fd
[   36.764359] Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 fa ee ff ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 4e ef ff ff 48
[   36.764361] RSP: 002b:00007ffca4a55940 EFLAGS: 00000293 ORIG_RAX: 000000000000002e
[   36.764365] RAX: ffffffffffffffda RBX: 0000000000000014 RCX: 00007f44b99876fd
[   36.764368] RDX: 0000000000000000 RSI: 00007ffca4a55980 RDI: 000000000000000c
[   36.764370] RBP: 0000560f3b81d030 R08: 0000000000000000 R09: 0000000000000000
[   36.764372] R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000000
[   36.764374] R13: 00007ffca4a55ae0 R14: 00007ffca4a55adc R15: 0000000000000000
[   36.764390] irq event stamp: 246365
[   36.764391] hardirqs last  enabled at (246363): [<ffffffffb70e7b12>] __local_bh_enable_ip+0x82/0xd0
[   36.764395] hardirqs last disabled at (246364): [<ffffffffb7d111c4>] _raw_read_lock_irqsave+0x94/0xa0
[   36.764398] softirqs last  enabled at (246362): [<ffffffffc0a8527b>] iwl_pcie_gen2_enqueue_hcmd+0x56b/0x8c0 [iwlwifi]
[   36.764414] softirqs last disabled at (246365): [<ffffffffc0a84dfb>] iwl_pcie_gen2_enqueue_hcmd+0xeb/0x8c0 [iwlwifi]

Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
index 4456abb9a074..34bde8c87324 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
@@ -40,6 +40,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
 	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
 	struct iwl_tfh_tfd *tfd;
+	unsigned long flags;
 
 	copy_size = sizeof(struct iwl_cmd_header_wide);
 	cmd_size = sizeof(struct iwl_cmd_header_wide);
@@ -108,14 +109,14 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 		goto free_dup_buf;
 	}
 
-	spin_lock_bh(&txq->lock);
+	spin_lock_irqsave(&txq->lock, flags);
 
 	idx = iwl_txq_get_cmd_index(txq, txq->write_ptr);
 	tfd = iwl_txq_get_tfd(trans, txq, txq->write_ptr);
 	memset(tfd, 0, sizeof(*tfd));
 
 	if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
-		spin_unlock_bh(&txq->lock);
+		spin_unlock_irqrestore(&txq->lock, flags);
 
 		IWL_ERR(trans, "No space in command queue\n");
 		iwl_op_mode_cmd_queue_full(trans->op_mode);
@@ -250,7 +251,7 @@ int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	spin_unlock(&trans_pcie->reg_lock);
 
 out:
-	spin_unlock_bh(&txq->lock);
+	spin_unlock_irqrestore(&txq->lock, flags);
 free_dup_buf:
 	if (idx < 0)
 		kfree(dup_buf);
-- 
2.31.1