All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt
@ 2023-04-25 16:40 Mirsad Goran Todorovac
  2023-04-26  6:41 ` Leon Romanovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Mirsad Goran Todorovac @ 2023-04-25 16:40 UTC (permalink / raw
  To: Johannes Berg, linux-wireless, netdev, linux-kernel
  Cc: Johannes Berg, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Mirsad Goran Todorovac, Gregory Greenman,
	Leon Romanovsky, Alexander Wetzel

In the function ieee80211_tx_dequeue() there is a particular locking
sequence:

begin:
	spin_lock(&local->queue_stop_reason_lock);
	q_stopped = local->queue_stop_reasons[q];
	spin_unlock(&local->queue_stop_reason_lock);

However small the chance (increased by ftracetest), an asynchronous
interrupt can occur in between of spin_lock() and spin_unlock(),
and the interrupt routine will attempt to lock the same
&local->queue_stop_reason_lock again.

This will cause a costly reset of the CPU and the wifi device or an
altogether hang in the single CPU and single core scenario.

The only remaining spin_lock(&local->queue_stop_reason_lock) that
did not disable interrupts was patched, which should prevent any
deadlocks on the same CPU/core and the same wifi device.

This is the probable trace of the deadlock:

kernel: ================================
kernel: WARNING: inconsistent lock state
kernel: 6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4 Tainted: G        W
kernel: --------------------------------
kernel: inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
kernel: kworker/5:0/25656 [HC0[0]:SC0[0]:HE1:SE1] takes:
kernel: ffff9d6190779478 (&local->queue_stop_reason_lock){+.?.}-{2:2}, at: return_to_handler+0x0/0x40
kernel: {IN-SOFTIRQ-W} state was registered at:
kernel:   lock_acquire+0xc7/0x2d0
kernel:   _raw_spin_lock+0x36/0x50
kernel:   ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
kernel:   iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
kernel:   iwl_mvm_mac_wake_tx_queue+0x2d/0xd0 [iwlmvm]
kernel:   ieee80211_queue_skb+0x450/0x730 [mac80211]
kernel:   __ieee80211_xmit_fast.constprop.66+0x834/0xa50 [mac80211]
kernel:   __ieee80211_subif_start_xmit+0x217/0x530 [mac80211]
kernel:   ieee80211_subif_start_xmit+0x60/0x580 [mac80211]
kernel:   dev_hard_start_xmit+0xb5/0x260
kernel:   __dev_queue_xmit+0xdbe/0x1200
kernel:   neigh_resolve_output+0x166/0x260
kernel:   ip_finish_output2+0x216/0xb80
kernel:   __ip_finish_output+0x2a4/0x4d0
kernel:   ip_finish_output+0x2d/0xd0
kernel:   ip_output+0x82/0x2b0
kernel:   ip_local_out+0xec/0x110
kernel:   igmpv3_sendpack+0x5c/0x90
kernel:   igmp_ifc_timer_expire+0x26e/0x4e0
kernel:   call_timer_fn+0xa5/0x230
kernel:   run_timer_softirq+0x27f/0x550
kernel:   __do_softirq+0xb4/0x3a4
kernel:   irq_exit_rcu+0x9b/0xc0
kernel:   sysvec_apic_timer_interrupt+0x80/0xa0
kernel:   asm_sysvec_apic_timer_interrupt+0x1f/0x30
kernel:   _raw_spin_unlock_irqrestore+0x3f/0x70
kernel:   free_to_partial_list+0x3d6/0x590
kernel:   __slab_free+0x1b7/0x310
kernel:   kmem_cache_free+0x52d/0x550
kernel:   putname+0x5d/0x70
kernel:   do_sys_openat2+0x1d7/0x310
kernel:   do_sys_open+0x51/0x80
kernel:   __x64_sys_openat+0x24/0x30
kernel:   do_syscall_64+0x5c/0x90
kernel:   entry_SYSCALL_64_after_hwframe+0x72/0xdc
kernel: irq event stamp: 5120729
kernel: hardirqs last  enabled at (5120729): [<ffffffff9d149936>] trace_graph_return+0xd6/0x120
kernel: hardirqs last disabled at (5120728): [<ffffffff9d149950>] trace_graph_return+0xf0/0x120
kernel: softirqs last  enabled at (5069900): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
kernel: softirqs last disabled at (5067555): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
kernel:
        other info that might help us debug this:
kernel:  Possible unsafe locking scenario:
kernel:        CPU0
kernel:        ----
kernel:   lock(&local->queue_stop_reason_lock);
kernel:   <Interrupt>
kernel:     lock(&local->queue_stop_reason_lock);
kernel:
         *** DEADLOCK ***
kernel: 8 locks held by kworker/5:0/25656:
kernel:  #0: ffff9d618009d138 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x1ca/0x530
kernel:  #1: ffffb1ef4637fe68 ((work_completion)(&local->restart_work)){+.+.}-{0:0}, at: process_one_work+0x1ce/0x530
kernel:  #2: ffffffff9f166548 (rtnl_mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
kernel:  #3: ffff9d6190778728 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
kernel:  #4: ffff9d619077b480 (&mvm->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
kernel:  #5: ffff9d61907bacd8 (&trans_pcie->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
kernel:  #6: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_queue_state_change+0x59/0x3a0 [iwlmvm]
kernel:  #7: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
kernel:
        stack backtrace:
kernel: CPU: 5 PID: 25656 Comm: kworker/5:0 Tainted: G        W          6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4
kernel: Hardware name: LENOVO 82H8/LNVNB161216, BIOS GGCN51WW 11/16/2022
kernel: Workqueue: events_freezable ieee80211_restart_work [mac80211]
kernel: Call Trace:
kernel:  <TASK>
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  dump_stack_lvl+0x5f/0xa0
kernel:  dump_stack+0x14/0x20
kernel:  print_usage_bug.part.46+0x208/0x2a0
kernel:  mark_lock.part.47+0x605/0x630
kernel:  ? sched_clock+0xd/0x20
kernel:  ? trace_clock_local+0x14/0x30
kernel:  ? __rb_reserve_next+0x5f/0x490
kernel:  ? _raw_spin_lock+0x1b/0x50
kernel:  __lock_acquire+0x464/0x1990
kernel:  ? mark_held_locks+0x4e/0x80
kernel:  lock_acquire+0xc7/0x2d0
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  ? ftrace_return_to_handler+0x8b/0x100
kernel:  ? preempt_count_add+0x4/0x70
kernel:  _raw_spin_lock+0x36/0x50
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
kernel:  ? prepare_ftrace_return+0xc5/0x190
kernel:  ? ftrace_graph_func+0x16/0x20
kernel:  ? 0xffffffffc02ab0b1
kernel:  ? lock_acquire+0xc7/0x2d0
kernel:  ? iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
kernel:  ? ieee80211_tx_dequeue+0x9/0x1330 [mac80211]
kernel:  ? __rcu_read_lock+0x4/0x40
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_mvm_queue_state_change+0x311/0x3a0 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_mvm_wake_sw_queue+0x17/0x20 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_txq_gen2_unmap+0x1c9/0x1f0 [iwlwifi]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_txq_gen2_free+0x55/0x130 [iwlwifi]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_txq_gen2_tx_free+0x63/0x80 [iwlwifi]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  _iwl_trans_pcie_gen2_stop_device+0x3f3/0x5b0 [iwlwifi]
kernel:  ? _iwl_trans_pcie_gen2_stop_device+0x9/0x5b0 [iwlwifi]
kernel:  ? mutex_lock_nested+0x4/0x30
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_trans_pcie_gen2_stop_device+0x5f/0x90 [iwlwifi]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_mvm_stop_device+0x78/0xd0 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  __iwl_mvm_mac_start+0x114/0x210 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  iwl_mvm_mac_start+0x76/0x150 [iwlmvm]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  drv_start+0x79/0x180 [mac80211]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  ieee80211_reconfig+0x1523/0x1ce0 [mac80211]
kernel:  ? synchronize_net+0x4/0x50
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  ieee80211_restart_work+0x108/0x170 [mac80211]
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  process_one_work+0x250/0x530
kernel:  ? ftrace_regs_caller_end+0x66/0x66
kernel:  worker_thread+0x48/0x3a0
kernel:  ? __pfx_worker_thread+0x10/0x10
kernel:  kthread+0x10f/0x140
kernel:  ? __pfx_kthread+0x10/0x10
kernel:  ret_from_fork+0x29/0x50
kernel:  </TASK>

Fixes: 4444bc2116ae ("wifi: mac80211: Proper mark iTXQs for resumption")
Link: https://lore.kernel.org/all/1f58a0d1-d2b9-d851-73c3-93fcc607501c@alu.unizg.hr/
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Cc: Gregory Greenman <gregory.greenman@intel.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Link: https://lore.kernel.org/all/cdc80531-f25f-6f9d-b15f-25e16130b53a@alu.unizg.hr/
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Alexander Wetzel <alexander@wetzel-home.de>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
---
v3 -> v4:
- Added whole lockdep trace as advised.
- Trimmed irrelevant line prefix.
v2 -> v3:
- Fix the Fixes: tag as advised.
- Change the net: to wifi: to comply with the original patch that
  is being fixed.
v1 -> v2:
- Minor rewording and clarification.
- Cc:-ed people that replied to the original bug report (forgotten
  in v1 by omission).

 net/mac80211/tx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7699fb410670..45cb8e7bcc61 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3781,6 +3781,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	ieee80211_tx_result r;
 	struct ieee80211_vif *vif = txq->vif;
 	int q = vif->hw_queue[txq->ac];
+	unsigned long flags;
 	bool q_stopped;
 
 	WARN_ON_ONCE(softirq_count() == 0);
@@ -3789,9 +3790,9 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 		return NULL;
 
 begin:
-	spin_lock(&local->queue_stop_reason_lock);
+	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 	q_stopped = local->queue_stop_reasons[q];
-	spin_unlock(&local->queue_stop_reason_lock);
+	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
 	if (unlikely(q_stopped)) {
 		/* mark for waking later */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt
  2023-04-25 16:40 [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt Mirsad Goran Todorovac
@ 2023-04-26  6:41 ` Leon Romanovsky
  2023-04-26 14:02   ` Mirsad Todorovac
  0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2023-04-26  6:41 UTC (permalink / raw
  To: Mirsad Goran Todorovac
  Cc: Johannes Berg, linux-wireless, netdev, linux-kernel,
	Johannes Berg, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Gregory Greenman, Alexander Wetzel

On Tue, Apr 25, 2023 at 06:40:08PM +0200, Mirsad Goran Todorovac wrote:
> In the function ieee80211_tx_dequeue() there is a particular locking
> sequence:
> 
> begin:
> 	spin_lock(&local->queue_stop_reason_lock);
> 	q_stopped = local->queue_stop_reasons[q];
> 	spin_unlock(&local->queue_stop_reason_lock);
> 
> However small the chance (increased by ftracetest), an asynchronous
> interrupt can occur in between of spin_lock() and spin_unlock(),
> and the interrupt routine will attempt to lock the same
> &local->queue_stop_reason_lock again.
> 
> This will cause a costly reset of the CPU and the wifi device or an
> altogether hang in the single CPU and single core scenario.
> 
> The only remaining spin_lock(&local->queue_stop_reason_lock) that
> did not disable interrupts was patched, which should prevent any
> deadlocks on the same CPU/core and the same wifi device.
> 
> This is the probable trace of the deadlock:
> 
> kernel: ================================
> kernel: WARNING: inconsistent lock state
> kernel: 6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4 Tainted: G        W
> kernel: --------------------------------
> kernel: inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> kernel: kworker/5:0/25656 [HC0[0]:SC0[0]:HE1:SE1] takes:
> kernel: ffff9d6190779478 (&local->queue_stop_reason_lock){+.?.}-{2:2}, at: return_to_handler+0x0/0x40
> kernel: {IN-SOFTIRQ-W} state was registered at:
> kernel:   lock_acquire+0xc7/0x2d0
> kernel:   _raw_spin_lock+0x36/0x50
> kernel:   ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
> kernel:   iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
> kernel:   iwl_mvm_mac_wake_tx_queue+0x2d/0xd0 [iwlmvm]
> kernel:   ieee80211_queue_skb+0x450/0x730 [mac80211]
> kernel:   __ieee80211_xmit_fast.constprop.66+0x834/0xa50 [mac80211]
> kernel:   __ieee80211_subif_start_xmit+0x217/0x530 [mac80211]
> kernel:   ieee80211_subif_start_xmit+0x60/0x580 [mac80211]
> kernel:   dev_hard_start_xmit+0xb5/0x260
> kernel:   __dev_queue_xmit+0xdbe/0x1200
> kernel:   neigh_resolve_output+0x166/0x260
> kernel:   ip_finish_output2+0x216/0xb80
> kernel:   __ip_finish_output+0x2a4/0x4d0
> kernel:   ip_finish_output+0x2d/0xd0
> kernel:   ip_output+0x82/0x2b0
> kernel:   ip_local_out+0xec/0x110
> kernel:   igmpv3_sendpack+0x5c/0x90
> kernel:   igmp_ifc_timer_expire+0x26e/0x4e0
> kernel:   call_timer_fn+0xa5/0x230
> kernel:   run_timer_softirq+0x27f/0x550
> kernel:   __do_softirq+0xb4/0x3a4
> kernel:   irq_exit_rcu+0x9b/0xc0
> kernel:   sysvec_apic_timer_interrupt+0x80/0xa0
> kernel:   asm_sysvec_apic_timer_interrupt+0x1f/0x30
> kernel:   _raw_spin_unlock_irqrestore+0x3f/0x70
> kernel:   free_to_partial_list+0x3d6/0x590
> kernel:   __slab_free+0x1b7/0x310
> kernel:   kmem_cache_free+0x52d/0x550
> kernel:   putname+0x5d/0x70
> kernel:   do_sys_openat2+0x1d7/0x310
> kernel:   do_sys_open+0x51/0x80
> kernel:   __x64_sys_openat+0x24/0x30
> kernel:   do_syscall_64+0x5c/0x90
> kernel:   entry_SYSCALL_64_after_hwframe+0x72/0xdc
> kernel: irq event stamp: 5120729
> kernel: hardirqs last  enabled at (5120729): [<ffffffff9d149936>] trace_graph_return+0xd6/0x120
> kernel: hardirqs last disabled at (5120728): [<ffffffff9d149950>] trace_graph_return+0xf0/0x120
> kernel: softirqs last  enabled at (5069900): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
> kernel: softirqs last disabled at (5067555): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
> kernel:
>         other info that might help us debug this:
> kernel:  Possible unsafe locking scenario:
> kernel:        CPU0
> kernel:        ----
> kernel:   lock(&local->queue_stop_reason_lock);
> kernel:   <Interrupt>
> kernel:     lock(&local->queue_stop_reason_lock);
> kernel:
>          *** DEADLOCK ***
> kernel: 8 locks held by kworker/5:0/25656:
> kernel:  #0: ffff9d618009d138 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x1ca/0x530
> kernel:  #1: ffffb1ef4637fe68 ((work_completion)(&local->restart_work)){+.+.}-{0:0}, at: process_one_work+0x1ce/0x530
> kernel:  #2: ffffffff9f166548 (rtnl_mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
> kernel:  #3: ffff9d6190778728 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
> kernel:  #4: ffff9d619077b480 (&mvm->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
> kernel:  #5: ffff9d61907bacd8 (&trans_pcie->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
> kernel:  #6: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_queue_state_change+0x59/0x3a0 [iwlmvm]
> kernel:  #7: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
> kernel:
>         stack backtrace:
> kernel: CPU: 5 PID: 25656 Comm: kworker/5:0 Tainted: G        W          6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4
> kernel: Hardware name: LENOVO 82H8/LNVNB161216, BIOS GGCN51WW 11/16/2022
> kernel: Workqueue: events_freezable ieee80211_restart_work [mac80211]
> kernel: Call Trace:
> kernel:  <TASK>
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  dump_stack_lvl+0x5f/0xa0
> kernel:  dump_stack+0x14/0x20
> kernel:  print_usage_bug.part.46+0x208/0x2a0
> kernel:  mark_lock.part.47+0x605/0x630
> kernel:  ? sched_clock+0xd/0x20
> kernel:  ? trace_clock_local+0x14/0x30
> kernel:  ? __rb_reserve_next+0x5f/0x490
> kernel:  ? _raw_spin_lock+0x1b/0x50
> kernel:  __lock_acquire+0x464/0x1990
> kernel:  ? mark_held_locks+0x4e/0x80
> kernel:  lock_acquire+0xc7/0x2d0
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  ? ftrace_return_to_handler+0x8b/0x100
> kernel:  ? preempt_count_add+0x4/0x70
> kernel:  _raw_spin_lock+0x36/0x50
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
> kernel:  ? prepare_ftrace_return+0xc5/0x190
> kernel:  ? ftrace_graph_func+0x16/0x20
> kernel:  ? 0xffffffffc02ab0b1
> kernel:  ? lock_acquire+0xc7/0x2d0
> kernel:  ? iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
> kernel:  ? ieee80211_tx_dequeue+0x9/0x1330 [mac80211]
> kernel:  ? __rcu_read_lock+0x4/0x40
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_mvm_queue_state_change+0x311/0x3a0 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_mvm_wake_sw_queue+0x17/0x20 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_txq_gen2_unmap+0x1c9/0x1f0 [iwlwifi]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_txq_gen2_free+0x55/0x130 [iwlwifi]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_txq_gen2_tx_free+0x63/0x80 [iwlwifi]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  _iwl_trans_pcie_gen2_stop_device+0x3f3/0x5b0 [iwlwifi]
> kernel:  ? _iwl_trans_pcie_gen2_stop_device+0x9/0x5b0 [iwlwifi]
> kernel:  ? mutex_lock_nested+0x4/0x30
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_trans_pcie_gen2_stop_device+0x5f/0x90 [iwlwifi]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_mvm_stop_device+0x78/0xd0 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  __iwl_mvm_mac_start+0x114/0x210 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  iwl_mvm_mac_start+0x76/0x150 [iwlmvm]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  drv_start+0x79/0x180 [mac80211]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  ieee80211_reconfig+0x1523/0x1ce0 [mac80211]
> kernel:  ? synchronize_net+0x4/0x50
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  ieee80211_restart_work+0x108/0x170 [mac80211]
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  process_one_work+0x250/0x530
> kernel:  ? ftrace_regs_caller_end+0x66/0x66
> kernel:  worker_thread+0x48/0x3a0
> kernel:  ? __pfx_worker_thread+0x10/0x10
> kernel:  kthread+0x10f/0x140
> kernel:  ? __pfx_kthread+0x10/0x10
> kernel:  ret_from_fork+0x29/0x50
> kernel:  </TASK>
> 
> Fixes: 4444bc2116ae ("wifi: mac80211: Proper mark iTXQs for resumption")
> Link: https://lore.kernel.org/all/1f58a0d1-d2b9-d851-73c3-93fcc607501c@alu.unizg.hr/
> Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> Cc: Gregory Greenman <gregory.greenman@intel.com>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Link: https://lore.kernel.org/all/cdc80531-f25f-6f9d-b15f-25e16130b53a@alu.unizg.hr/
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Alexander Wetzel <alexander@wetzel-home.de>
> Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> ---
> v3 -> v4:
> - Added whole lockdep trace as advised.
> - Trimmed irrelevant line prefix.
> v2 -> v3:
> - Fix the Fixes: tag as advised.
> - Change the net: to wifi: to comply with the original patch that
>   is being fixed.
> v1 -> v2:
> - Minor rewording and clarification.
> - Cc:-ed people that replied to the original bug report (forgotten
>   in v1 by omission).
> 
>  net/mac80211/tx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt
  2023-04-26  6:41 ` Leon Romanovsky
