[PATCH v3] nvme-rdma: fix ib_device removal race that hangs PCI unbind

[Casey Chen]

nvme_rdma_remove_one() samples nvme_rdma_ctrl_list once, then blocks in
flush_workqueue(nvme_delete_wq). A connect can publish a controller on
the same ib_device during that window: nvme_rdma_find_get_device()
matches on node GUID in nvme_rdma's private device_list and never
consults ib_core unregistration state. Such a controller is never
deleted, so its rdma_cm_ids keep a reference on the cma_device.

ib_clients are removed LIFO, so nvme_rdma_remove_one() runs before
cma_remove_one(), which then waits for that reference forever. Removing
the RDMA interface underneath live NVMe-oF connections:

  echo 1 | sudo tee /sys/bus/pci/devices/0000:2a:00.1/remove

wedges the unbind permanently:

  INFO: task tee:164872 blocked for more than 200 seconds.
  task:tee             state:D stack:0     pid:164872 ppid:164870 flags:0x00004002
  Call Trace:
   <TASK>
   __schedule+0x4b4/0xf90
   schedule+0x5a/0xc0
   schedule_timeout+0x105/0x110
   ? cma_process_remove+0x1f9/0x240 [rdma_cm]
   __wait_for_common+0xc7/0x1f0
   ? usleep_range_state+0xb0/0xb0
   cma_remove_one+0x50/0xb0 [rdma_cm]
   remove_client_context+0x88/0xc0 [ib_core]
   disable_device+0x8a/0x160 [ib_core]
   __ib_unregister_device+0x42/0xa0 [ib_core]
   ib_unregister_device+0x22/0x30 [ib_core]
   mlx5r_remove+0x39/0x60 [mlx5_ib]
   auxiliary_bus_remove+0x18/0x30
   device_release_driver_internal+0x18f/0x1f0
   bus_remove_device+0xbc/0x120
   device_del+0x154/0x3d0
   ? devl_param_driverinit_value_get+0x29/0x90
   mlx5_rescan_drivers_locked.part.0+0x78/0x1c0 [mlx5_core]
   mlx5_unregister_device+0x34/0x50 [mlx5_core]
   mlx5_uninit_one+0x45/0x110 [mlx5_core]
   remove_one+0x4e/0xc0 [mlx5_core]
   pci_device_remove+0x39/0xa0
   device_release_driver_internal+0x18f/0x1f0
   pci_stop_bus_device+0x68/0x90
   pci_stop_and_remove_bus_device_locked+0x28/0x40
   remove_store+0x75/0x90
   kernfs_fop_write_iter+0x147/0x1d0
   vfs_write+0x2af/0x410
   ksys_write+0x5f/0xe0
   do_syscall_64+0x35/0x80
   entry_SYSCALL_64_after_hwframe+0x4b/0xb5
   </TASK>

Because the unbind stalls mid-teardown the netdev is never unregistered,
so userspace keeps reconnecting over the interface and loses the race
again.

nvme_rdma has no ->add callback, so its client_data is unused. Use it as
a per-ib_device liveness token: ->add stores the ib_device itself,
nvme_rdma_remove_one() clears it before doing anything else, and ib_core
erases it once ->remove has returned. ib_get_client_data() returning
NULL therefore means "this device is being removed, or already has
been", which is exactly the span over which nvme_rdma must refuse it.
The token needs no allocation, so ->add cannot fail and leave a device
attached with no ->remove to follow.

Two sites test it:

 - nvme_rdma_find_get_device() refuses a device that is going away, so a
   connect starting after removal begins never builds a PD, CQ or QP on
   it, and no stale nvme_rdma_device is handed out by the node GUID
   match.

 - nvme_rdma_create_ctrl() tests it under nvme_rdma_ctrl_mutex
   immediately before publishing, which covers a connect that was
   already in flight. The token is cleared before
   nvme_rdma_remove_one() acquires that mutex, so the two orderings are
   exhaustive: a publisher that takes the mutex first is on the list and
   is deleted by the walk, and one that takes it afterwards observes
   NULL and deletes its own controller. No controller can be added
   behind the walk, so a single sweep suffices.

Keying on the ib_device rather than on nvme_rdma_device matters: the
lookup matches on node GUID while the removal matches on the ib_device
pointer, so the two are not one to one, and the token also outlives the
nvme_rdma_device, which is freed as soon as its last queue is torn down.

The controller is fully live at the point the connect is refused, so it
is torn down with nvme_delete_ctrl_sync(). ->list is still empty there,
leaving opts to nvmf_create_ctrl(), and nvme_init_ctrl() left two
references while nvme_delete_ctrl_sync() consumes only the one that
nvme_uninit_ctrl() drops, so the other is put explicitly.

Reproduced on a 6.6 based kernel by removing and rescanning the mlx5
interface carrying the NVMe-oF RDMA connections in a loop, with IO
running and a userspace daemon reconnecting the controllers throughout.
The hang is racy: most removals complete normally, and only one that
lands while a connect is in flight leaves the sysfs write stuck in D
state with the trace above. With this patch the loop ran clean: removals
complete and the controllers reconnect after the following PCI rescan.

Fixes: e87a911fed07 ("nvme-rdma: use ib_client API to detect device removal")
Signed-off-by: Casey Chen <cachen@xxxxxxxxxxxxxxx>
---

