[PATCH 0/2] nvme: add reference counting for transport modules

[PATCH 0/2] nvme: add reference counting for transport modules

[Nilay Shroff]

Hi,

This patchset adds reference counting for NVMe transport modules while
the corresponding multipath head node is open. This prevents the
underlying transport module from being unloaded while it is still in
use by the multipath head node. Unloading a transport module while it
is still in use can result in undefined behavior.

The second patch adds a Clang context annotation for the shared field
used to track the number of active head node openers, allowing the
Clang context analyzer to validate accesses to the field.

As usual, feedback and suggestions are welcome.

Thanks!

Nilay Shroff (2):
  nvme: keep transport module referenced while head node is open
  nvme: add context annotation for nvme_ns_head::nr_openers

 drivers/nvme/host/core.c      | 11 +++++++++--
 drivers/nvme/host/multipath.c | 37 +++++++++++++++++++++++++++++++++--
 drivers/nvme/host/nvme.h      | 33 +++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/2] nvme: keep transport module referenced while head node is open

[Nilay Shroff]

When a user opens an NVMe multipath head node, we take a reference to
the head node, but this does not prevent the transport module backing
its paths from being unloaded. This can result in the multipath head
remaining open while its underlying transport module is unloaded.

Fix this by taking a reference to the transport module for each active
path when the multipath head node is opened. Keep track of the number
of active head node openers so that dynamically added paths acquire the
same number of transport module references.

Similarly, when a path is removed while the head node is open, release
the transport module reference once for each active head node opener.
When the head node is closed, release the transport module reference
held for each remaining active path.

This ensures that a transport module cannot be unloaded while it is
still referenced by an open multipath head node.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/core.c      |  6 ++++++
 drivers/nvme/host/multipath.c | 37 +++++++++++++++++++++++++++++++++--
 drivers/nvme/host/nvme.h      | 32 ++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1322c678f4eb..fdf760c57e02 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4173,6 +4173,9 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 
 	list_add_tail_rcu(&ns->siblings, &head->list);
 	ns->head = head;
+	ret = nvme_module_get(ns, head->nr_openers);
+	if (ret)
+		goto out_err_module_get;
 	mutex_unlock(&ctrl->subsys->lock);
 
 #ifdef CONFIG_NVME_MULTIPATH
@@ -4181,6 +4184,8 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 #endif
 	return 0;
 
+out_err_module_get:
+	list_del_rcu(&ns->siblings);
 out_put_ns_head:
 	nvme_put_ns_head(head);
 out_unlock:
@@ -4370,6 +4375,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 			list_del_init(&ns->head->entry);
 		last_path = true;
 	}
+	nvme_module_put(ns, ns->head->nr_openers);
 	mutex_unlock(&ns->ctrl->subsys->lock);
 
 	/* guarantee not available in head->list */
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 75dbb58286a3..822cd0d23e1d 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -579,14 +579,47 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
 
 static int nvme_ns_head_open(struct gendisk *disk, blk_mode_t mode)
 {
-	if (!nvme_tryget_ns_head(disk->private_data))
+	struct nvme_ns_head *head = disk->private_data;
+	struct nvme_subsystem *subsys = head->subsys;
+	struct nvme_ns *ns;
+	int ret;
+
+	if (!nvme_tryget_ns_head(head))
 		return -ENXIO;
+
+	mutex_lock(&subsys->lock);
+	list_for_each_entry(ns, &head->list, siblings) {
+		ret = nvme_module_get(ns, 1);
+		if (ret)
+			goto out_unwind;
+	}
+	head->nr_openers++;
+	mutex_unlock(&subsys->lock);
+
 	return 0;
+
+out_unwind:
+	list_for_each_entry_continue_reverse(ns, &head->list, siblings)
+		nvme_module_put(ns, 1);
+	mutex_unlock(&subsys->lock);
+
+	nvme_put_ns_head(head);
+	return ret;
 }
 
 static void nvme_ns_head_release(struct gendisk *disk)
 {
-	nvme_put_ns_head(disk->private_data);
+	struct nvme_ns_head *head = disk->private_data;
+	struct nvme_subsystem *subsys = head->subsys;
+	struct nvme_ns *ns;
+
+	mutex_lock(&subsys->lock);
+	list_for_each_entry(ns, &head->list, siblings)
+		nvme_module_put(ns, 1);
+	head->nr_openers--;
+	mutex_unlock(&subsys->lock);
+
+	nvme_put_ns_head(head);
 }
 
 static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 75e5d5a8a77c..db08f4618f92 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -569,6 +569,8 @@ struct nvme_ns_head {
 
 	struct gendisk		*disk;
 
+	unsigned int		nr_openers;
+
 	u16			nr_plids;
 	u16			*plids;
 #ifdef CONFIG_NVME_MULTIPATH
@@ -1109,6 +1111,28 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
 		return true;
 	return false;
 }
