RE: [PATCH rtw v4 4/4] wifi: rtw89: avoid circular locking dependency in ser_state_run()

[Ping-Ke Shih <pkshih@realtek.com>]

Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> Lockdep gives a splat [1] when ser_hdl_work item is executed.  It is
> scheduled at mac80211 workqueue via ieee80211_queue_work() and takes a
> wiphy lock inside.  However, this workqueue can be flushed when e.g.
> closing the interface and wiphy lock is already taken in that case.
> 
> Choosing wiphy_work_queue() for SER is likely not suitable.  Back on to
> the global workqueue.
> 
> [1]:
> 
>  WARNING: possible circular locking dependency detected
>  6.17.0-rc2 #17 Not tainted
>  ------------------------------------------------------
>  kworker/u32:1/61 is trying to acquire lock:
>  ffff88811bc00768 (&rdev->wiphy.mtx){+.+.}-{4:4}, at: ser_state_run+0x5e/0x180 [rtw89_core]
> 
>  but task is already holding lock:
>  ffffc9000048fd30 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}, at:
> process_one_work+0x7b5/0x1450
> 
>  which lock already depends on the new lock.
> 
>  the existing dependency chain (in reverse order) is:
> 
>  -> #2 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}:
>         process_one_work+0x7c6/0x1450
>         worker_thread+0x49e/0xd00
>         kthread+0x313/0x640
>         ret_from_fork+0x221/0x300
>         ret_from_fork_asm+0x1a/0x30
> 
>  -> #1 ((wq_completion)phy0){+.+.}-{0:0}:
>         touch_wq_lockdep_map+0x8e/0x180
>         __flush_workqueue+0x129/0x10d0
>         ieee80211_stop_device+0xa8/0x110
>         ieee80211_do_stop+0x14ce/0x2880
>         ieee80211_stop+0x13a/0x2c0
>         __dev_close_many+0x18f/0x510
>         __dev_change_flags+0x25f/0x670
>         netif_change_flags+0x7b/0x160
>         do_setlink.isra.0+0x1640/0x35d0
>         rtnl_newlink+0xd8c/0x1d30
>         rtnetlink_rcv_msg+0x700/0xb80
>         netlink_rcv_skb+0x11d/0x350
>         netlink_unicast+0x49a/0x7a0
>         netlink_sendmsg+0x759/0xc20
>         ____sys_sendmsg+0x812/0xa00
>         ___sys_sendmsg+0xf7/0x180
>         __sys_sendmsg+0x11f/0x1b0
>         do_syscall_64+0xbb/0x360
>         entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
>  -> #0 (&rdev->wiphy.mtx){+.+.}-{4:4}:
>         __lock_acquire+0x124c/0x1d20
>         lock_acquire+0x154/0x2e0
>         __mutex_lock+0x17b/0x12f0
>         ser_state_run+0x5e/0x180 [rtw89_core]
>         rtw89_ser_hdl_work+0x119/0x220 [rtw89_core]
>         process_one_work+0x82d/0x1450
>         worker_thread+0x49e/0xd00
>         kthread+0x313/0x640
>         ret_from_fork+0x221/0x300
>         ret_from_fork_asm+0x1a/0x30
> 
>  other info that might help us debug this:
> 
>  Chain exists of:
>    &rdev->wiphy.mtx --> (wq_completion)phy0 --> (work_completion)(&ser->ser_hdl_work)
> 
>   Possible unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock((work_completion)(&ser->ser_hdl_work));
>                                 lock((wq_completion)phy0);
>                                 lock((work_completion)(&ser->ser_hdl_work));
>    lock(&rdev->wiphy.mtx);
> 
>   *** DEADLOCK ***
> 
>  2 locks held by kworker/u32:1/61:
>   #0: ffff888103835148 ((wq_completion)phy0){+.+.}-{0:0}, at: process_one_work+0xefa/0x1450
>   #1: ffffc9000048fd30 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}, at:
> process_one_work+0x7b5/0x1450
> 
>  stack backtrace:
>  CPU: 0 UID: 0 PID: 61 Comm: kworker/u32:1 Not tainted 6.17.0-rc2 #17 PREEMPT(voluntary)
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS edk2-20250523-14.fc42 05/23/2025
>  Workqueue: phy0 rtw89_ser_hdl_work [rtw89_core]
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x5d/0x80
>   print_circular_bug.cold+0x178/0x1be
>   check_noncircular+0x14c/0x170
>   __lock_acquire+0x124c/0x1d20
>   lock_acquire+0x154/0x2e0
>   __mutex_lock+0x17b/0x12f0
>   ser_state_run+0x5e/0x180 [rtw89_core]
>   rtw89_ser_hdl_work+0x119/0x220 [rtw89_core]
>   process_one_work+0x82d/0x1450
>   worker_thread+0x49e/0xd00
>   kthread+0x313/0x640
>   ret_from_fork+0x221/0x300
>   ret_from_fork_asm+0x1a/0x30
>   </TASK>
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Fixes: ebfc9199df05 ("wifi: rtw89: add wiphy_lock() to work that isn't held wiphy_lock() yet")
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

By the way, you mark this patchset as 'rtw'. Does it mean this patchset is
urgent to you? If not, it will be more smooth (avoid possible merge conflict)
if it goes via 'rtw-next'. Let me know your preference.