@ 2023-04-26 14:02   ` Mirsad Todorovac
  2023-04-26 15:05     ` Johannes Berg
  0 siblings, 1 reply; 5+ messages in thread
From: Mirsad Todorovac @ 2023-04-26 14:02 UTC (permalink / raw
  To: Leon Romanovsky
  Cc: Johannes Berg, linux-wireless, netdev, linux-kernel,
	Johannes Berg, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Gregory Greenman, Alexander Wetzel

On 4/26/23 08:41, Leon Romanovsky wrote:
> On Tue, Apr 25, 2023 at 06:40:08PM +0200, Mirsad Goran Todorovac wrote:
>> In the function ieee80211_tx_dequeue() there is a particular locking
>> sequence:
>>
>> begin:
>> 	spin_lock(&local->queue_stop_reason_lock);
>> 	q_stopped = local->queue_stop_reasons[q];
>> 	spin_unlock(&local->queue_stop_reason_lock);
>>
>> However small the chance (increased by ftracetest), an asynchronous
>> interrupt can occur in between of spin_lock() and spin_unlock(),
>> and the interrupt routine will attempt to lock the same
>> &local->queue_stop_reason_lock again.
>>
>> This will cause a costly reset of the CPU and the wifi device or an
>> altogether hang in the single CPU and single core scenario.
>>
>> The only remaining spin_lock(&local->queue_stop_reason_lock) that
>> did not disable interrupts was patched, which should prevent any
>> deadlocks on the same CPU/core and the same wifi device.
>>
>> This is the probable trace of the deadlock:
>>
>> kernel: ================================
>> kernel: WARNING: inconsistent lock state
>> kernel: 6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4 Tainted: G        W
>> kernel: --------------------------------
>> kernel: inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
>> kernel: kworker/5:0/25656 [HC0[0]:SC0[0]:HE1:SE1] takes:
>> kernel: ffff9d6190779478 (&local->queue_stop_reason_lock){+.?.}-{2:2}, at: return_to_handler+0x0/0x40
>> kernel: {IN-SOFTIRQ-W} state was registered at:
>> kernel:   lock_acquire+0xc7/0x2d0
>> kernel:   _raw_spin_lock+0x36/0x50
>> kernel:   ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
>> kernel:   iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
>> kernel:   iwl_mvm_mac_wake_tx_queue+0x2d/0xd0 [iwlmvm]
>> kernel:   ieee80211_queue_skb+0x450/0x730 [mac80211]
>> kernel:   __ieee80211_xmit_fast.constprop.66+0x834/0xa50 [mac80211]
>> kernel:   __ieee80211_subif_start_xmit+0x217/0x530 [mac80211]
>> kernel:   ieee80211_subif_start_xmit+0x60/0x580 [mac80211]
>> kernel:   dev_hard_start_xmit+0xb5/0x260
>> kernel:   __dev_queue_xmit+0xdbe/0x1200
>> kernel:   neigh_resolve_output+0x166/0x260
>> kernel:   ip_finish_output2+0x216/0xb80
>> kernel:   __ip_finish_output+0x2a4/0x4d0
>> kernel:   ip_finish_output+0x2d/0xd0
>> kernel:   ip_output+0x82/0x2b0
>> kernel:   ip_local_out+0xec/0x110
>> kernel:   igmpv3_sendpack+0x5c/0x90
>> kernel:   igmp_ifc_timer_expire+0x26e/0x4e0
>> kernel:   call_timer_fn+0xa5/0x230
>> kernel:   run_timer_softirq+0x27f/0x550
>> kernel:   __do_softirq+0xb4/0x3a4
>> kernel:   irq_exit_rcu+0x9b/0xc0
>> kernel:   sysvec_apic_timer_interrupt+0x80/0xa0
>> kernel:   asm_sysvec_apic_timer_interrupt+0x1f/0x30
>> kernel:   _raw_spin_unlock_irqrestore+0x3f/0x70
>> kernel:   free_to_partial_list+0x3d6/0x590
>> kernel:   __slab_free+0x1b7/0x310
>> kernel:   kmem_cache_free+0x52d/0x550
>> kernel:   putname+0x5d/0x70
>> kernel:   do_sys_openat2+0x1d7/0x310
>> kernel:   do_sys_open+0x51/0x80
>> kernel:   __x64_sys_openat+0x24/0x30
>> kernel:   do_syscall_64+0x5c/0x90
>> kernel:   entry_SYSCALL_64_after_hwframe+0x72/0xdc
>> kernel: irq event stamp: 5120729
>> kernel: hardirqs last  enabled at (5120729): [<ffffffff9d149936>] trace_graph_return+0xd6/0x120
>> kernel: hardirqs last disabled at (5120728): [<ffffffff9d149950>] trace_graph_return+0xf0/0x120
>> kernel: softirqs last  enabled at (5069900): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
>> kernel: softirqs last disabled at (5067555): [<ffffffff9cf65b60>] return_to_handler+0x0/0x40
>> kernel:
>>          other info that might help us debug this:
>> kernel:  Possible unsafe locking scenario:
>> kernel:        CPU0
>> kernel:        ----
>> kernel:   lock(&local->queue_stop_reason_lock);
>> kernel:   <Interrupt>
>> kernel:     lock(&local->queue_stop_reason_lock);
>> kernel:
>>           *** DEADLOCK ***
>> kernel: 8 locks held by kworker/5:0/25656:
>> kernel:  #0: ffff9d618009d138 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x1ca/0x530
>> kernel:  #1: ffffb1ef4637fe68 ((work_completion)(&local->restart_work)){+.+.}-{0:0}, at: process_one_work+0x1ce/0x530
>> kernel:  #2: ffffffff9f166548 (rtnl_mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
>> kernel:  #3: ffff9d6190778728 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
>> kernel:  #4: ffff9d619077b480 (&mvm->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
>> kernel:  #5: ffff9d61907bacd8 (&trans_pcie->mutex){+.+.}-{3:3}, at: return_to_handler+0x0/0x40
>> kernel:  #6: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_queue_state_change+0x59/0x3a0 [iwlmvm]
>> kernel:  #7: ffffffff9ef9cda0 (rcu_read_lock){....}-{1:2}, at: iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
>> kernel:
>>          stack backtrace:
>> kernel: CPU: 5 PID: 25656 Comm: kworker/5:0 Tainted: G        W          6.3.0-rc6-mt-20230401-00001-gf86822a1170f #4
>> kernel: Hardware name: LENOVO 82H8/LNVNB161216, BIOS GGCN51WW 11/16/2022
>> kernel: Workqueue: events_freezable ieee80211_restart_work [mac80211]
>> kernel: Call Trace:
>> kernel:  <TASK>
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  dump_stack_lvl+0x5f/0xa0
>> kernel:  dump_stack+0x14/0x20
>> kernel:  print_usage_bug.part.46+0x208/0x2a0
>> kernel:  mark_lock.part.47+0x605/0x630
>> kernel:  ? sched_clock+0xd/0x20
>> kernel:  ? trace_clock_local+0x14/0x30
>> kernel:  ? __rb_reserve_next+0x5f/0x490
>> kernel:  ? _raw_spin_lock+0x1b/0x50
>> kernel:  __lock_acquire+0x464/0x1990
>> kernel:  ? mark_held_locks+0x4e/0x80
>> kernel:  lock_acquire+0xc7/0x2d0
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  ? ftrace_return_to_handler+0x8b/0x100
>> kernel:  ? preempt_count_add+0x4/0x70
>> kernel:  _raw_spin_lock+0x36/0x50
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  ieee80211_tx_dequeue+0xb4/0x1330 [mac80211]
>> kernel:  ? prepare_ftrace_return+0xc5/0x190
>> kernel:  ? ftrace_graph_func+0x16/0x20
>> kernel:  ? 0xffffffffc02ab0b1
>> kernel:  ? lock_acquire+0xc7/0x2d0
>> kernel:  ? iwl_mvm_mac_itxq_xmit+0x42/0x210 [iwlmvm]
>> kernel:  ? ieee80211_tx_dequeue+0x9/0x1330 [mac80211]
>> kernel:  ? __rcu_read_lock+0x4/0x40
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_mvm_mac_itxq_xmit+0xae/0x210 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_mvm_queue_state_change+0x311/0x3a0 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_mvm_wake_sw_queue+0x17/0x20 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_txq_gen2_unmap+0x1c9/0x1f0 [iwlwifi]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_txq_gen2_free+0x55/0x130 [iwlwifi]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_txq_gen2_tx_free+0x63/0x80 [iwlwifi]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  _iwl_trans_pcie_gen2_stop_device+0x3f3/0x5b0 [iwlwifi]
>> kernel:  ? _iwl_trans_pcie_gen2_stop_device+0x9/0x5b0 [iwlwifi]
>> kernel:  ? mutex_lock_nested+0x4/0x30
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_trans_pcie_gen2_stop_device+0x5f/0x90 [iwlwifi]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_mvm_stop_device+0x78/0xd0 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  __iwl_mvm_mac_start+0x114/0x210 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  iwl_mvm_mac_start+0x76/0x150 [iwlmvm]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  drv_start+0x79/0x180 [mac80211]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  ieee80211_reconfig+0x1523/0x1ce0 [mac80211]
>> kernel:  ? synchronize_net+0x4/0x50
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  ieee80211_restart_work+0x108/0x170 [mac80211]
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  process_one_work+0x250/0x530
>> kernel:  ? ftrace_regs_caller_end+0x66/0x66
>> kernel:  worker_thread+0x48/0x3a0
>> kernel:  ? __pfx_worker_thread+0x10/0x10
>> kernel:  kthread+0x10f/0x140
>> kernel:  ? __pfx_kthread+0x10/0x10
>> kernel:  ret_from_fork+0x29/0x50
>> kernel:  </TASK>
>>
>> Fixes: 4444bc2116ae ("wifi: mac80211: Proper mark iTXQs for resumption")
>> Link: https://lore.kernel.org/all/1f58a0d1-d2b9-d851-73c3-93fcc607501c@alu.unizg.hr/
>> Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
>> Cc: Gregory Greenman <gregory.greenman@intel.com>
>> Cc: Johannes Berg <johannes.berg@intel.com>
>> Link: https://lore.kernel.org/all/cdc80531-f25f-6f9d-b15f-25e16130b53a@alu.unizg.hr/
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Leon Romanovsky <leon@kernel.org>
>> Cc: Alexander Wetzel <alexander@wetzel-home.de>
>> Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
>> ---
>> v3 -> v4:
>> - Added whole lockdep trace as advised.
>> - Trimmed irrelevant line prefix.
>> v2 -> v3:
>> - Fix the Fixes: tag as advised.
>> - Change the net: to wifi: to comply with the original patch that
>>    is being fixed.
>> v1 -> v2:
>> - Minor rewording and clarification.
>> - Cc:-ed people that replied to the original bug report (forgotten
>>    in v1 by omission).
>>
>>   net/mac80211/tx.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Not at all.

That's awesome! Just to ask, do I need to send the PATCH v5 with the
Reviewed-by: tag, or it goes automatically?

Thanks.

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu

System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia

"What’s this thing suddenly coming towards me very fast? Very very fast.
... I wonder if it will be friends with me?"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt
  2023-04-26 14:02   ` Mirsad Todorovac
@ 2023-04-26 15:05     ` Johannes Berg
  2023-04-26 17:52       ` Mirsad Goran Todorovac
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2023-04-26 15:05 UTC (permalink / raw
  To: Mirsad Todorovac, Leon Romanovsky
  Cc: linux-wireless, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Gregory Greenman,
	Alexander Wetzel

On Wed, 2023-04-26 at 16:02 +0200, Mirsad Todorovac wrote:
> 
> That's awesome! Just to ask, do I need to send the PATCH v5 with the
> Reviewed-by: tag, or it goes automatically?
> 

Patchwork will be pick it up automatically.

johannes

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt
  2023-04-26 15:05     ` Johannes Berg
@ 2023-04-26 17:52       ` Mirsad Goran Todorovac
  0 siblings, 0 replies; 5+ messages in thread
From: Mirsad Goran Todorovac @ 2023-04-26 17:52 UTC (permalink / raw
  To: Johannes Berg, Leon Romanovsky
  Cc: linux-wireless, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Gregory Greenman,
	Alexander Wetzel

On 26. 04. 2023. 17:05, Johannes Berg wrote:
> On Wed, 2023-04-26 at 16:02 +0200, Mirsad Todorovac wrote:
>>
>> That's awesome! Just to ask, do I need to send the PATCH v5 with the
>> Reviewed-by: tag, or it goes automatically?
>>
> 
> Patchwork will be pick it up automatically.
> 
> johannes

That's awesome, thank you for the update.

Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-26 17:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-25 16:40 [PATCH v4 1/1] wifi: mac80211: fortify the spinlock against deadlock by interrupt Mirsad Goran Todorovac
2023-04-26  6:41 ` Leon Romanovsky
2023-04-26 14:02   ` Mirsad Todorovac
2023-04-26 15:05     ` Johannes Berg
2023-04-26 17:52       ` Mirsad Goran Todorovac

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.