+
+static inline int nvme_module_get(struct nvme_ns *ns, unsigned int count)
+{
+	unsigned int i;
+
+	for (i = 0; i < count; i++) {
+		if (!try_module_get(ns->ctrl->ops->module))
+			goto out_unwind;
+	}
+
+	return 0;
+out_unwind:
+	while (i--)
+		module_put(ns->ctrl->ops->module);
+	return -ENXIO;
+}
+
+static inline void nvme_module_put(struct nvme_ns *ns, unsigned int count)
+{
+	while (count--)
+		module_put(ns->ctrl->ops->module);
+}
 #else
 #define multipath false
 static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
@@ -1199,6 +1223,14 @@ static inline bool nvme_disk_is_ns_head(struct gendisk *disk)
 static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
 {
 	return false;
+}
+static inline int nvme_module_get(struct nvme_ns *ns, unsigned int count)
+{
+	return 0;
+}
+static inline void nvme_module_put(struct nvme_ns *ns, unsigned int count)
+{
+
 }
 #endif /* CONFIG_NVME_MULTIPATH */
 
-- 
2.53.0

[PATCH 2/2] nvme: add context annotation for nvme_ns_head::nr_openers

[Nilay Shroff]

nvme_ns_head::nr_openers is protected by nvme_subsystem::lock. Annotate
it with __guarded_by(&subsys->lock) so that Clang's context analyzer can
validate that all accesses to nvme_ns_head::nr_openers are protected by
nvme_subsystem::lock.

Clang's context analyzer cannot currently model the same lock when it
is accessed through different object paths. Explicitly add the
capability in nvme_init_ns_head() using
__assume_ctx_lock(&head->subsys->lock) to avoid a false positive, as
ctrl->subsys->lock and head->subsys->lock refer to the same lock.

Similarly, in nvme_ns_remove(), replace ns->ctrl->subsys->lock with
ns->head->subsys->lock so that the lock expression matches the
capability used by the __guarded_by() annotation and avoids a false
positive from the context analyzer.

Cc: Marco Elver <elver@google.com>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/core.c | 5 +++--
 drivers/nvme/host/nvme.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index fdf760c57e02..29e3f2a7b5dd 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4173,6 +4173,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 
 	list_add_tail_rcu(&ns->siblings, &head->list);
 	ns->head = head;
+	__assume_ctx_lock(&head->subsys->lock);
 	ret = nvme_module_get(ns, head->nr_openers);
 	if (ret)
 		goto out_err_module_get;
@@ -4368,7 +4369,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 	if (nvme_mpath_clear_current_path(ns))
 		synchronize_srcu(&ns->head->srcu);
 
-	mutex_lock(&ns->ctrl->subsys->lock);
+	mutex_lock(&ns->head->subsys->lock);
 	list_del_rcu(&ns->siblings);
 	if (list_empty(&ns->head->list)) {
 		if (!nvme_mpath_queue_if_no_path(ns->head))
@@ -4376,7 +4377,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 		last_path = true;
 	}
 	nvme_module_put(ns, ns->head->nr_openers);