Changes since v2:
https://lore.kernel.org/all/20260828232436.270184-1-cachen@xxxxxxxxxxxxxxx/

 - Drop ->dying, nvme_rdma_removing_list and struct
   nvme_rdma_removing_device entirely (Sagi, Leon). The state is now a
   per-ib_device liveness token in client_data, which nvme_rdma was not
   using, so there is no new field, list or struct and nothing to reset
   on re-probe.
 - The test is keyed on the ib_device rather than on nvme_rdma_device.
   That also fixes something v2 got wrong: the lookup matches on node
   GUID while the removal matches on the ib_device pointer, so the two
   are not one to one, and a flag on nvme_rdma_device can be set on an
   ndev shared with a device that is not being removed.
 - This closes the window v2 documented as knowingly open. Once
   nvme_rdma_remove_one() returned, v2 had no state left, so a connect
   arriving before cma_remove_one() unlinks the cma_device could still
   strand a controller. remove_client_context() erases client_data after
   ->remove returns, so the token stays NULL and that path is refused.

Why the test cannot move to nvme_rdma_setup_ctrl(), and the alternatives
to this approach, are discussed in the reply to Sagi on v2:
https://lore.kernel.org/all/20260831220754.1714514-1-cachen@xxxxxxxxxxxxxxx/

Two side effects of adding ->add, neither of which I think is a problem
but both worth naming:

 - A device with !kverbs_provider never gets ->add
   (add_client_context() returns early), so nvme_rdma now refuses it in
   nvme_rdma_find_get_device() rather than failing later at QP creation.
   nvme_rdma cannot use such a device either way.

 - Clients are added FIFO and rdma_cm registers before nvme_rdma, so
   during ib_register_device() cma_add_one() runs before
   nvme_rdma_add_one(). A connect resolving in that gap is refused with
   -ECONNREFUSED until ->add has run. Self correcting on retry, but a
   real transient at probe time.

Note ib_get_client_data()'s kernel-doc says it "can only be called while
the client is registered to the device, once the ib_client remove()
callback returns this cannot be called", and this uses it precisely to
detect that state. It works, since it is a bare xa_load() and xa_erase()
runs after ->remove, but it is outside what the API documents. Leon, if
you would rather ib_core grew something explicit for this, or if you
prefer the flag based variant, say so and I will respin.

The ->add path is new in this version and has not run on the setup that
reproduced the hang, so this wants a fresh soak before it is applied.
 drivers/nvme/host/core.c |  1 +
 drivers/nvme/host/rdma.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 758245c799a1..bede16fe1ff5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -282,6 +282,7 @@ void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl)
 		nvme_do_delete_ctrl(ctrl);
 	nvme_put_ctrl(ctrl);
 }
+EXPORT_SYMBOL_GPL(nvme_delete_ctrl_sync);
 
 static blk_status_t nvme_error_status(u16 status)
 {
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 29ecbe71bb2e..04687fe22804 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -46,6 +46,19 @@ static LIST_HEAD_GUARDED(device_list, device_list_mutex);
 static DEFINE_MUTEX(nvme_rdma_ctrl_mutex);
 static LIST_HEAD_GUARDED(nvme_rdma_ctrl_list, nvme_rdma_ctrl_mutex);
 
+static struct ib_client nvme_rdma_ib_client;
+
+static int nvme_rdma_add_one(struct ib_device *ib_device)
+{
+	ib_set_client_data(ib_device, &nvme_rdma_ib_client, ib_device);
+	return 0;
+}
+
+static bool nvme_rdma_device_removing(struct ib_device *ib_device)
+{
+	return !ib_get_client_data(ib_device, &nvme_rdma_ib_client);
+}
+
 struct nvme_rdma_device {
 	struct ib_device	*dev;
 	struct ib_pd		*pd;
@@ -377,6 +390,9 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
 	struct nvme_rdma_device *ndev;
 
 	mutex_lock(&device_list_mutex);
+	if (nvme_rdma_device_removing(cm_id->device))
+		goto out_err;
+
 	list_for_each_entry(ndev, &device_list, entry) {
 		if (ndev->dev->node_guid == cm_id->device->node_guid &&
 		    nvme_rdma_dev_get(ndev))
@@ -2380,6 +2396,15 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 		nvmf_ctrl_subsysnqn(&ctrl->ctrl), &ctrl->addr, opts->host->nqn);
 
 	mutex_lock(&nvme_rdma_ctrl_mutex);
+	if (nvme_rdma_device_removing(ctrl->device->dev)) {
+		mutex_unlock(&nvme_rdma_ctrl_mutex);
+		dev_info(ctrl->ctrl.device,
+			 "hca %s is being removed, aborting connect\n",
+			 dev_name(ctrl->device->dev->dma_device));
+		nvme_delete_ctrl_sync(&ctrl->ctrl);
+		nvme_put_ctrl(&ctrl->ctrl);
+		return ERR_PTR(-ECONNREFUSED);
+	}
 	list_add_tail(&ctrl->list, &nvme_rdma_ctrl_list);
 	mutex_unlock(&nvme_rdma_ctrl_mutex);
 
@@ -2411,6 +2436,8 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
 	struct nvme_rdma_device *ndev;
 	bool found = false;
 
+	ib_set_client_data(ib_device, &nvme_rdma_ib_client, NULL);
+
 	mutex_lock(&device_list_mutex);
 	list_for_each_entry(ndev, &device_list, entry) {
 		if (ndev->dev == ib_device) {
@@ -2437,6 +2464,7 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
 
 static struct ib_client nvme_rdma_ib_client = {
 	.name   = "nvme_rdma",
+	.add    = nvme_rdma_add_one,
 	.remove = nvme_rdma_remove_one
 };
 

base-commit: 9eabc91952f9821824ca0288a76b3aba57961c6b
-- 
2.34.1