[PATCH 00/15] Support Clang context analysis for NVMe host drivers

[PATCH 00/15] Support Clang context analysis for NVMe host drivers

[Nilay Shroff]

This series adds support for Clang's context analysis to the NVMe host
drivers.

Recent work[1] by Marco Elver introduced infrastructure for lock context
analysis in the kernel, allowing Clang to verify locking requirements at
compile time through various annotations.

This series build on top of that infrastructure by annotating the NVMe
host drivers with the appropriate locking requirements and by addressing
warnings reported by the analyzer. 

The series was built and tested with LLVM/Clang 23.x and enables the
NVMe host drivers to build cleanly with CONFIG_CONTEXT_ANALYSIS enabled.

Support for the NVMe target drivers will be addressed separately.

As usual, comments/feedback/suggestions are most welcome!

Thanks!

[1] https://lore.kernel.org/lkml/20251219154418.3592607-1-elver@google.com/

Nilay Shroff (15):
  nvme: update nvme_passthru_end() signature
  nvme: add Clang context annotations for nvme_passthru_{start|stop}
  nvme: add Clang context annotations for nvme_ns_head::srcu
  nvme: add Clang context annotations for nvme_ns_head::requeue_list
  nvme: add Clang context annotations for nvme_ns_head::current_path
  nvme: add Clang context annotations for nvme_dev::shutdown_lock
  nvme: add Clang context annotations for nvme_subsystem::lock
  nvme: add Clang context annotations for nvme_ctrl::ana_lock
  nvme: add Clang context annotations for nvme_subsystems_lock
  nvme: add Clang context annotations in fabric.c
  nvme: add Clang context annotations for nvme_queue::sq_lock
  nvme: add Clang context annotations for nvme_queue::cq_poll_lock
  nvme: add Clang context annotations in rdma.c
  nvme: add Clang context annotations in tcp.c
  nvme: enable Clang context analysis support for nvme host driver

 drivers/nvme/host/Makefile    |  1 +
 drivers/nvme/host/core.c      | 23 ++++++++++++++++-----
 drivers/nvme/host/fabrics.c   |  4 ++--
 drivers/nvme/host/ioctl.c     |  2 +-
 drivers/nvme/host/multipath.c | 26 +++++++++++++++++++++---
 drivers/nvme/host/nvme.h      | 38 ++++++++++++++++++++++-------------
 drivers/nvme/host/pci.c       | 35 ++++++++++++++++++++++++--------
 drivers/nvme/host/rdma.c      | 27 +++++++++++++++----------
 drivers/nvme/host/tcp.c       | 19 +++++++++++-------
 9 files changed, 124 insertions(+), 51 deletions(-)

-- 
2.53.0

[PATCH 01/15] nvme: update nvme_passthru_end() signature

[Nilay Shroff]

Change nvme_passthru_end() to return the command effects value passed
to it.

This is a preparatory change for Clang's context/thread-safety analysis
support. The conditional release annotations (__cond_releases()) model
lock release based on a function's return value. Returning the existing
effects value allows a subsequent patch to annotate nvme_passthru_end()
as conditionally releasing locks acquired by nvme_passthru_start().

No functional change intended.

A follow-up patch will add the corresponding context analysis
annotations.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c3032d6ad6b1..d6153c3e0007 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1257,7 +1257,7 @@ u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
 }
 EXPORT_SYMBOL_NS_GPL(nvme_passthru_start, "NVME_TARGET_PASSTHRU");
 
-void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
+u32 nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 		       struct nvme_command *cmd, int status)
 {
 	if (effects & NVME_CMD_EFFECTS_CSE_MASK) {
@@ -1278,7 +1278,7 @@ void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 		flush_work(&ctrl->scan_work);
 	}
 	if (ns)
-		return;
+		return effects;
 
 	switch (cmd->common.opcode) {
 	case nvme_admin_set_features:
@@ -1299,6 +1299,8 @@ void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 	default:
 		break;
 	}
+
+	return effects;
 }
 EXPORT_SYMBOL_NS_GPL(nvme_passthru_end, "NVME_TARGET_PASSTHRU");
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ccd5e05dac98..aa42f5a9d1fa 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1274,7 +1274,7 @@ u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 			 u8 opcode);
 u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode);
 int nvme_execute_rq(struct request *rq, bool at_head);
-void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
+u32 nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 		       struct nvme_command *cmd, int status);
 struct nvme_ctrl *nvme_ctrl_from_file(struct file *file);
 struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid);
-- 
2.53.0

[PATCH 02/15] nvme: add Clang context annotations for nvme_passthru_{start|stop}

[Nilay Shroff]

Annotate nvme_passthru_start() and nvme_passthru_end() for Clang
context/thread-safety analysis.

The __cond_acquires() and __cond_releases() annotations model
conditional lock acquisition and release based on a function's return
value. Use a nonzero return value as the abstract condition denoting
that the associated locks have been acquired or released.

This allows the analyzer to track the lock state across the
nvme_passthru_start() / nvme_passthru_end() pair and verify correct
locking semantics.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/nvme.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index aa42f5a9d1fa..3b6754ec2edd 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1272,10 +1272,16 @@ static inline void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl) {};
 
 u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 			 u8 opcode);
-u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode);
+u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
+	__cond_acquires(nonzero, &ctrl->subsys->lock)
+	__cond_acquires(nonzero, &ctrl->scan_lock);
+
 int nvme_execute_rq(struct request *rq, bool at_head);
 u32 nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
-		       struct nvme_command *cmd, int status);
+		       struct nvme_command *cmd, int status)
+		       __cond_releases(nonzero, &ctrl->scan_lock)
+		       __cond_releases(nonzero, &ctrl->subsys->lock);
+
 struct nvme_ctrl *nvme_ctrl_from_file(struct file *file);
 struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid);
 bool nvme_get_ns(struct nvme_ns *ns);
-- 
2.53.0

[PATCH 03/15] nvme: add Clang context annotations for nvme_ns_head::srcu

[Nilay Shroff]

Add Clang lock context annotations for helpers that operate under
head->srcu read-side protection.

The path selection helpers invoked by nvme_find_path() access SRCU-
protected data through srcu_dereference() and therefore require the
caller to hold head->srcu. Annotate these helpers and nvme_find_path()
with __must_hold_shared(&head->srcu) so that Clang's lock context
analysis can verify the SRCU locking requirements across the call chain.