-	mutex_unlock(&ns->ctrl->subsys->lock);
+	mutex_unlock(&ns->head->subsys->lock);
 
 	/* guarantee not available in head->list */
 	synchronize_srcu(&ns->head->srcu);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index db08f4618f92..d2b91140712c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -569,7 +569,8 @@ struct nvme_ns_head {
 
 	struct gendisk		*disk;
 
-	unsigned int		nr_openers;
+	unsigned int		nr_openers
+		__guarded_by(&subsys->lock);
 
 	u16			nr_plids;
 	u16			*plids;
-- 
2.53.0

Re: [PATCH 0/2] nvme: add reference counting for transport modules

[John Garry]

On 8/31/26 16:19, Nilay Shroff wrote:
> Hi,
> 
> This patchset adds reference counting for NVMe transport modules while
> the corresponding multipath head node is open. This prevents the
> underlying transport module from being unloaded while it is still in
> use by the multipath head node. Unloading a transport module while it
> is still in use can result in undefined behavior.
> 

What is the undefined behaviour specifically?

When the ctrl ops module ref counting was originally introduced, the 
commit message mentioned a crash which it solves.

So far for this problem we have seen a report that if we remove the 
module, the mounted FS will have IOs fail. The same can be experienced 
if the admin unbinds the device from the driver. However, I have not 
seen a mention of a crash, kernel data corruption, a hang, etc.


> The second patch adds a Clang context annotation for the shared field
> used to track the number of active head node openers, allowing the
> Clang context analyzer to validate accesses to the field.
> 
> As usual, feedback and suggestions are welcome.
> 
> Thanks!
> 
> Nilay Shroff (2):
>    nvme: keep transport module referenced while head node is open
>    nvme: add context annotation for nvme_ns_head::nr_openers
> 
>   drivers/nvme/host/core.c      | 11 +++++++++--
>   drivers/nvme/host/multipath.c | 37 +++++++++++++++++++++++++++++++++--
>   drivers/nvme/host/nvme.h      | 33 +++++++++++++++++++++++++++++++
>   3 files changed, 77 insertions(+), 4 deletions(-)
> 

Re: [PATCH 0/2] nvme: add reference counting for transport modules

[Nilay Shroff]

On 9/1/26 1:44 PM, John Garry wrote:
> On 8/31/26 16:19, Nilay Shroff wrote:
>> Hi,
>>
>> This patchset adds reference counting for NVMe transport modules while
>> the corresponding multipath head node is open. This prevents the
>> underlying transport module from being unloaded while it is still in
>> use by the multipath head node. Unloading a transport module while it
>> is still in use can result in undefined behavior.
>>
> 
> What is the undefined behaviour specifically?
> 
Yes, so far we have observed I/O errors when the transport module is
unloaded while a filesystem is mounted on the multipath NVMe disk. This
becomes particularly problematic if the root filesystem is on that
device: once the transport module is unloaded, I/O fails and we can no
longer run commands to reload the NVMe transport module. In our testing,
the only recovery option in that situation has been to power-cycle the
system.

> When the ctrl ops module ref counting was originally introduced, the commit message mentioned a crash which it solves.
> 
> So far for this problem we have seen a report that if we remove the module, the mounted FS will have IOs fail. The same can be experienced if the admin unbinds the device from the driver. However, I have not seen a mention of a crash, kernel data corruption, a hang, etc.
> 
Yes I have also not observed a kernel crash or data corruption.
Regarding unbinding, this is triggered through the PCI/device-model
unbind path. The driver unbind callback returns void so the NVMe driver
has no way to reject or otherwise prevent the unbind operation from the
driver side. The purpose of this patch is therefore to protect the transport
module from being unloaded while the multipath head is still open.

Thanks,
--Nilay

Re: [PATCH 0/2] nvme: add reference counting for transport modules

[John Garry]

> 
>> When the ctrl ops module ref counting was originally introduced, the 
>> commit message mentioned a crash which it solves.
>>
>> So far for this problem we have seen a report that if we remove the 
>> module, the mounted FS will have IOs fail. The same can be experienced 
>> if the admin unbinds the device from the driver. However, I have not 
>> seen a mention of a crash, kernel data corruption, a hang, etc.
>>
> Yes I have also not observed a kernel crash or data corruption.
> Regarding unbinding, this is triggered through the PCI/device-model
> unbind path. The driver unbind callback returns void so the NVMe driver
> has no way to reject or otherwise prevent the unbind operation from the
> driver side. 

There's suppress_bind_attrs, but I don't think that anyone using VFIO 
would be happy if that were set.

> The purpose of this patch is therefore to protect the 
> transport
> module from being unloaded while the multipath head is still open.

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Keith Busch]

On Mon, Aug 31, 2026 at 08:49:54PM +0530, Nilay Shroff wrote:
> +static inline int nvme_module_get(struct nvme_ns *ns, unsigned int count)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < count; i++) {
> +		if (!try_module_get(ns->ctrl->ops->module))
> +			goto out_unwind;
> +	}
> +
> +	return 0;
> +out_unwind:
> +	while (i--)
> +		module_put(ns->ctrl->ops->module);
> +	return -ENXIO;
> +}
> +
> +static inline void nvme_module_put(struct nvme_ns *ns, unsigned int count)
> +{
> +	while (count--)
> +		module_put(ns->ctrl->ops->module);
> +}

