[PATCH RFC 1/2] RDMA/cma: Add rdma_reclassify_id_for_listen()

[Shin'ichiro Kawasaki]

Since the commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue
allocation and startup"), the host sends commands on an I/O queue before
the target has seen RDMA_CM_EVENT_ESTABLISHED for that queue. nvmet-rdma
keeps such commands on queue->rsp_wait_list and executes them from
nvmet_rdma_cm_handler() when the ESTABLISHED event arrives, that is,
with the handler_mutex of the queue's cm_id held. Executing a connect
command takes nvmet_config_sem, which creates a handler_mutex ->
nvmet_config_sem dependency. This is the reverse of the order in
nvmet_port_subsys_drop_link(), which holds nvmet_config_sem and calls
rdma_destroy_id() on the port's listening cm_id via
nvmet_disable_port(). Lockdep reports the warning below when blktests
nvme/004 or nvme/031 is run repeatedly with the rdma transport and the
rxe driver:

    WARNING: possible circular locking dependency detected
    rm/7346 is trying to acquire lock:
     (&id_priv->handler_mutex){+.+.}-{4:4}, at: rdma_destroy_id+0x17/0x20 [rdma_cm]
    but task is already holding lock:
     (nvmet_config_sem){++++}-{4:4}, at: nvmet_port_subsys_drop_link+0x52/0x340 [nvmet]

    -> #1 (nvmet_config_sem){++++}-{4:4}:
           down_read+0xa6/0x530
           nvmet_find_get_subsys+0x1f4/0x630 [nvmet]
           nvmet_ctrl_find_get+0xaa/0x320 [nvmet]
           nvmet_execute_io_connect+0x1e0/0x480 [nvmet]
           nvmet_rdma_execute_command+0x18e/0x480 [nvmet_rdma]
           nvmet_rdma_handle_command+0x428/0xb30 [nvmet_rdma]
           nvmet_rdma_cm_handler+0x8a8/0x30f0 [nvmet_rdma]
           cma_cm_event_handler+0xb0/0x3c0 [rdma_cm]
           cma_ib_handler+0x28a/0x9b0 [rdma_cm]
           cm_process_work+0x4b/0x3e0 [ib_cm]
           cm_work_handler+0x2485/0xac10 [ib_cm]

    -> #0 (&id_priv->handler_mutex){+.+.}-{4:4}:
           rdma_destroy_id+0x17/0x20 [rdma_cm]
           nvmet_rdma_disable_port+0x44/0x300 [nvmet_rdma]
           nvmet_rdma_remove_port+0x42/0x60 [nvmet_rdma]
           nvmet_disable_port+0x120/0x1f0 [nvmet]
           nvmet_port_subsys_drop_link+0x261/0x340 [nvmet]
           configfs_unlink+0x36d/0x790
           vfs_unlink+0x278/0xb40
           filename_unlinkat+0x31d/0x670
           __x64_sys_unlinkat+0xae/0x100

    other info that might help us debug this:

     Possible unsafe locking scenario:

           CPU0                    CPU1
           ----                    ----
      lock(nvmet_config_sem);
                                   lock(&id_priv->handler_mutex);
                                   lock(nvmet_config_sem);
      lock(&id_priv->handler_mutex);

     *** DEADLOCK ***

However, the two handler_mutex instances belong to different cm_ids. The
one held during command execution belongs to a connected queue's cm_id,
while the one taken by rdma_destroy_id() belongs to the listening cm_id.
nvmet-rdma does not take nvmet_config_sem while handling events of the
listening cm_id, so this is not a real deadlock. Lockdep handles the two
in the same manner because all handler_mutex instances share a single
lock class.

Add rdma_reclassify_id_for_listen(), which assigns a separate lockdep
class to the handler_mutex of a listening cm_id. rdma users can call it
right after creating the cm_id they listen on. A follow-up patch makes
nvmet-rdma call it.

Link: https://lore.kernel.org/linux-nvme/at36um4zrsfOr3kh@shinmob/
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
---
 drivers/infiniband/core/cma.c | 13 +++++++++++++
 include/rdma/rdma_cm.h        |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 337a49d1acf7..436e2467a36f 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1064,6 +1064,19 @@ __rdma_create_kernel_id(struct net *net, rdma_cm_event_handler event_handler,
 }
 EXPORT_SYMBOL(__rdma_create_kernel_id);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key cma_listen_handler_mutex_key;
+void rdma_reclassify_id_for_listen(struct rdma_cm_id *id)
+{
+	struct rdma_id_private *id_priv;
+
+	id_priv = container_of(id, struct rdma_id_private, id);
+	lockdep_set_class(&id_priv->handler_mutex,
+			  &cma_listen_handler_mutex_key);
+}
+EXPORT_SYMBOL(rdma_reclassify_id_for_listen);
+#endif
+
 struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
 				       void *context,
 				       enum rdma_ucm_port_space ps,
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index d639ff889e64..59dda4b0173e 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -158,6 +158,12 @@ struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
 	__rdma_create_kernel_id(net, event_handler, context, ps, qp_type,      \
 				KBUILD_MODNAME)
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+void rdma_reclassify_id_for_listen(struct rdma_cm_id *id);
+#else
+static inline void rdma_reclassify_id_for_listen(struct rdma_cm_id *id) {}
+#endif
+
 /**
   * rdma_destroy_id - Destroys an RDMA identifier.
   *
-- 
2.56.0