Also update nvme_ns_head_ctrl_ioctl() to use __releases_shared()
to match the shared SRCU read-side lock acquired through
srcu_read_lock().

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

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 08889b20e5d8..42fa36ae5d9b 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -683,7 +683,7 @@ int nvme_ns_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
 static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
 		void __user *argp, struct nvme_ns_head *head, int srcu_idx,
 		bool open_for_write)
-	__releases(&head->srcu)
+	__releases_shared(&head->srcu)
 {
 	struct nvme_ctrl *ctrl = ns->ctrl;
 	int ret;
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 263161cb8ac0..a275c03e7c59 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -292,6 +292,7 @@ static bool nvme_path_is_disabled(struct nvme_ns *ns)
 }
 
 static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
+	__must_hold_shared(&head->srcu)
 {
 	int found_distance = INT_MAX, fallback_distance = INT_MAX, distance;
 	struct nvme_ns *found = NULL, *fallback = NULL, *ns;
@@ -343,6 +344,7 @@ static struct nvme_ns *nvme_next_ns(struct nvme_ns_head *head,
 }
 
 static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
+	__must_hold_shared(&head->srcu)
 {
 	struct nvme_ns *ns, *found = NULL;
 	int node = numa_node_id();
@@ -391,6 +393,7 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
 }
 
 static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
+	__must_hold_shared(&head->srcu)
 {
 	struct nvme_ns *best_opt = NULL, *best_nonopt = NULL, *ns;
 	unsigned int min_depth_opt = UINT_MAX, min_depth_nonopt = UINT_MAX;
@@ -434,6 +437,7 @@ static inline bool nvme_path_is_optimized(struct nvme_ns *ns)
 }
 
 static struct nvme_ns *nvme_numa_path(struct nvme_ns_head *head)
+	__must_hold_shared(&head->srcu)
 {
 	int node = numa_node_id();
 	struct nvme_ns *ns;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 3b6754ec2edd..6bea0947eeb0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1017,7 +1017,8 @@ extern const struct attribute_group *nvme_dev_attr_groups[];
 extern const struct block_device_operations nvme_bdev_ops;
 
 void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
-struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
+struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
+	__must_hold_shared(&head->srcu);
 #ifdef CONFIG_NVME_MULTIPATH
 static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
 {
-- 
2.53.0

[PATCH 04/15] nvme: add Clang context annotations for nvme_ns_head::requeue_list

[Nilay Shroff]

nvme_ns_head::requeue_list is protected by
nvme_ns_head::requeue_lock. Annotate requeue_list with
__guarded_by(&requeue_lock) so that Clang's context analysis can
validate accesses to the list.

Replace spin_lock_init(&head->requeue_lock) with
guard(spinlock_init)(&head->requeue_lock). The guard-based
initialization generates synthetic lock acquire and release events
visible to Clang's context analysis, allowing initialization of the
guarded requeue_list in nvme_mpath_alloc_disk() without triggering
context analysis warnings.

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

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a275c03e7c59..985a8c693c6f 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -706,8 +706,8 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
 	struct queue_limits lim;
 
 	mutex_init(&head->lock);
+	guard(spinlock_init)(&head->requeue_lock);
 	bio_list_init(&head->requeue_list);
-	spin_lock_init(&head->requeue_lock);
 	INIT_WORK(&head->requeue_work, nvme_requeue_work);
 	INIT_WORK(&head->partition_scan_work, nvme_partition_scan_work);
 	INIT_DELAYED_WORK(&head->remove_work, nvme_remove_head_work);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6bea0947eeb0..f60852944709 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -555,7 +555,7 @@ struct nvme_ns_head {
 	u16			nr_plids;
 	u16			*plids;
 #ifdef CONFIG_NVME_MULTIPATH
-	struct bio_list		requeue_list;
+	struct bio_list		requeue_list __guarded_by(&requeue_lock);
 	spinlock_t		requeue_lock;
 	struct work_struct	requeue_work;
 	struct work_struct	partition_scan_work;
-- 
2.53.0

[PATCH 05/15] nvme: add Clang context annotations for nvme_ns_head::current_path

[Nilay Shroff]

Annotate nvme_ns_head::current_path[] with __rcu_guarded so that
Clang's context analysis can validate accesses to the SRCU/RCU
protected pointer.

The nvme_mpath_clear_current_path() helper only compares the
current_path[] pointer value using rcu_access_pointer() and does not
dereference the referenced namespace. Annotate this access with
context_unsafe() to suppress the corresponding context analysis
warning.

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

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 985a8c693c6f..ef072588eca0 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -231,8 +231,16 @@ bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
 	bool changed = false;
 	int node;
 
+	/*
+	 * This helper is used by namespace failover/teardown and I/O policy
+	 * update paths. We only compare the head->current_path[] pointer value
+	 * and do not dereference the referenced namespace, so suppress the
+	 * context analysis warning for this lockless inspection of the
+	 * __rcu_guarded pointer.
+	 */
 	for_each_node(node) {
-		if (ns == rcu_access_pointer(head->current_path[node])) {
+		if (context_unsafe(ns ==
+				rcu_access_pointer(head->current_path[node]))) {
 			rcu_assign_pointer(head->current_path[node], NULL);
 			changed = true;
 		}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f60852944709..41f42f1aee09 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -565,7 +565,7 @@ struct nvme_ns_head {
 	unsigned int		delayed_removal_secs;
 #define NVME_NSHEAD_DISK_LIVE		0
 #define NVME_NSHEAD_QUEUE_IF_NO_PATH	1
-	struct nvme_ns __rcu	*current_path[];
+	struct nvme_ns __rcu_guarded	*current_path[];
 #endif
 };
 
-- 
2.53.0

[PATCH 06/15] nvme: add Clang context annotations for nvme_dev::shutdown_lock

[Nilay Shroff]

nvme_setup_io_queues_trylock() conditionally acquires dev->shutdown_lock
using mutex_trylock(). The function returns 0 when the lock is
successfully acquired and a negative error code otherwise.

Annotate the function with __cond_acquires(0, &dev->shutdown_lock) so
that Clang's lock context analysis can track the lock state based on
the return value and verify correct lock usage at call sites.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b5f846200678..894da63127a7 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2194,6 +2194,7 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
  * Try getting shutdown_lock while setting up IO queues.
  */
 static int nvme_setup_io_queues_trylock(struct nvme_dev *dev)
+	__cond_acquires(0, &dev->shutdown_lock)
 {
 	/*
 	 * Give up if the lock is being held by nvme_dev_disable.
-- 
2.53.0

[PATCH 07/15] nvme: add Clang context annotations for nvme_subsystem::lock

[Nilay Shroff]

Several helpers access or traverse data structures protected by
nvme_subsystem::lock and therefore require callers to hold the lock.
Annotate nvme_mpath_unfreeze(), nvme_mpath_wait_freeze(),
nvme_mpath_start_freeze(), nvme_find_ns_head(), and
nvme_subsys_check_duplicate_ids() with __must_hold(&subsys->lock) so
that Clang's lock context analysis can validate the locking requirements
at compile time.

Also annotate nvme_subsystem::nsheads and
nvme_ns_head::delayed_removal_secs with __guarded_by(&subsys->lock),
as both are protected by the subsystem lock.

Replace mutex_init(&subsys->lock) with guard(mutex_init)() so that
Clang's context analysis can observe the synthetic acquisition and
release of subsys->lock during initialization of the guarded
nvme_subsystem::nsheads list in nvme_init_subsystem().

The initialization of delayed_removal_secs in nvme_mpath_alloc_disk()
occurs before the namespace head is published and therefore does not
require synchronization. So annotate the delayed_removal_secs
initialization with context_unsafe() to suppress the corresponding
context analysis warning.

While we add above annotation, one notable finding was:

drivers/nvme/host/core.c:3967:45: warning: passing pointer to variable 'nsheads' requires holding mutex '&nvme_subsystem::lock' [-Wthread-safety-pointer]
 3967 |         list_add_tail(&head->entry, &ctrl->subsys->nsheads);
      |                                                    ^

So fixed it by acquiring nvme_subsystem::lock while updating
ctrl->subsys->nsheads.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d6153c3e0007..a18c4abf7b38 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3247,7 +3247,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 		return -ENOMEM;
 
 	subsys->instance = -1;
-	mutex_init(&subsys->lock);
+	guard(mutex_init)(&subsys->lock);
 	kref_init(&subsys->ref);
 	INIT_LIST_HEAD(&subsys->ctrls);
 	INIT_LIST_HEAD(&subsys->nsheads);
@@ -3809,6 +3809,7 @@ static const struct file_operations nvme_dev_fops = {
 
 static struct nvme_ns_head *nvme_find_ns_head(struct nvme_ctrl *ctrl,
 		unsigned nsid)
+	__must_hold(&ctrl->subsys->lock)
 {
 	struct nvme_ns_head *h;
 
@@ -3831,6 +3832,7 @@ static struct nvme_ns_head *nvme_find_ns_head(struct nvme_ctrl *ctrl,
 
 static int nvme_subsys_check_duplicate_ids(struct nvme_subsystem *subsys,
 		struct nvme_ns_ids *ids)
+	__must_hold(&subsys->lock)
 {
 	bool has_uuid = !uuid_is_null(&ids->uuid);
 	bool has_nguid = memchr_inv(ids->nguid, 0, sizeof(ids->nguid));
@@ -3962,7 +3964,9 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
 	if (ret)
 		goto out_cleanup_srcu;
 
+	mutex_lock(&ctrl->subsys->lock);
 	list_add_tail(&head->entry, &ctrl->subsys->nsheads);
+	mutex_unlock(&ctrl->subsys->lock);
 
 	kref_get(&ctrl->subsys->ref);
 
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index ef072588eca0..29e7e5178a5a 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -719,7 +719,14 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
 	INIT_WORK(&head->requeue_work, nvme_requeue_work);
 	INIT_WORK(&head->partition_scan_work, nvme_partition_scan_work);
 	INIT_DELAYED_WORK(&head->remove_work, nvme_remove_head_work);
-	head->delayed_removal_secs = 0;
+	/*
+	 * The namespace head is not yet visible to other threads, so
+	 * initializing delayed_removal_secs does not require holding
+	 * subsys->lock. So suppress Clang's context analyzer warning by
+	 * annotating initialization of delayed_removal_secs using
+	 * context_unsafe.
+	 */
+	context_unsafe(head->delayed_removal_secs = 0);
 
 	/*
 	 * If "multipath_always_on" is enabled, a multipath node is added
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 41f42f1aee09..be1467bfae28 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -490,7 +490,7 @@ struct nvme_subsystem {
 	struct list_head	entry;
 	struct mutex		lock;
 	struct list_head	ctrls;
-	struct list_head	nsheads;
+	struct list_head	nsheads __guarded_by(&lock);
 	char			subnqn[NVMF_NQN_SIZE];
 	char			serial[20];
 	char			model[40];
@@ -562,7 +562,7 @@ struct nvme_ns_head {
 	struct mutex		lock;
 	unsigned long		flags;
 	struct delayed_work	remove_work;
-	unsigned int		delayed_removal_secs;
+	unsigned int		delayed_removal_secs __guarded_by(&subsys->lock);
 #define NVME_NSHEAD_DISK_LIVE		0
 #define NVME_NSHEAD_QUEUE_IF_NO_PATH	1
 	struct nvme_ns __rcu_guarded	*current_path[];
@@ -1025,9 +1025,12 @@ static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
 	return ctrl->ana_log_buf != NULL;
 }
 
-void nvme_mpath_unfreeze(struct nvme_subsystem *subsys);
-void nvme_mpath_wait_freeze(struct nvme_subsystem *subsys);
-void nvme_mpath_start_freeze(struct nvme_subsystem *subsys);
+void nvme_mpath_unfreeze(struct nvme_subsystem *subsys)
+	__must_hold(&subsys->lock);
+void nvme_mpath_wait_freeze(struct nvme_subsystem *subsys)
+	__must_hold(&subsys->lock);
+void nvme_mpath_start_freeze(struct nvme_subsystem *subsys)
+	__must_hold(&subsys->lock);
 void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys);
 void nvme_failover_req(struct request *req);
 void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
-- 
2.53.0

[PATCH 08/15] nvme: add Clang context annotations for nvme_ctrl::ana_lock

[Nilay Shroff]

nvme_parse_ana_log() accesses ANA state protected by ctrl->ana_lock and
therefore requires callers to hold the lock.

Annotate nvme_parse_ana_log() with __must_hold(&ctrl->ana_lock) so that
Clang's lock context analysis can verify the locking requirement at
compile time.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/multipath.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 29e7e5178a5a..b12be93cc681 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -818,6 +818,7 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
 static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
 		int (*cb)(struct nvme_ctrl *ctrl, struct nvme_ana_group_desc *,
 			void *))
+		__must_hold(&ctrl->ana_lock)
 {
 	void *base = ctrl->ana_log_buf;
 	size_t offset = sizeof(struct nvme_ana_rsp_hdr);
-- 
2.53.0

[PATCH 09/15] nvme: add Clang context annotations for nvme_subsystems_lock

[Nilay Shroff]

The global nvme_subsystems list, nvme_subsystem::entry,
nvme_subsystem::ctrls, and nvme_ctrl::subsys_entry are protected by
nvme_subsystems_lock. Annotate these objects with
__guarded_by(&nvme_subsystems_lock) so that Clang's context analysis
can validate accesses to them.

__nvme_find_get_subsystem() and nvme_validate_cntlid() traverse the
global subsystem list and subsystem controller list and therefore
require callers to hold nvme_subsystems_lock. Annotate both helpers
with __must_hold(&nvme_subsystems_lock).

The initialization of subsys->ctrls in nvme_init_subsystem() occurs
before the subsystem is published and therefore does not require
protection by nvme_subsystems_lock. Annotate the initialization with
context_unsafe() to suppress the corresponding context analysis
warning.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a18c4abf7b38..409aff13c69d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -126,7 +126,7 @@ EXPORT_SYMBOL_GPL(nvme_reset_wq);
 struct workqueue_struct *nvme_delete_wq;
 EXPORT_SYMBOL_GPL(nvme_delete_wq);
 
-static LIST_HEAD(nvme_subsystems);
+static __guarded_by(&nvme_subsystems_lock) LIST_HEAD(nvme_subsystems);
 DEFINE_MUTEX(nvme_subsystems_lock);
 
 static DEFINE_IDA(nvme_instance_ida);
@@ -3164,6 +3164,7 @@ static void nvme_put_subsystem(struct nvme_subsystem *subsys)
 }
 
 static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
+	__must_hold(&nvme_subsystems_lock)
 {
 	struct nvme_subsystem *subsys;
 
@@ -3208,6 +3209,7 @@ static inline bool nvme_is_io_ctrl(struct nvme_ctrl *ctrl)
 
 static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
 		struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
+	__must_hold(&nvme_subsystems_lock)
 {
 	struct nvme_ctrl *tmp;
 
@@ -3249,7 +3251,12 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	subsys->instance = -1;
 	guard(mutex_init)(&subsys->lock);
 	kref_init(&subsys->ref);
-	INIT_LIST_HEAD(&subsys->ctrls);
+	/*
+	 * Initializing subsys->ctrls list doesn't need to be protected
+	 * using @nvme_subsystems_lock. So suppress the Clang's warning
+	 * declaring context_unsafe.
+	 */
+	context_unsafe(INIT_LIST_HEAD(&subsys->ctrls));
 	INIT_LIST_HEAD(&subsys->nsheads);
 	nvme_init_subnqn(subsys, ctrl, id);
 	memcpy(subsys->serial, id->sn, sizeof(subsys->serial));
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index be1467bfae28..837e279bdf01 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -361,7 +361,7 @@ struct nvme_ctrl {
 	wait_queue_head_t state_wq;
 
 	struct nvme_subsystem *subsys;
-	struct list_head subsys_entry;
+	struct list_head subsys_entry __guarded_by(&nvme_subsystems_lock);
 
 	struct opal_dev *opal_dev;
 
@@ -487,9 +487,9 @@ struct nvme_subsystem {
 	 * a separate refcount.
 	 */
 	struct kref		ref;
-	struct list_head	entry;
+	struct list_head	entry __guarded_by(&nvme_subsystems_lock);
 	struct mutex		lock;
-	struct list_head	ctrls;
+	struct list_head	ctrls __guarded_by(&nvme_subsystems_lock);
 	struct list_head	nsheads __guarded_by(&lock);
 	char			subnqn[NVMF_NQN_SIZE];
 	char			serial[20];
-- 
2.53.0

[PATCH 10/15] nvme: add Clang context annotations in fabric.c

[Nilay Shroff]

The global nvmf_transports list is protected by nvmf_transports_rwsem
and the global nvmf_hosts list is protected by nvmf_hosts_mutex.

Annotate both lists with __guarded_by() so that Clang's context analysis
can validate accesses to the lists against the corresponding locking
requirements.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/fabrics.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index ac3d4f400601..9a627a5095ac 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -14,11 +14,11 @@
 #include "fabrics.h"
 #include <linux/nvme-keyring.h>
 
-static LIST_HEAD(nvmf_transports);
 static DECLARE_RWSEM(nvmf_transports_rwsem);
+static __guarded_by(&nvmf_transports_rwsem) LIST_HEAD(nvmf_transports);
 
-static LIST_HEAD(nvmf_hosts);
 static DEFINE_MUTEX(nvmf_hosts_mutex);
+static __guarded_by(&nvmf_hosts_mutex) LIST_HEAD(nvmf_hosts);
 
 static struct nvmf_host *nvmf_default_host;
 
-- 
2.53.0

[PATCH 11/15] nvme: add Clang context annotations for nvme_queue::sq_lock

[Nilay Shroff]

nvme_queue::sq_tail and nvme_queue::last_sq_tail are protected by
nvme_queue::sq_lock. Annotate both fields with
__guarded_by(&sq_lock) and annotate helpers that access them with
__must_hold(&sq_lock) so that Clang's context analysis can validate
the locking requirements.

Accesses to sq_tail used solely for tracing are annotated with
context_unsafe(), as they only require a lockless snapshot of the
value. Likewise, nvme_init_queue() and nvme_free_queue() operate on
queues that have not yet been published or are no longer reachable,
and therefore do not require sq_lock protection.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/pci.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 894da63127a7..c464570cffd0 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -375,9 +375,9 @@ struct nvme_queue {
 	u32 __iomem *q_db;
 	u32 q_depth;
 	u16 cq_vector;
-	u16 sq_tail;
-	u16 last_sq_tail;
 	u16 cq_head;
+	u16 sq_tail __guarded_by(&sq_lock);
+	u16 last_sq_tail __guarded_by(&sq_lock);
 	u16 qid;
 	u8 cq_phase;
 	u8 sqes;
@@ -711,6 +711,7 @@ static void nvme_pci_map_queues(struct blk_mq_tag_set *set)
  * Write sq tail if we are asked to, or if the next command would wrap.
  */
 static inline void nvme_write_sq_db(struct nvme_queue *nvmeq, bool write_sq)
+	__must_hold(&nvmeq->sq_lock)
 {
 	if (!write_sq) {
 		u16 next_tail = nvmeq->sq_tail + 1;
@@ -729,6 +730,7 @@ static inline void nvme_write_sq_db(struct nvme_queue *nvmeq, bool write_sq)
 
 static inline void nvme_sq_copy_cmd(struct nvme_queue *nvmeq,
 				    struct nvme_command *cmd)
+	__must_hold(&nvmeq->sq_lock)
 {
 	memcpy(nvmeq->sq_cmds + (nvmeq->sq_tail << nvmeq->sqes),
 		absolute_pointer(cmd), sizeof(*cmd));
@@ -1581,7 +1583,10 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
 		return;
 	}
 
-	trace_nvme_sq(req, cqe->sq_head, nvmeq->sq_tail);
+	/*
+	 * Tracing only; a lockless snapshot of nvmeq sq_xxx/cqe is sufficient.
+	 */
+	context_unsafe(trace_nvme_sq(req, cqe->sq_head, nvmeq->sq_tail));
 	if (!nvme_try_complete_req(req, cqe->status, cqe->result) &&
 	    !blk_mq_add_to_batch(req, iob,
 				 nvme_req(req)->status != NVME_SC_SUCCESS,
@@ -2008,6 +2013,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
 }
 
 static void nvme_free_queue(struct nvme_queue *nvmeq)
+	__context_unsafe(/* frees queue which is no longer in use */)
 {
 	dma_free_coherent(nvmeq->dev->dev, CQ_SIZE(nvmeq),
 				(void *)nvmeq->cqes, nvmeq->cq_dma_addr);
@@ -2176,6 +2182,7 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
 }
 
 static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
+	__context_unsafe(/* safe to init queue without any protection */)
 {
 	struct nvme_dev *dev = nvmeq->dev;
 
-- 
2.53.0

[PATCH 12/15] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Nilay Shroff]

nvme_queue::cqes, nvme_queue::cq_head, and nvme_queue::cq_phase are
protected by nvme_queue::cq_poll_lock. Annotate these fields with
__guarded_by(&cq_poll_lock) and annotate helpers accessing them with
__must_hold(&cq_poll_lock) so that Clang's context analysis can
validate the locking requirements.

IRQ-based queues do not use cq_poll_lock and instead rely on interrupt
serialization. Annotate nvme_irq() and nvme_irq_check() with
__context_unsafe() to suppress the corresponding context analysis
warnings.

After annotating the completion queue state and helper functions,
Clang's context analysis reports that nvme_poll() invokes
nvme_cqe_pending() without holding nvme_queue::cq_poll_lock:

drivers/nvme/host/pci.c:1683:7: warning: calling function 'nvme_cqe_pending' requires holding spinlock 'hctx->driver_data->cq_poll_lock'
      exclusively [-Wthread-safety-analysis]
 1683 |             !nvme_cqe_pending(nvmeq))

Fix the warning by acquiring nvme_queue::cq_poll_lock before invoking
nvme_cqe_pending() in nvme_poll().

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/pci.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c464570cffd0..343860584b31 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -369,17 +369,17 @@ struct nvme_queue {
 	void *sq_cmds;
 	 /* only used for poll queues: */
 	spinlock_t cq_poll_lock ____cacheline_aligned_in_smp;
-	struct nvme_completion *cqes;
+	struct nvme_completion *cqes __guarded_by(&cq_poll_lock);
 	dma_addr_t sq_dma_addr;
 	dma_addr_t cq_dma_addr;
 	u32 __iomem *q_db;
 	u32 q_depth;
 	u16 cq_vector;
-	u16 cq_head;
+	u16 cq_head __guarded_by(&cq_poll_lock);
 	u16 sq_tail __guarded_by(&sq_lock);
 	u16 last_sq_tail __guarded_by(&sq_lock);
 	u16 qid;
-	u8 cq_phase;
+	u8 cq_phase __guarded_by(&cq_poll_lock);
 	u8 sqes;
 	unsigned long flags;
 #define NVMEQ_ENABLED		0
@@ -1534,6 +1534,7 @@ static void nvme_pci_complete_batch(struct io_comp_batch *iob)
 
 /* We read the CQE phase first to check if the rest of the entry is valid */
 static inline bool nvme_cqe_pending(struct nvme_queue *nvmeq)
+	__must_hold(nvmeq->cq_poll_lock)
 {
 	struct nvme_completion *hcqe = &nvmeq->cqes[nvmeq->cq_head];
 
@@ -1541,6 +1542,7 @@ static inline bool nvme_cqe_pending(struct nvme_queue *nvmeq)
 }
 
 static inline void nvme_ring_cq_doorbell(struct nvme_queue *nvmeq)
+	__must_hold(nvmeq->cq_poll_lock)
 {
 	u16 head = nvmeq->cq_head;
 
@@ -1558,6 +1560,7 @@ static inline struct blk_mq_tags *nvme_queue_tagset(struct nvme_queue *nvmeq)
 
 static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
 				   struct io_comp_batch *iob, u16 idx)
+	__must_hold(nvmeq->cq_poll_lock)
 {
 	struct nvme_completion *cqe = &nvmeq->cqes[idx];
 	__u16 command_id = READ_ONCE(cqe->command_id);
@@ -1595,6 +1598,7 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
 }
 
 static inline void nvme_update_cq_head(struct nvme_queue *nvmeq)
+	__must_hold(nvmeq->cq_poll_lock)
 {
 	u32 tmp = nvmeq->cq_head + 1;
 
@@ -1608,6 +1612,7 @@ static inline void nvme_update_cq_head(struct nvme_queue *nvmeq)
 
 static inline bool nvme_poll_cq(struct nvme_queue *nvmeq,
 			        struct io_comp_batch *iob)
+	__must_hold(nvmeq->cq_poll_lock)
 {
 	bool found = false;
 
@@ -1628,6 +1633,7 @@ static inline bool nvme_poll_cq(struct nvme_queue *nvmeq,
 }
 
 static irqreturn_t nvme_irq(int irq, void *data)
+	__context_unsafe(/* IRQ queues do not use cq_poll_lock  */)
 {
 	struct nvme_queue *nvmeq = data;
 	DEFINE_IO_COMP_BATCH(iob);
@@ -1641,6 +1647,7 @@ static irqreturn_t nvme_irq(int irq, void *data)
 }
 
 static irqreturn_t nvme_irq_check(int irq, void *data)
+	__context_unsafe(/* IRQ queues do not use cq_poll_lock */)
 {
 	struct nvme_queue *nvmeq = data;
 
@@ -1673,11 +1680,14 @@ static int nvme_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
 	struct nvme_queue *nvmeq = hctx->driver_data;
 	bool found;
 
-	if (!test_bit(NVMEQ_POLLED, &nvmeq->flags) ||
-	    !nvme_cqe_pending(nvmeq))
+	if (!test_bit(NVMEQ_POLLED, &nvmeq->flags))
 		return 0;
 
 	spin_lock(&nvmeq->cq_poll_lock);
+	if (!nvme_cqe_pending(nvmeq)) {
+		spin_unlock(&nvmeq->cq_poll_lock);
+		return 0;
+	}
 	found = nvme_poll_cq(nvmeq, iob);
 	spin_unlock(&nvmeq->cq_poll_lock);
 
@@ -2133,6 +2143,7 @@ static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,
 }
 
 static int nvme_alloc_queue(struct nvme_dev *dev, int qid, int depth)
+	__context_unsafe(/* safe to allocate queue without any protection */)
 {
 	struct nvme_queue *nvmeq = &dev->queues[qid];
 
-- 
2.53.0

[PATCH 13/15] nvme: add Clang context annotations in rdma.c

[Nilay Shroff]

The device_list and nvme_rdma_device::entry are protected by
device_list_mutex. Annotate both with __guarded_by() so that Clang's
context analysis can validate accesses against the corresponding
locking requirements.

Similarly, nvme_rdma_ctrl_list and nvme_rdma_ctrl::list are protected
by nvme_rdma_ctrl_mutex. Annotate them with
__guarded_by(&nvme_rdma_ctrl_mutex).

It is safe to initialize nvme_rdma_ctrl::list while allocating the
controller object because the list entry has not yet been added to
nvme_rdma_ctrl_list. Annotate the initialization with
context_unsafe() to suppress the corresponding Clang warning.

After adding the above annotations, Clang reports the following
warning:

drivers/nvme/host/rdma.c:972:24: warning: passing pointer to variable 'list' requires holding mutex 'nvme_rdma_ctrl_mutex'
      [-Wthread-safety-pointer]
  972 |         if (list_empty(&ctrl->list))
      |                               ^

Fix the warning by performing the list_empty() check while holding
nvme_rdma_ctrl_mutex.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/rdma.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index f77c960f7632..a50efe3709d8 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -39,11 +39,17 @@
 #define NVME_RDMA_METADATA_SGL_SIZE \
 	(sizeof(struct scatterlist) * NVME_INLINE_METADATA_SG_CNT)
 
+static DEFINE_MUTEX(device_list_mutex);
+static __guarded_by(&device_list_mutex) LIST_HEAD(device_list);
+
+static DEFINE_MUTEX(nvme_rdma_ctrl_mutex);
+static __guarded_by(&nvme_rdma_ctrl_mutex) LIST_HEAD(nvme_rdma_ctrl_list);
+
 struct nvme_rdma_device {
 	struct ib_device	*dev;
 	struct ib_pd		*pd;
 	struct kref		ref;
-	struct list_head	entry;
+	struct list_head	entry __guarded_by(&device_list_mutex);
 	unsigned int		num_inline_segments;
 };
 
@@ -112,7 +118,7 @@ struct nvme_rdma_ctrl {
 
 	struct delayed_work	reconnect_work;
 
-	struct list_head	list;
+	struct list_head	list __guarded_by(&nvme_rdma_ctrl_mutex);
 
 	struct blk_mq_tag_set	admin_tag_set;
 	struct nvme_rdma_device	*device;
@@ -132,12 +138,6 @@ static inline struct nvme_rdma_ctrl *to_rdma_ctrl(struct nvme_ctrl *ctrl)
 	return container_of(ctrl, struct nvme_rdma_ctrl, ctrl);
 }
 
-static LIST_HEAD(device_list);
-static DEFINE_MUTEX(device_list_mutex);
-
-static LIST_HEAD(nvme_rdma_ctrl_list);
-static DEFINE_MUTEX(nvme_rdma_ctrl_mutex);
-
 /*
  * Disabling this option makes small I/O goes faster, but is fundamentally
  * unsafe.  With it turned off we will have to register a global rkey that
@@ -969,10 +969,12 @@ static void nvme_rdma_free_ctrl(struct nvme_ctrl *nctrl)
 {
 	struct nvme_rdma_ctrl *ctrl = to_rdma_ctrl(nctrl);
 
-	if (list_empty(&ctrl->list))
-		goto free_ctrl;
 
 	mutex_lock(&nvme_rdma_ctrl_mutex);
+	if (list_empty(&ctrl->list)) {
+		mutex_unlock(&nvme_rdma_ctrl_mutex);
+		goto free_ctrl;
+	}
 	list_del(&ctrl->list);
 	mutex_unlock(&nvme_rdma_ctrl_mutex);
 
@@ -2252,7 +2254,10 @@ static struct nvme_rdma_ctrl *nvme_rdma_alloc_ctrl(struct device *dev,
 	if (!ctrl)
 		return ERR_PTR(-ENOMEM);
 	ctrl->ctrl.opts = opts;
-	INIT_LIST_HEAD(&ctrl->list);
+	/*
+	 * Safe to init list while allocating ctrl object.
+	 */
+	context_unsafe(INIT_LIST_HEAD(&ctrl->list));
 
 	if (!(opts->mask & NVMF_OPT_TRSVCID)) {
 		opts->trsvcid =
-- 
2.53.0

[PATCH 14/15] nvme: add Clang context annotations in tcp.c

[Nilay Shroff]

The nvme_tcp_ctrl_list and nvme_tcp_ctrl::list are protected by
nvme_tcp_ctrl_mutex. Annotate both with
__guarded_by(&nvme_tcp_ctrl_mutex) so that Clang's context analysis
can validate accesses against the corresponding locking requirements.

It is safe to initialize nvme_tcp_ctrl::list while allocating the
controller object because the list entry has not yet been added to
nvme_tcp_ctrl_list. Annotate the initialization with context_unsafe()
to suppress the corresponding Clang warning.

After adding the above annotations, Clang reports the following
warning:
    drivers/nvme/host/tcp.c:2572:24: warning: passing pointer to variable 'list' requires holding mutex 'nvme_tcp_ctrl_mutex'
          [-Wthread-safety-pointer]
     2572 |         if (list_empty(&ctrl->list))
          |                               ^

Fix the warning by performing the list_empty() check while holding
nvme_tcp_ctrl_mutex.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/tcp.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 68a1d7640494..86ef98aefeb4 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -182,13 +182,16 @@ struct nvme_tcp_queue {
 	void (*write_space)(struct sock *);
 };
 
+static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
+static __guarded_by(&nvme_tcp_ctrl_mutex) LIST_HEAD(nvme_tcp_ctrl_list);
+
 struct nvme_tcp_ctrl {
 	/* read only in the hot path */
 	struct nvme_tcp_queue	*queues;
 	struct blk_mq_tag_set	tag_set;
 
 	/* other member variables */
-	struct list_head	list;
+	struct list_head	list __guarded_by(&nvme_tcp_ctrl_mutex);
 	struct blk_mq_tag_set	admin_tag_set;
 	struct sockaddr_storage addr;
 	struct sockaddr_storage src_addr;
@@ -200,8 +203,6 @@ struct nvme_tcp_ctrl {
 	u32			io_queues[HCTX_MAX_TYPES];
 };
 
-static LIST_HEAD(nvme_tcp_ctrl_list);
-static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
 static struct workqueue_struct *nvme_tcp_wq;
 static const struct blk_mq_ops nvme_tcp_mq_ops;
 static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
@@ -2568,10 +2569,11 @@ static void nvme_tcp_free_ctrl(struct nvme_ctrl *nctrl)
 {
 	struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
 
-	if (list_empty(&ctrl->list))
-		goto free_ctrl;
-
 	mutex_lock(&nvme_tcp_ctrl_mutex);
+	if (list_empty(&ctrl->list)) {
+		mutex_unlock(&nvme_tcp_ctrl_mutex);
+		goto free_ctrl;
+	}
 	list_del(&ctrl->list);
 	mutex_unlock(&nvme_tcp_ctrl_mutex);
 
@@ -2910,7 +2912,10 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
 	if (!ctrl)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_LIST_HEAD(&ctrl->list);
+	/*
+	 * Safe to init list while allocating ctrl object.
+	 */
+	context_unsafe(INIT_LIST_HEAD(&ctrl->list));
 	ctrl->ctrl.opts = opts;
 	ctrl->ctrl.queue_count = opts->nr_io_queues + opts->nr_write_queues +
 				opts->nr_poll_queues + 1;
-- 
2.53.0

[PATCH 15/15] nvme: enable Clang context analysis support for nvme host driver

[Nilay Shroff]

Update nvme host driver makefile to enable support for the Clang's
context anaysis.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
index 6414ec968f99..67563a69f7dc 100644
--- a/drivers/nvme/host/Makefile
+++ b/drivers/nvme/host/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
+CONTEXT_ANALYSIS			:= y
 ccflags-y				+= -I$(src)
 
 obj-$(CONFIG_NVME_CORE)			+= nvme-core.o
-- 
2.53.0

Re: [PATCH 04/15] nvme: add Clang context annotations for nvme_ns_head::requeue_list

[Bart Van Assche]

On 6/10/26 7:27 AM, Nilay Shroff wrote:
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index a275c03e7c59..985a8c693c6f 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -706,8 +706,8 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>   	struct queue_limits lim;
>   
>   	mutex_init(&head->lock);
> +	guard(spinlock_init)(&head->requeue_lock);
>   	bio_list_init(&head->requeue_list);
> -	spin_lock_init(&head->requeue_lock);
scoped_guard() is preferred over guard() because with scoped_guard() the
scope the guard applies to is explicit. When using guard(), if new code
is added, it may end up being covered by a guard() statement although it
shouldn't.

Thanks,

Bart.

Re: [PATCH 07/15] nvme: add Clang context annotations for nvme_subsystem::lock

[Bart Van Assche]

On 6/10/26 7:27 AM, Nilay Shroff wrote:
> +	mutex_lock(&ctrl->subsys->lock);
>   	list_add_tail(&head->entry, &ctrl->subsys->nsheads);
> +	mutex_unlock(&ctrl->subsys->lock);

Has it been considered to use scoped_guard()?

Thanks,

Bart.

Re: [PATCH 09/15] nvme: add Clang context annotations for nvme_subsystems_lock

[Bart Van Assche]

On 6/10/26 7:27 AM, Nilay Shroff wrote:
> -	INIT_LIST_HEAD(&subsys->ctrls);
> +	/*
> +	 * Initializing subsys->ctrls list doesn't need to be protected
> +	 * using @nvme_subsystems_lock. So suppress the Clang's warning
> +	 * declaring context_unsafe.
> +	 */
> +	context_unsafe(INIT_LIST_HEAD(&subsys->ctrls));

Why context_unsafe() instead of scoped_guard(mutex_init, ...)?

Thanks,

Bart.

Re: [PATCH 11/15] nvme: add Clang context annotations for nvme_queue::sq_lock

[Bart Van Assche]

On 6/10/26 7:27 AM, Nilay Shroff wrote:
>   static void nvme_free_queue(struct nvme_queue *nvmeq)
> +	__context_unsafe(/* frees queue which is no longer in use */)
>   {
>   	dma_free_coherent(nvmeq->dev->dev, CQ_SIZE(nvmeq),
>   				(void *)nvmeq->cqes, nvmeq->cq_dma_addr);
> @@ -2176,6 +2182,7 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
>   }
>   
>   static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
> +	__context_unsafe(/* safe to init queue without any protection */)
>   {
>   	struct nvme_dev *dev = nvmeq->dev;

__context_unsafe() is a big hammer that disables context analysis
for the entire function body. Has it been considered to use
guard(..._init)(...) instead?

Thanks,

Bart.

Re: [PATCH 12/15] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Bart Van Assche]

On 6/10/26 7:27 AM, Nilay Shroff wrote:
> Fix the warning by acquiring nvme_queue::cq_poll_lock before invoking
> nvme_cqe_pending() in nvme_poll().

Please separate functionality changes from Clang context annotations.

Thanks,

Bart.

Re: [PATCH 12/15] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Keith Busch]

On Wed, Jun 10, 2026 at 07:57:32PM +0530, Nilay Shroff wrote:
> Fix the warning by acquiring nvme_queue::cq_poll_lock before invoking
> nvme_cqe_pending() in nvme_poll().

The check outside the lock was done on purpose, though.

Re: [PATCH 04/15] nvme: add Clang context annotations for nvme_ns_head::requeue_list

[Nilay Shroff]

On 6/10/26 9:55 PM, Bart Van Assche wrote:
> On 6/10/26 7:27 AM, Nilay Shroff wrote:
>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>> index a275c03e7c59..985a8c693c6f 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -706,8 +706,8 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>>       struct queue_limits lim;
>>       mutex_init(&head->lock);
>> +    guard(spinlock_init)(&head->requeue_lock);
>>       bio_list_init(&head->requeue_list);
>> -    spin_lock_init(&head->requeue_lock);
> scoped_guard() is preferred over guard() because with scoped_guard() the
> scope the guard applies to is explicit. When using guard(), if new code
> is added, it may end up being covered by a guard() statement although it
> shouldn't.
> 
Yes makes sense, I'll replace guard() with scoped_guard().

Thanks,
--Nilay

Re: [PATCH 07/15] nvme: add Clang context annotations for nvme_subsystem::lock

[Nilay Shroff]

On 6/10/26 9:58 PM, Bart Van Assche wrote:
> On 6/10/26 7:27 AM, Nilay Shroff wrote:
>> +    mutex_lock(&ctrl->subsys->lock);
>>       list_add_tail(&head->entry, &ctrl->subsys->nsheads);
>> +    mutex_unlock(&ctrl->subsys->lock);
> 
> Has it been considered to use scoped_guard()?
> 

Yes, scoped_guard() would work here. However, my preference is
to use the guard helpers primarily for object initialization,
where they create a temporary synthetic acquire/release pattern
that helps Clang's context analysis understand accesses to newly
initialized objects.

In this case, we are simply protecting a list update with a
straightforward lock/unlock pair. I find the explicit locking
pattern a bit more intuitive and easier to follow here, since
the locking scope is small and there are no early returns or
error paths that would particularly benefit from scoped_guard().

Thanks,
--Nilay

Re: [PATCH 09/15] nvme: add Clang context annotations for nvme_subsystems_lock

[Nilay Shroff]

On 6/10/26 10:00 PM, Bart Van Assche wrote:
> On 6/10/26 7:27 AM, Nilay Shroff wrote:
>> -    INIT_LIST_HEAD(&subsys->ctrls);
>> +    /*
>> +     * Initializing subsys->ctrls list doesn't need to be protected
>> +     * using @nvme_subsystems_lock. So suppress the Clang's warning
>> +     * declaring context_unsafe.
>> +     */
>> +    context_unsafe(INIT_LIST_HEAD(&subsys->ctrls));
> 
> Why context_unsafe() instead of scoped_guard(mutex_init, ...)?
> 
Well, in this case the subsys->ctrls is protected using a mutex
which is defined statically: DEFINE_MUTEX(nvme_subsystems_lock).
So to suppress the compiler warning, used context_unsafe.

Thanks,
--Nilay

Re: [PATCH 11/15] nvme: add Clang context annotations for nvme_queue::sq_lock

[Nilay Shroff]

On 6/10/26 10:03 PM, Bart Van Assche wrote:
> On 6/10/26 7:27 AM, Nilay Shroff wrote:
>>   static void nvme_free_queue(struct nvme_queue *nvmeq)
>> +    __context_unsafe(/* frees queue which is no longer in use */)
>>   {
>>       dma_free_coherent(nvmeq->dev->dev, CQ_SIZE(nvmeq),
>>                   (void *)nvmeq->cqes, nvmeq->cq_dma_addr);
>> @@ -2176,6 +2182,7 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
>>   }
>>   static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
>> +    __context_unsafe(/* safe to init queue without any protection */)
>>   {
>>       struct nvme_dev *dev = nvmeq->dev;
> 
> __context_unsafe() is a big hammer that disables context analysis
> for the entire function body. Has it been considered to use
> guard(..._init)(...) instead?
> 
Yeah, I considered using guard(..._init), but it is a bit tricky in
this case. The lock (nvmeq->sq_lock) that protects nvmeq->sq_tail and
nvmeq->last_sq_tail is initialized in nvme_alloc_queue(), while those
fields are initialized later in nvme_init_queue(). Because the lock
initialization and the protected-field initialization happen in different
functions, I don't see a straightforward way to use guard() here to
create the synthetic acquire/release pattern around the accesses that
Clang complains about.

That said, I agree that __context_unsafe() is a fairly big hammer since
it disables context analysis for the entire function body. One alternative
would be to wrap the individual field initializations with context_unsafe(),
but that would require annotating several initialization/freeing operations
throughout these functions, which would add a fair amount of noise and make
the code less readable. Given that these functions operate on objects that
are still being initialized (or are already being torn down), using
__context_unsafe() seemed like the cleaner tradeoff.

Thanks,
--Nilay

Re: [PATCH 12/15] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Nilay Shroff]

On 6/10/26 10:05 PM, Bart Van Assche wrote:
> On 6/10/26 7:27 AM, Nilay Shroff wrote:
>> Fix the warning by acquiring nvme_queue::cq_poll_lock before invoking
>> nvme_cqe_pending() in nvme_poll().
> 
> Please separate functionality changes from Clang context annotations.
> 
Okay, I would split this into a separate patch.

Thanks,
--Nilay

Re: [PATCH 12/15] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Nilay Shroff]

On 6/10/26 10:16 PM, Keith Busch wrote:
> On Wed, Jun 10, 2026 at 07:57:32PM +0530, Nilay Shroff wrote:
>> Fix the warning by acquiring nvme_queue::cq_poll_lock before invoking
>> nvme_cqe_pending() in nvme_poll().
> 
> The check outside the lock was done on purpose, though.

Oh okay, I didn't know that. In that case we could skip
the locking around nvme_cqe_pending() and silence the
compiler warning using context_unsafe().

Thanks,
--Nilay