Thanks, looks correct to me.

I don't like the looping though. This could be done in a single
atomic_sub instead of multiple atomic_dec calls if the module api
provided something to get/put many references.

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Wen Xiong]

On 2026-08-31 10:19, Nilay Shroff wrote:
> When a user opens an NVMe multipath head node, we take a reference to
> the head node, but this does not prevent the transport module backing
> its paths from being unloaded. This can result in the multipath head
> remaining open while its underlying transport module is unloaded.
> 
> Fix this by taking a reference to the transport module for each active
> path when the multipath head node is opened. Keep track of the number
> of active head node openers so that dynamically added paths acquire the
> same number of transport module references.
> 
> Similarly, when a path is removed while the head node is open, release
> the transport module reference once for each active head node opener.
> When the head node is closed, release the transport module reference
> held for each remaining active path.
> 
> This ensures that a transport module cannot be unloaded while it is
> still referenced by an open multipath head node.
> 
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>

Hi Nilay,

Thank you for your time and effort in working on this issue. I really 
appreciate your help!

Reviewed-by: Wen Xiong <wenxiong@linux.ibm.com>

Thanks,
Wendy

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Nilay Shroff]

On 9/1/26 10:09 PM, Keith Busch wrote:
> On Mon, Aug 31, 2026 at 08:49:54PM +0530, Nilay Shroff wrote:
>> +static inline int nvme_module_get(struct nvme_ns *ns, unsigned int count)
>> +{
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < count; i++) {
>> +		if (!try_module_get(ns->ctrl->ops->module))
>> +			goto out_unwind;
>> +	}
>> +
>> +	return 0;
>> +out_unwind:
>> +	while (i--)
>> +		module_put(ns->ctrl->ops->module);
>> +	return -ENXIO;
>> +}
>> +
>> +static inline void nvme_module_put(struct nvme_ns *ns, unsigned int count)
>> +{
>> +	while (count--)
>> +		module_put(ns->ctrl->ops->module);
>> +}
> 
> Thanks, looks correct to me.
> 
> I don't like the looping though. This could be done in a single
> atomic_sub instead of multiple atomic_dec calls if the module api
> provided something to get/put many references.

Yeah I though of the same however unfortunately module APIs don't
provide something using which we could atomically get/put more than
one references.

Thanks,
--Nilay

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Christoph Hellwig]

On Mon, Aug 31, 2026 at 08:49:54PM +0530, Nilay Shroff wrote:
> When a user opens an NVMe multipath head node, we take a reference to
> the head node, but this does not prevent the transport module backing
> its paths from being unloaded. This can result in the multipath head
> remaining open while its underlying transport module is unloaded.

Which makes sense.  Different paths can use different transports, but
even when all paths go away, the head can stick around.

> Fix this by taking a reference to the transport module for each active
> path when the multipath head node is opened. Keep track of the number
> of active head node openers so that dynamically added paths acquire the
> same number of transport module references.

I'm not sure this is correct.  Unloading the layer below should be
just fine.

What practical problem do you want to solve with this?

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Nilay Shroff]

On 9/2/26 7:01 PM, Christoph Hellwig wrote:
> On Mon, Aug 31, 2026 at 08:49:54PM +0530, Nilay Shroff wrote:
>> When a user opens an NVMe multipath head node, we take a reference to
>> the head node, but this does not prevent the transport module backing
>> its paths from being unloaded. This can result in the multipath head
>> remaining open while its underlying transport module is unloaded.
> 
> Which makes sense.  Different paths can use different transports, but
> even when all paths go away, the head can stick around.
> 
>> Fix this by taking a reference to the transport module for each active
>> path when the multipath head node is opened. Keep track of the number
>> of active head node openers so that dynamically added paths acquire the
>> same number of transport module references.
> 
> I'm not sure this is correct.  Unloading the layer below should be
> just fine.
> 
> What practical problem do you want to solve with this?
> 

The problem we are trying to solve is that the multipath head node can
remain open and be used by a filesystem even though the transport module
backing its paths can be unloaded.

For example, we have a shared namespace exposed through two PCIe paths:

nvme-subsys0
     nvme0 -> nvme0n1
     nvme1 -> nvme0n1

The namespace is formatted with ext4 and mounted:

# mount -t ext4 /dev/nvme0n1 /mnt/disk

At this point, the multipath head has a reference to nvme_core, but the
nvme transport module itself has a refcount of zero:

# lsmod | grep nvme
nvme                  262144  0
nvme_core             458752  2
nvme_keyring          262144  1
nvme_auth             262144  1

Consequently, the transport module can be unloaded:

# rmmod nvme

After this, the filesystem remains mounted, but I/O starts failing:

     EXT4-fs (...): shut down requested (2)
     Aborting journal on device nvme0n1-8.
     block nvme0n1: no available path - failing I/O
     Buffer I/O error on dev nvme0n1, logical block ..., lost sync page write
     JBD2: I/O error when updating journal superblock for nvme0n1-8.

This becomes particularly problematic if the multipath NVMe device is
used as the root filesystem. Once the transport module is unloaded, I/O
fails and we can no longer run commands to reload the NVMe module. In
our testing, the only recovery option in that situation has been to
power-cycle the system.

So the practical problem is that an open multipath head can continue to
be used after its underlying transport module has been unloaded. The
proposed change keeps the transport module referenced while the head
node is open, preventing the module from being unloaded in this
scenario.

Thanks,
--Nilay

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Christoph Hellwig]

On Wed, Sep 02, 2026 at 07:39:32PM +0530, Nilay Shroff wrote:
> So the practical problem is that an open multipath head can continue to
> be used after its underlying transport module has been unloaded. The
> proposed change keeps the transport module referenced while the head
> node is open, preventing the module from being unloaded in this
> scenario.

But why do you unload it in the first place?

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Keith Busch]

On Wed, Sep 02, 2026 at 04:13:47PM +0200, Christoph Hellwig wrote:
> On Wed, Sep 02, 2026 at 07:39:32PM +0530, Nilay Shroff wrote:
> > So the practical problem is that an open multipath head can continue to
> > be used after its underlying transport module has been unloaded. The
> > proposed change keeps the transport module referenced while the head
> > node is open, preventing the module from being unloaded in this
> > scenario.
> 
> But why do you unload it in the first place?

I don't think there's a use case to do it on purpose. It'd probably be
by mistake or mischief. This seems like a reasonable way to remove that
foot gun. We already do this for non-multipath, too.

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Nilay Shroff]

On 9/2/26 7:43 PM, Christoph Hellwig wrote:
> On Wed, Sep 02, 2026 at 07:39:32PM +0530, Nilay Shroff wrote:
>> So the practical problem is that an open multipath head can continue to
>> be used after its underlying transport module has been unloaded. The
>> proposed change keeps the transport module referenced while the head
>> node is open, preventing the module from being unloaded in this
>> scenario.
> 
> But why do you unload it in the first place?
> 
Well, in my view, unloading a module while it is not in use should be a legitimate operation.

In the example shown earlier, the NVMe transport module is still being
used by the open multipath head. Shouldn't we explicitly reflect that by
incrementing its module refcount? We don't explicitly unload the module
as part of normal NVMe operation, but in theory I think the transport module
should remain referenced for as long as it is being used. That would also
prohibit user from unloading module by mistake.

Thanks,
--Nilay

Re: [PATCH 1/2] nvme: keep transport module referenced while head node is open

[Nilay Shroff]

Hi Christoph,

A gentle ping on this one... Do you have any further comments on this?

Thanks,
--Nilay

On 9/2/26 7:57 PM, Nilay Shroff wrote:
> On 9/2/26 7:43 PM, Christoph Hellwig wrote:
>> On Wed, Sep 02, 2026 at 07:39:32PM +0530, Nilay Shroff wrote:
>>> So the practical problem is that an open multipath head can continue to
>>> be used after its underlying transport module has been unloaded. The
>>> proposed change keeps the transport module referenced while the head
>>> node is open, preventing the module from being unloaded in this
>>> scenario.
>>
>> But why do you unload it in the first place?
>>
> Well, in my view, unloading a module while it is not in use should be a legitimate operation.
> 
> In the example shown earlier, the NVMe transport module is still being
> used by the open multipath head. Shouldn't we explicitly reflect that by
> incrementing its module refcount? We don't explicitly unload the module
> as part of normal NVMe operation, but in theory I think the transport module
> should remain referenced for as long as it is being used. That would also
> prohibit user from unloading module by mistake.
> 
> Thanks,
> --Nilay