drivers/nvme/host/core.c | 1 + drivers/nvme/host/rdma.c | 41 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)
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.
Close the window at both ends, each with state guarded by the lock that
already covers the list it belongs to:
- nvme_rdma_device gains ->dying, set and tested under
device_list_mutex. nvme_rdma_find_get_device() refuses a device that
is going away, so no new queues, PD or QP are created on it once
nvme_rdma_remove_one() has started. A re-probed HCA with the same node
GUID is a distinct ib_device and gets a fresh nvme_rdma_device.
- nvme_rdma_remove_one() records the ib_device on nvme_rdma_removing_list
in the same nvme_rdma_ctrl_mutex section that walks
nvme_rdma_ctrl_list, and nvme_rdma_create_ctrl() tests that list under
the same mutex immediately before publishing. A connect that took the
mutex first is on the list and is deleted by the walk; one that takes
it afterwards sees the entry and deletes its own controller. No
controller can be added behind the walk, so a single sweep suffices.
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@purestorage.com>
---
Changes since v1:
https://lore.kernel.org/all/20260806211822.317074-1-cachen@purestorage.com/
- ->dying is no longer read under nvme_rdma_ctrl_mutex (Leon). It stays
guarded by device_list_mutex and is only tested in
nvme_rdma_find_get_device(), which already holds that mutex. The test at
the publish point now uses a separate nvme_rdma_removing_list guarded by
nvme_rdma_ctrl_mutex, so each piece of state is accessed only under the
lock that declares it and the READ_ONCE()/WRITE_ONCE() pair is gone. The
two mutexes are still never held at the same time.
- Drop the code comments; the reasoning lives in the commit message (Sagi).
Leon, on moving the dying device off device_list onto a local removal list
instead of keeping ->dying: that does remove the flag, but it also lets a
connect racing the removal allocate a fresh nvme_rdma_device and call
ib_alloc_pd() on the device being unregistered, since
nvme_rdma_find_get_device() would no longer see anything to refuse. That
exposure exists upstream today, so unlinking is not a regression, but
keeping ->dying closes it as well, which seemed worth the one bool now that
it is no longer read across locks. A local (on-stack) removal list also
needs care: nvme_rdma_free_dev() does list_del() whenever the last kref
drops, and a connect holding a reference can outlive nvme_rdma_remove_one(),
so the list head would have to be static rather than on the stack. Happy to
switch to unlinking if you prefer it.
Sagi, on moving the test into nvme_rdma_setup_ctrl() before
nvme_start_ctrl(): that would let the existing destroy_io path do the
unwind, which is nicer, but I do not think it closes the race on its own.
setup_ctrl() returns before nvme_rdma_create_ctrl() takes
nvme_rdma_ctrl_mutex and calls list_add_tail(), so a removal landing in that
gap still walks nvme_rdma_ctrl_list before the controller is published. The
test has to be atomic with the publish, which is why it stayed under the
mutex. Happy to be told I am missing something.
One window is knowingly left open. Once nvme_rdma_remove_one() has returned,
the removal entry is gone and the nvme_rdma_device it marked has usually
been freed by the last nvme_rdma_dev_put(), taking ->dying with it. A
connect arriving between that point and cma_remove_one() unlinking the
cma_device allocates a fresh nvme_rdma_device and can still publish a
controller that nothing will delete. ib_clients are removed LIFO, so that
gap spans every remaining client's remove callback. It exists upstream today
and is strictly narrower with this patch, so I did not try to cover it here.
Closing it needs a test keyed on ib_core state rather than nvme_rdma's, for
example an ->add callback storing a token so that
ib_get_client_data(cm_id->device, &nvme_rdma_ib_client) == NULL identifies
"our remove callback has already returned". That reads client_data outside
what its kernel-doc permits, so it seemed better kept as a separate patch.
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/rdma.c | 41 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 42 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..63453d902619 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -45,6 +45,24 @@ 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 LIST_HEAD_GUARDED(nvme_rdma_removing_list, nvme_rdma_ctrl_mutex);
+
+struct nvme_rdma_removing_device {
+ struct list_head entry
+ __guarded_by(&nvme_rdma_ctrl_mutex);
+ struct ib_device *dev;
+};
+
+static bool nvme_rdma_device_removing(struct ib_device *ib_device)
+ __must_hold(&nvme_rdma_ctrl_mutex)
+{
+ struct nvme_rdma_removing_device *removing;
+
+ list_for_each_entry(removing, &nvme_rdma_removing_list, entry)
+ if (removing->dev == ib_device)
+ return true;
+ return false;
+}
struct nvme_rdma_device {
struct ib_device *dev;
@@ -53,6 +71,8 @@ struct nvme_rdma_device {
struct list_head entry
__guarded_by(&device_list_mutex);
unsigned int num_inline_segments;
+ bool dying
+ __guarded_by(&device_list_mutex);
};
struct nvme_rdma_qe {
@@ -378,6 +398,11 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
mutex_lock(&device_list_mutex);
list_for_each_entry(ndev, &device_list, entry) {
+ if (ndev->dying) {
+ if (ndev->dev == cm_id->device)
+ goto out_err;
+ continue;
+ }
if (ndev->dev->node_guid == cm_id->device->node_guid &&
nvme_rdma_dev_get(ndev))
goto out_unlock;
@@ -2380,6 +2405,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);
@@ -2407,6 +2441,7 @@ static struct nvmf_transport_ops nvme_rdma_transport = {
static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
{
+ struct nvme_rdma_removing_device removing = { .dev = ib_device };
struct nvme_rdma_ctrl *ctrl;
struct nvme_rdma_device *ndev;
bool found = false;
@@ -2414,6 +2449,7 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
mutex_lock(&device_list_mutex);
list_for_each_entry(ndev, &device_list, entry) {
if (ndev->dev == ib_device) {
+ ndev->dying = true;
found = true;
break;
}
@@ -2425,6 +2461,7 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
/* Delete all controllers using this device */
mutex_lock(&nvme_rdma_ctrl_mutex);
+ list_add(&removing.entry, &nvme_rdma_removing_list);
list_for_each_entry(ctrl, &nvme_rdma_ctrl_list, list) {
if (ctrl->device->dev != ib_device)
continue;
@@ -2433,6 +2470,10 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
mutex_unlock(&nvme_rdma_ctrl_mutex);
flush_workqueue(nvme_delete_wq);
+
+ mutex_lock(&nvme_rdma_ctrl_mutex);
+ list_del(&removing.entry);
+ mutex_unlock(&nvme_rdma_ctrl_mutex);
}
static struct ib_client nvme_rdma_ib_client = {
base-commit: 9eabc91952f9821824ca0288a76b3aba57961c6b
--
2.34.1
On 29/08/2026 2:24, Casey Chen wrote:
> 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.
>
> Close the window at both ends, each with state guarded by the lock that
> already covers the list it belongs to:
>
> - nvme_rdma_device gains ->dying, set and tested under
> device_list_mutex. nvme_rdma_find_get_device() refuses a device that
> is going away, so no new queues, PD or QP are created on it once
> nvme_rdma_remove_one() has started. A re-probed HCA with the same node
> GUID is a distinct ib_device and gets a fresh nvme_rdma_device.
>
> - nvme_rdma_remove_one() records the ib_device on nvme_rdma_removing_list
> in the same nvme_rdma_ctrl_mutex section that walks
> nvme_rdma_ctrl_list, and nvme_rdma_create_ctrl() tests that list under
> the same mutex immediately before publishing. A connect that took the
> mutex first is on the list and is deleted by the walk; one that takes
> it afterwards sees the entry and deletes its own controller. No
> controller can be added behind the walk, so a single sweep suffices.
>
> 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@purestorage.com>
> ---
>
> Changes since v1:
> https://lore.kernel.org/all/20260806211822.317074-1-cachen@purestorage.com/
>
> - ->dying is no longer read under nvme_rdma_ctrl_mutex (Leon). It stays
> guarded by device_list_mutex and is only tested in
> nvme_rdma_find_get_device(), which already holds that mutex. The test at
> the publish point now uses a separate nvme_rdma_removing_list guarded by
> nvme_rdma_ctrl_mutex, so each piece of state is accessed only under the
> lock that declares it and the READ_ONCE()/WRITE_ONCE() pair is gone. The
> two mutexes are still never held at the same time.
> - Drop the code comments; the reasoning lives in the commit message (Sagi).
>
> Leon, on moving the dying device off device_list onto a local removal list
> instead of keeping ->dying: that does remove the flag, but it also lets a
> connect racing the removal allocate a fresh nvme_rdma_device and call
> ib_alloc_pd() on the device being unregistered, since
> nvme_rdma_find_get_device() would no longer see anything to refuse. That
> exposure exists upstream today, so unlinking is not a regression, but
> keeping ->dying closes it as well, which seemed worth the one bool now that
> it is no longer read across locks. A local (on-stack) removal list also
> needs care: nvme_rdma_free_dev() does list_del() whenever the last kref
> drops, and a connect holding a reference can outlive nvme_rdma_remove_one(),
> so the list head would have to be static rather than on the stack. Happy to
> switch to unlinking if you prefer it.
>
> Sagi, on moving the test into nvme_rdma_setup_ctrl() before
> nvme_start_ctrl(): that would let the existing destroy_io path do the
> unwind, which is nicer, but I do not think it closes the race on its own.
> setup_ctrl() returns before nvme_rdma_create_ctrl() takes
> nvme_rdma_ctrl_mutex and calls list_add_tail(), so a removal landing in that
> gap still walks nvme_rdma_ctrl_list before the controller is published. The
> test has to be atomic with the publish, which is why it stayed under the
> mutex. Happy to be told I am missing something.
>
> One window is knowingly left open. Once nvme_rdma_remove_one() has returned,
> the removal entry is gone and the nvme_rdma_device it marked has usually
> been freed by the last nvme_rdma_dev_put(), taking ->dying with it. A
> connect arriving between that point and cma_remove_one() unlinking the
> cma_device allocates a fresh nvme_rdma_device and can still publish a
> controller that nothing will delete. ib_clients are removed LIFO, so that
> gap spans every remaining client's remove callback. It exists upstream today
> and is strictly narrower with this patch, so I did not try to cover it here.
> Closing it needs a test keyed on ib_core state rather than nvme_rdma's, for
> example an ->add callback storing a token so that
> ib_get_client_data(cm_id->device, &nvme_rdma_ib_client) == NULL identifies
> "our remove callback has already returned". That reads client_data outside
> what its kernel-doc permits, so it seemed better kept as a separate patch.
> drivers/nvme/host/core.c | 1 +
> drivers/nvme/host/rdma.c | 41 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 42 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..63453d902619 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -45,6 +45,24 @@ 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 LIST_HEAD_GUARDED(nvme_rdma_removing_list, nvme_rdma_ctrl_mutex);
> +
> +struct nvme_rdma_removing_device {
> + struct list_head entry
> + __guarded_by(&nvme_rdma_ctrl_mutex);
> + struct ib_device *dev;
> +};
I don't understand why this is needed.
> +
> +static bool nvme_rdma_device_removing(struct ib_device *ib_device)
> + __must_hold(&nvme_rdma_ctrl_mutex)
> +{
> + struct nvme_rdma_removing_device *removing;
> +
> + list_for_each_entry(removing, &nvme_rdma_removing_list, entry)
> + if (removing->dev == ib_device)
> + return true;
> + return false;
> +}
>
> struct nvme_rdma_device {
> struct ib_device *dev;
> @@ -53,6 +71,8 @@ struct nvme_rdma_device {
> struct list_head entry
> __guarded_by(&device_list_mutex);
> unsigned int num_inline_segments;
> + bool dying
> + __guarded_by(&device_list_mutex);
> };
>
> struct nvme_rdma_qe {
> @@ -378,6 +398,11 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
>
> mutex_lock(&device_list_mutex);
> list_for_each_entry(ndev, &device_list, entry) {
> + if (ndev->dying) {
> + if (ndev->dev == cm_id->device)
> + goto out_err;
> + continue;
> + }
> if (ndev->dev->node_guid == cm_id->device->node_guid &&
> nvme_rdma_dev_get(ndev))
> goto out_unlock;
> @@ -2380,6 +2405,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);
> + }
Why not instead of this, simply do in nvme_rdma_setup_ctrl:
> list_add_tail(&ctrl->list, &nvme_rdma_ctrl_list);
> mutex_unlock(&nvme_rdma_ctrl_mutex);
>
> @@ -2407,6 +2441,7 @@ static struct nvmf_transport_ops nvme_rdma_transport = {
>
> static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
> {
> + struct nvme_rdma_removing_device removing = { .dev = ib_device };
> struct nvme_rdma_ctrl *ctrl;
> struct nvme_rdma_device *ndev;
> bool found = false;
> @@ -2414,6 +2449,7 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
> mutex_lock(&device_list_mutex);
> list_for_each_entry(ndev, &device_list, entry) {
> if (ndev->dev == ib_device) {
> + ndev->dying = true;
> found = true;
> break;
> }
> @@ -2425,6 +2461,7 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
>
> /* Delete all controllers using this device */
> mutex_lock(&nvme_rdma_ctrl_mutex);
> + list_add(&removing.entry, &nvme_rdma_removing_list);
> list_for_each_entry(ctrl, &nvme_rdma_ctrl_list, list) {
> if (ctrl->device->dev != ib_device)
> continue;
> @@ -2433,6 +2470,10 @@ static void nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
> mutex_unlock(&nvme_rdma_ctrl_mutex);
>
> flush_workqueue(nvme_delete_wq);
> +
> + mutex_lock(&nvme_rdma_ctrl_mutex);
> + list_del(&removing.entry);
> + mutex_unlock(&nvme_rdma_ctrl_mutex);
> }
>
> static struct ib_client nvme_rdma_ib_client = {
>
> base-commit: 9eabc91952f9821824ca0288a76b3aba57961c6b
On 31/08/2026 0:10, Sagi Grimberg wrote:
>
>
> On 29/08/2026 2:24, Casey Chen wrote:
>> 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.
>>
>> Close the window at both ends, each with state guarded by the lock that
>> already covers the list it belongs to:
>>
>> - nvme_rdma_device gains ->dying, set and tested under
>> device_list_mutex. nvme_rdma_find_get_device() refuses a device that
>> is going away, so no new queues, PD or QP are created on it once
>> nvme_rdma_remove_one() has started. A re-probed HCA with the same
>> node
>> GUID is a distinct ib_device and gets a fresh nvme_rdma_device.
>>
>> - nvme_rdma_remove_one() records the ib_device on
>> nvme_rdma_removing_list
>> in the same nvme_rdma_ctrl_mutex section that walks
>> nvme_rdma_ctrl_list, and nvme_rdma_create_ctrl() tests that list
>> under
>> the same mutex immediately before publishing. A connect that took
>> the
>> mutex first is on the list and is deleted by the walk; one that
>> takes
>> it afterwards sees the entry and deletes its own controller. No
>> controller can be added behind the walk, so a single sweep suffices.
>>
>> 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@purestorage.com>
>> ---
>>
>> Changes since v1:
>> https://lore.kernel.org/all/20260806211822.317074-1-cachen@purestorage.com/
>>
>>
>> - ->dying is no longer read under nvme_rdma_ctrl_mutex (Leon). It
>> stays
>> guarded by device_list_mutex and is only tested in
>> nvme_rdma_find_get_device(), which already holds that mutex. The
>> test at
>> the publish point now uses a separate nvme_rdma_removing_list
>> guarded by
>> nvme_rdma_ctrl_mutex, so each piece of state is accessed only
>> under the
>> lock that declares it and the READ_ONCE()/WRITE_ONCE() pair is
>> gone. The
>> two mutexes are still never held at the same time.
>> - Drop the code comments; the reasoning lives in the commit message
>> (Sagi).
>>
>> Leon, on moving the dying device off device_list onto a local removal
>> list
>> instead of keeping ->dying: that does remove the flag, but it also
>> lets a
>> connect racing the removal allocate a fresh nvme_rdma_device and call
>> ib_alloc_pd() on the device being unregistered, since
>> nvme_rdma_find_get_device() would no longer see anything to refuse. That
>> exposure exists upstream today, so unlinking is not a regression, but
>> keeping ->dying closes it as well, which seemed worth the one bool
>> now that
>> it is no longer read across locks. A local (on-stack) removal list also
>> needs care: nvme_rdma_free_dev() does list_del() whenever the last kref
>> drops, and a connect holding a reference can outlive
>> nvme_rdma_remove_one(),
>> so the list head would have to be static rather than on the stack.
>> Happy to
>> switch to unlinking if you prefer it.
>>
>> Sagi, on moving the test into nvme_rdma_setup_ctrl() before
>> nvme_start_ctrl(): that would let the existing destroy_io path do the
>> unwind, which is nicer, but I do not think it closes the race on its
>> own.
>> setup_ctrl() returns before nvme_rdma_create_ctrl() takes
>> nvme_rdma_ctrl_mutex and calls list_add_tail(), so a removal landing
>> in that
>> gap still walks nvme_rdma_ctrl_list before the controller is
>> published. The
>> test has to be atomic with the publish, which is why it stayed under the
>> mutex. Happy to be told I am missing something.
>>
>> One window is knowingly left open. Once nvme_rdma_remove_one() has
>> returned,
>> the removal entry is gone and the nvme_rdma_device it marked has usually
>> been freed by the last nvme_rdma_dev_put(), taking ->dying with it. A
>> connect arriving between that point and cma_remove_one() unlinking the
>> cma_device allocates a fresh nvme_rdma_device and can still publish a
>> controller that nothing will delete. ib_clients are removed LIFO, so
>> that
>> gap spans every remaining client's remove callback. It exists
>> upstream today
>> and is strictly narrower with this patch, so I did not try to cover
>> it here.
>> Closing it needs a test keyed on ib_core state rather than
>> nvme_rdma's, for
>> example an ->add callback storing a token so that
>> ib_get_client_data(cm_id->device, &nvme_rdma_ib_client) == NULL
>> identifies
>> "our remove callback has already returned". That reads client_data
>> outside
>> what its kernel-doc permits, so it seemed better kept as a separate
>> patch.
>> drivers/nvme/host/core.c | 1 +
>> drivers/nvme/host/rdma.c | 41 ++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 42 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..63453d902619 100644
>> --- a/drivers/nvme/host/rdma.c
>> +++ b/drivers/nvme/host/rdma.c
>> @@ -45,6 +45,24 @@ 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 LIST_HEAD_GUARDED(nvme_rdma_removing_list,
>> nvme_rdma_ctrl_mutex);
>> +
>> +struct nvme_rdma_removing_device {
>> + struct list_head entry
>> + __guarded_by(&nvme_rdma_ctrl_mutex);
>> + struct ib_device *dev;
>> +};
>
> I don't understand why this is needed.
>
>> +
>> +static bool nvme_rdma_device_removing(struct ib_device *ib_device)
>> + __must_hold(&nvme_rdma_ctrl_mutex)
>> +{
>> + struct nvme_rdma_removing_device *removing;
>> +
>> + list_for_each_entry(removing, &nvme_rdma_removing_list, entry)
>> + if (removing->dev == ib_device)
>> + return true;
>> + return false;
>> +}
>> struct nvme_rdma_device {
>> struct ib_device *dev;
>> @@ -53,6 +71,8 @@ struct nvme_rdma_device {
>> struct list_head entry
>> __guarded_by(&device_list_mutex);
>> unsigned int num_inline_segments;
>> + bool dying
>> + __guarded_by(&device_list_mutex);
This should probably be a flag.
>> };
>> struct nvme_rdma_qe {
>> @@ -378,6 +398,11 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
>> mutex_lock(&device_list_mutex);
>> list_for_each_entry(ndev, &device_list, entry) {
>> + if (ndev->dying) {
>> + if (ndev->dev == cm_id->device)
>> + goto out_err;
>> + continue;
>> + }
>> if (ndev->dev->node_guid == cm_id->device->node_guid &&
>> nvme_rdma_dev_get(ndev))
>> goto out_unlock;
>> @@ -2380,6 +2405,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);
>> + }
>
> Why not instead of this, simply do in nvme_rdma_setup_ctrl:
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index b606565c2d85..58dedb57214c 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1108,6 +1108,11 @@ static int nvme_rdma_setup_ctrl(struct
nvme_rdma_ctrl *ctrl, bool new)
goto destroy_io;
}
+ if (nvme_rdma_device_dying(ctrl->device->dev)) {
+ nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_DELETING);
+ goto destroy_io;
+ }
+
nvme_start_ctrl(&ctrl->ctrl);
return 0;
>
>> list_add_tail(&ctrl->list, &nvme_rdma_ctrl_list);
>> mutex_unlock(&nvme_rdma_ctrl_mutex);
>> @@ -2407,6 +2441,7 @@ static struct nvmf_transport_ops
>> nvme_rdma_transport = {
>> static void nvme_rdma_remove_one(struct ib_device *ib_device,
>> void *client_data)
>> {
>> + struct nvme_rdma_removing_device removing = { .dev = ib_device };
>> struct nvme_rdma_ctrl *ctrl;
>> struct nvme_rdma_device *ndev;
>> bool found = false;
>> @@ -2414,6 +2449,7 @@ static void nvme_rdma_remove_one(struct
>> ib_device *ib_device, void *client_data)
>> mutex_lock(&device_list_mutex);
>> list_for_each_entry(ndev, &device_list, entry) {
>> if (ndev->dev == ib_device) {
>> + ndev->dying = true;
>> found = true;
>> break;
>> }
I'm wandering why not remove it from the list here?
I cannot see why this additional list with a dedicated struct is needed?
>> @@ -2425,6 +2461,7 @@ static void nvme_rdma_remove_one(struct
>> ib_device *ib_device, void *client_data)
>> /* Delete all controllers using this device */
>> mutex_lock(&nvme_rdma_ctrl_mutex);
>> + list_add(&removing.entry, &nvme_rdma_removing_list);
>> list_for_each_entry(ctrl, &nvme_rdma_ctrl_list, list) {
>> if (ctrl->device->dev != ib_device)
>> continue;
>> @@ -2433,6 +2470,10 @@ static void nvme_rdma_remove_one(struct
>> ib_device *ib_device, void *client_data)
>> mutex_unlock(&nvme_rdma_ctrl_mutex);
>> flush_workqueue(nvme_delete_wq);
>> +
>> + mutex_lock(&nvme_rdma_ctrl_mutex);
>> + list_del(&removing.entry);
>> + mutex_unlock(&nvme_rdma_ctrl_mutex);
>> }
>> static struct ib_client nvme_rdma_ib_client = {
>>
>> base-commit: 9eabc91952f9821824ca0288a76b3aba57961c6b
>
On 31/08/2026 0:15, Sagi Grimberg wrote:
> Why not instead of this, simply do in nvme_rdma_setup_ctrl:
>
> + if (nvme_rdma_device_dying(ctrl->device->dev)) {
> + nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_DELETING);
> + goto destroy_io;
> + }
> +
> nvme_start_ctrl(&ctrl->ctrl);
> return 0;
I like that this reuses the existing error path, but I do not think it
closes the race. nvme_rdma_setup_ctrl() returns before
nvme_rdma_create_ctrl() takes nvme_rdma_ctrl_mutex and publishes, so the
test and the publish are not atomic with respect to the walk:
Thread A (connect) Thread B (nvme_rdma_remove_one)
---------------------------------- ----------------------------------
nvme_rdma_setup_ctrl()
admin + IO queues up, LIVE
device_dying() -> false
mark device
lock nvme_rdma_ctrl_mutex
walk nvme_rdma_ctrl_list (A absent)
unlock
flush_workqueue()
nvme_start_ctrl()
return 0
nvme_rdma_create_ctrl()
lock nvme_rdma_ctrl_mutex
list_add_tail(&ctrl->list, ...) <-- published after the walk
unlock
A's controller is now on nvme_rdma_ctrl_list, nvme_rdma_remove_one() has
already finished and will not run again for this device, so nothing ever
deletes it. Its rdma_cm_ids keep the cma_device reference and
cma_remove_one() waits on it forever - the hang this patch is fixing.
> I cannot see why this additional list with a dedicated struct is needed?
Only to give that test state it may legally read. It has to run under
nvme_rdma_ctrl_mutex, and ->dying is guarded by device_list_mutex -
reading it there is what Leon objected to in v1:
"The write to ->dying is protected by &device_list_mutex, whereas this
path relies on &nvme_rdma_ctrl_mutex."
So the list is simply "which ib_devices are being removed", keyed on the
ib_device and guarded by nvme_rdma_ctrl_mutex. To be clear, it is not
what Leon suggested - he proposed moving the ndev off device_list, which
removes ->dying from nvme_rdma_find_get_device() but not from the
publish path, and he said as much ("at least in this path").
The list is not the only way to get that. Alternatives, in increasing
order of how much I like them:
1. A second bool on nvme_rdma_device, guarded by nvme_rdma_ctrl_mutex
and set in the same section as the walk. Drops the list and the
struct. Needs nvme_rdma_remove_one() to hold nvme_rdma_dev_get()
across the callback so every racing connect tests the same ndev.
2. Read ->dying under device_list_mutex nested inside
nvme_rdma_ctrl_mutex. One flag, no list. I would rather not:
nvme_rdma_remove_one() already takes those two locks in the opposite
order (sequentially, not nested), so this plants an ABBA trap for
whoever tightens that function next.
3. Use client_data as a per-ib_device liveness token: an ->add that does
ib_set_client_data(ib_device, &nvme_rdma_ib_client, ib_device),
nvme_rdma_remove_one() clearing it first thing, and both
nvme_rdma_find_get_device() and nvme_rdma_create_ctrl() testing
ib_get_client_data() == NULL. No flag, no list, no struct, no pin,
nothing to reset on re-probe, and ->add cannot fail because the token
needs no allocation.
(3) also closes a window none of the others do, and which v2 leaves open
by its own admission: once nvme_rdma_remove_one() has returned, ->dying
is gone with the freed ndev, and a connect arriving before
cma_remove_one() unlinks the cma_device can still strand a controller.
remove_client_context() erases client_data after ->remove returns, so
the token stays NULL and that path is refused too.
The catch is that 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 (3) calls it
precisely to detect that state. It works - it is a bare xa_load() and
xa_erase() runs after ->remove - but it is outside what the API
documents. Leon, do you have an opinion on whether that is acceptable,
or whether ib_core should grow something explicit for it?
v3 follows doing (3). It is smaller than v2 (+29, no deletions) and
drops ->dying, the list and the struct, so it should address both of
your comments and Leon's. If the client_data use is not acceptable I
will respin as (1), which keeps everything inside nvme_rdma at the cost
of one bool and a kref held across the callback.
Two side effects of adding ->add that are worth naming:
- A device with !kverbs_provider never gets ->add at all
(add_client_context() returns early), so nvme_rdma now refuses it in
nvme_rdma_find_get_device() rather than failing later when the QP is
created. 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. It is self correcting on retry,
but it is a real transient at probe time.
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@purestorage.com>
---
Changes since v2:
https://lore.kernel.org/all/20260828232436.270184-1-cachen@purestorage.com/
- 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@purestorage.com/
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.1On 31/08/2026 16:13, Casey Chen wrote: > 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. That soak is done now, so the caveat above no longer applies. The test bounces both mlx5 interfaces carrying the NVMe-oF RDMA connections, one at a time, with IO running throughout and a userspace daemon reconnecting the controllers: ethtool -i <iface> | grep bus-info echo 1 | sudo tee /sys/bus/pci/devices/<bdf>/remove sleep 30 echo 1 | sudo tee /sys/bus/pci/rescan sleep 30 30 iterations over the two interfaces, so 60 remove/rescan cycles, with 60s of settle time between iterations - roughly 90 minutes. No hang with v3 applied. Every write to remove returned, the interface came back on the following rescan and the controllers reconnected. The ->add path is covered by this, since each rescan re-probes the HCA as a new ib_device and so goes through nvme_rdma_add_one(). For contrast, on the same setup without the fix the write to remove eventually never returns and the task is left in D state with the trace in the commit message. It is intermittent - most removals complete normally, and only one that lands while a connect is in flight strands a controller.
© 2016 - 2026 Red Hat, Inc.