Thread (5 messages) 5 messages, 3 authors, 2021-04-02

Re: [PATCH 2/2] nvmet-tcp: Fix incorrect locking in state_change sk callback

From: Sagi Grimberg <sagi@grimberg.me>
Date: 2021-03-25 22:40:21

Hi Sagi
With the two patch, I reproduced another lock dependency issue, here is 
Hey Yi,

This one is different, still the fixes for the other one are valid...

Will look into this one too...
the full log:

[  143.310362] run blktests nvme/003 at 2021-03-23 21:52:15
[  143.927284] loop: module loaded
[  144.027532] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[  144.059070] nvmet_tcp: enabling port 0 (127.0.0.1:4420)
[  144.201559] nvmet: creating controller 1 for subsystem 
nqn.2014-08.org.nvmexpress.discovery for NQN 
nqn.2014-08.org.nvmexpress:uuid:e25db33098f14032b70b755db1976647.
[  144.211644] nvme nvme1: new ctrl: NQN 
"nqn.2014-08.org.nvmexpress.discovery", addr 127.0.0.1:4420
[  154.400575] nvme nvme1: Removing ctrl: NQN 
"nqn.2014-08.org.nvmexpress.discovery"

[  154.407970] ======================================================
[  154.414871] WARNING: possible circular locking dependency detected
[  154.421765] 5.12.0-rc3.fix+ #2 Not tainted
[  154.426340] ------------------------------------------------------
[  154.433232] kworker/7:2/260 is trying to acquire lock:
[  154.438972] ffff888288e92030 
((work_completion)(&queue->io_work)){+.+.}-{0:0}, at: 
__flush_work+0x118/0x1a0
[  154.449882]
                but task is already holding lock:
[  154.456395] ffffc90002b57db0 
((work_completion)(&queue->release_work)){+.+.}-{0:0}, at: 
process_one_work+0x7c1/0x1480
[  154.468263]
                which lock already depends on the new lock.

[  154.477393]
                the existing dependency chain (in reverse order) is:
[  154.485739]
                -> #2 
((work_completion)(&queue->release_work)){+.+.}-{0:0}:
[  154.494884]        __lock_acquire+0xb77/0x18d0
[  154.499853]        lock_acquire+0x1ca/0x480
[  154.504528]        process_one_work+0x813/0x1480
[  154.509688]        worker_thread+0x590/0xf80
[  154.514458]        kthread+0x368/0x440
[  154.518650]        ret_from_fork+0x22/0x30
[  154.523232]
                -> #1 ((wq_completion)events){+.+.}-{0:0}:
[  154.530633]        __lock_acquire+0xb77/0x18d0
[  154.535597]        lock_acquire+0x1ca/0x480
[  154.540272]        flush_workqueue+0x101/0x1250
[  154.545334]        nvmet_tcp_install_queue+0x22c/0x2a0 [nvmet_tcp]
[  154.552242]        nvmet_install_queue+0x2a3/0x360 [nvmet]
[  154.558387]        nvmet_execute_admin_connect+0x321/0x420 [nvmet]
[  154.565305]        nvmet_tcp_io_work+0xa04/0xcfb [nvmet_tcp]
[  154.571629]        process_one_work+0x8b2/0x1480
[  154.576787]        worker_thread+0x590/0xf80
[  154.581560]        kthread+0x368/0x440
[  154.585749]        ret_from_fork+0x22/0x30
[  154.590328]
                -> #0 ((work_completion)(&queue->io_work)){+.+.}-{0:0}:
[  154.598989]        check_prev_add+0x15e/0x20f0
[  154.603953]        validate_chain+0xec9/0x19c0
[  154.608918]        __lock_acquire+0xb77/0x18d0
[  154.613883]        lock_acquire+0x1ca/0x480
[  154.618556]        __flush_work+0x139/0x1a0
[  154.623229]        nvmet_tcp_release_queue_work+0x2e5/0xcb0 [nvmet_tcp]
[  154.630621]        process_one_work+0x8b2/0x1480
[  154.635780]        worker_thread+0x590/0xf80
[  154.640549]        kthread+0x368/0x440
[  154.644741]        ret_from_fork+0x22/0x30
[  154.649321]
                other info that might help us debug this:

[  154.658257] Chain exists of:
                  (work_completion)(&queue->io_work) --> 
(wq_completion)events --> (work_completion)(&queue->release_work)

[  154.675070]  Possible unsafe locking scenario:

[  154.681679]        CPU0                    CPU1
[  154.686728]        ----                    ----
[  154.691776] lock((work_completion)(&queue->release_work));
[  154.698102] lock((wq_completion)events);
[  154.705493] lock((work_completion)(&queue->release_work));
[  154.714631]   lock((work_completion)(&queue->io_work));
[  154.720470]
                 *** DEADLOCK ***

[  154.727080] 2 locks held by kworker/7:2/260:
[  154.731849]  #0: ffff888100053148 
((wq_completion)events){+.+.}-{0:0}, at: process_one_work+0x792/0x1480
[  154.742458]  #1: ffffc90002b57db0 
((work_completion)(&queue->release_work)){+.+.}-{0:0}, at: 
process_one_work+0x7c1/0x1480
[  154.754809]
                stack backtrace:
[  154.759674] CPU: 7 PID: 260 Comm: kworker/7:2 Not tainted 
5.12.0-rc3.fix+ #2
[  154.767549] Hardware name: Dell Inc. PowerEdge 
R730xd/\xc9\xb2\xdePow, BIOS 2.12.1 12/04/2020
[  154.776197] Workqueue: events nvmet_tcp_release_queue_work [nvmet_tcp]
[  154.783497] Call Trace:
[  154.786231]  dump_stack+0x93/0xc2
[  154.789942]  check_noncircular+0x26a/0x310
[  154.794521]  ? print_circular_bug+0x460/0x460
[  154.799391]  ? deref_stack_reg+0x170/0x170
[  154.803967]  ? alloc_chain_hlocks+0x1de/0x520
[  154.808843]  check_prev_add+0x15e/0x20f0
[  154.813231]  validate_chain+0xec9/0x19c0
[  154.817611]  ? check_prev_add+0x20f0/0x20f0
[  154.822286]  ? save_trace+0x88/0x5e0
[  154.826290]  __lock_acquire+0xb77/0x18d0
[  154.830682]  lock_acquire+0x1ca/0x480
[  154.834775]  ? __flush_work+0x118/0x1a0
[  154.839066]  ? rcu_read_unlock+0x40/0x40
[  154.843455]  ? __lock_acquire+0xb77/0x18d0
[  154.848036]  __flush_work+0x139/0x1a0
[  154.852120]  ? __flush_work+0x118/0x1a0
[  154.856409]  ? start_flush_work+0x810/0x810
[  154.861084]  ? mark_lock+0xd3/0x1470
[  154.865082]  ? mark_lock_irq+0x1d10/0x1d10
[  154.869662]  ? lock_downgrade+0x100/0x100
[  154.874147]  ? mark_held_locks+0xa5/0xe0
[  154.878522]  ? sk_stream_wait_memory+0xe40/0xe40
[  154.883686]  ? lockdep_hardirqs_on_prepare.part.0+0x198/0x340
[  154.890394]  ? __local_bh_enable_ip+0xa2/0x100
[  154.895358]  ? trace_hardirqs_on+0x1c/0x160
[  154.900034]  ? sk_stream_wait_memory+0xe40/0xe40
[  154.905192]  nvmet_tcp_release_queue_work+0x2e5/0xcb0 [nvmet_tcp]
[  154.911999]  ? lock_is_held_type+0x9a/0x110
[  154.916676]  process_one_work+0x8b2/0x1480
[  154.921255]  ? pwq_dec_nr_in_flight+0x260/0x260
[  154.926315]  ? __lock_contended+0x910/0x910
[  154.930990]  ? worker_thread+0x150/0xf80
[  154.935374]  worker_thread+0x590/0xf80
[  154.939564]  ? __kthread_parkme+0xcb/0x1b0
[  154.944140]  ? process_one_work+0x1480/0x1480
[  154.949007]  kthread+0x368/0x440
[  154.952615]  ? _raw_spin_unlock_irq+0x24/0x30
[  154.957482]  ? __kthread_bind_mask+0x90/0x90
[  154.962255]  ret_from_fork+0x22/0x30


On 3/21/21 3:08 PM, Sagi Grimberg wrote:
quoted
We are not changing anything in the TCP connection state so
we should not take a write_lock but rather a read lock.

This caused a deadlock when running nvmet-tcp and nvme-tcp
on the same system, where state_change callbacks on the
host and on the controller side have causal relationship
and made lockdep report on this with blktests:
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help