[PATCHv2 00/17] Support Clang context analysis for NVMe host drivers

[PATCHv2 00/17] 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/

Changes from v1:
  - replace guard() with scoped_guard() for guarding request_list (Bart)
  - annotate nvme_alloc_ns_head() using __must_hold(&ctrl->subsys->lock)
    (Sashiko)
  - guard nvme_queue::sq_cmds using nvme_queue::sq_lock
  - annotate nvme_cqe_pending() using context_unsafe in nvme_poll()
    (Keith)
  - Split patch #13 and #14 to separate the context annotation change
    from functionality change (Bart)

Nilay Shroff (17):
  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: fix Clang context analysis warning in rdma.c
  nvme: add Clang context annotations in tcp.c
  nvme: fix Clang context analysis warning in tcp.c
  nvme: enable Clang context analysis support for nvme host driver

 drivers/nvme/host/Makefile    |  1 +
 drivers/nvme/host/core.c      | 24 +++++++++++++++------
 drivers/nvme/host/fabrics.c   |  4 ++--
 drivers/nvme/host/ioctl.c     |  2 +-
 drivers/nvme/host/multipath.c | 28 +++++++++++++++++++++----
 drivers/nvme/host/nvme.h      | 38 +++++++++++++++++++++-------------
 drivers/nvme/host/pci.c       | 39 ++++++++++++++++++++++++++++-------
 drivers/nvme/host/rdma.c      | 28 ++++++++++++++-----------
 drivers/nvme/host/tcp.c       | 19 ++++++++++-------
 9 files changed, 129 insertions(+), 54 deletions(-)

-- 
2.53.0

[PATCHv2 01/17] 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

[PATCHv2 02/17] 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

[PATCHv2 03/17] 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

[PATCHv2 04/17] 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
scoped_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 nvme_ns_head::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 | 4 ++--
 drivers/nvme/host/nvme.h      | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a275c03e7c59..9e7f12fe8cd6 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);
-	bio_list_init(&head->requeue_list);
-	spin_lock_init(&head->requeue_lock);
+	scoped_guard(spinlock_init, &head->requeue_lock)
+		bio_list_init(&head->requeue_list);
 	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

[PATCHv2 05/17] 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 9e7f12fe8cd6..94607c5d1a2b 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

[PATCHv2 06/17] 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

[PATCHv2 07/17] 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(), nvme_alloc_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 scoped_guard(mutex_init,
&subsys->lock) 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.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d6153c3e0007..341753a4be85 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3247,10 +3247,10 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 		return -ENOMEM;
 
 	subsys->instance = -1;
-	mutex_init(&subsys->lock);
+	scoped_guard(mutex_init, &subsys->lock)
+		INIT_LIST_HEAD(&subsys->nsheads);
 	kref_init(&subsys->ref);
 	INIT_LIST_HEAD(&subsys->ctrls);
-	INIT_LIST_HEAD(&subsys->nsheads);
 	nvme_init_subnqn(subsys, ctrl, id);
 	memcpy(subsys->serial, id->sn, sizeof(subsys->serial));
 	memcpy(subsys->model, id->mn, sizeof(subsys->model));
@@ -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));
@@ -3922,6 +3924,7 @@ static int nvme_add_ns_cdev(struct nvme_ns *ns)
 
 static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
 		struct nvme_ns_info *info)
+	__must_hold(&ctrl->subsys->lock)
 {
 	struct nvme_ns_head *head;
 	size_t size = sizeof(*head);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 94607c5d1a2b..e816d88ffeee 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

[PATCHv2 08/17] 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 e816d88ffeee..4eb050671f32 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

[PATCHv2 09/17] 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 341753a4be85..89abf94639ff 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;
 
@@ -3250,7 +3252,12 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	scoped_guard(mutex_init, &subsys->lock)
 		INIT_LIST_HEAD(&subsys->nsheads);
 	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));
 	nvme_init_subnqn(subsys, ctrl, id);
 	memcpy(subsys->serial, id->sn, sizeof(subsys->serial));
 	memcpy(subsys->model, id->mn, sizeof(subsys->model));
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

[PATCHv2 10/17] 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

[PATCHv2 11/17] nvme: add Clang context annotations for nvme_queue::sq_lock

[Nilay Shroff]

nvme_queue::sq_tail, nvme_queue::last_sq_tail and nvme_queue::sq_cmds
are protected by nvme_queue::sq_lock. Annotate each field 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. Similarly,
nvme_alloc_sq_cmds() allocates memory for nvme_queue::sq_cmds for
the queue which is not yet published or in use and hence it's safe
to annotate all these helpers using context_unsafe.

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

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 894da63127a7..86cbc88a05b5 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -366,7 +366,7 @@ struct nvme_queue {
 	struct nvme_dev *dev;
 	struct nvme_descriptor_pools descriptor_pools;
 	spinlock_t sq_lock;
-	void *sq_cmds;
+	void *sq_cmds __guarded_by(&sq_lock);
 	 /* only used for poll queues: */
 	spinlock_t cq_poll_lock ____cacheline_aligned_in_smp;
 	struct nvme_completion *cqes;
@@ -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);
@@ -2102,6 +2108,7 @@ static int nvme_cmb_qdepth(struct nvme_dev *dev, int nr_io_queues,
 
 static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,
 				int qid)
+	__context_unsafe(/* safe to allocate sq_cmds without any protection */)
 {
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 
@@ -2176,6 +2183,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

[PATCHv2 12/17] 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.

nvme_poll() invokes nvme_cqe_pending() as a lockless fast-path check
before acquiring cq_poll_lock. This check is intentionally kept outside
the lock because nvme_poll() may be called repeatedly in a tight polling
loop until completions are found. The result is only advisory, as the
completion queue is subsequently revalidated under cq_poll_lock by
nvme_poll_cq(). Suppress the corresponding context analysis warning by
annotating the lockless invocation of nvme_cqe_pending() with
context_unsafe().

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

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 86cbc88a05b5..2e60d95c11a2 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -369,17 +369,17 @@ struct nvme_queue {
 	void *sq_cmds __guarded_by(&sq_lock);
 	 /* 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,8 +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;
 
+	/*
+	 * nvme_cqe_pending() is intentionally used as a lockless fast-path
+	 * check before taking ->cq_poll_lock. The result is only advisory and
+	 * the CQ is revalidated under ->cq_poll_lock by nvme_poll_cq(), so
+	 * suppress the context analysis warning for this lockless inspection.
+	 */
 	if (!test_bit(NVMEQ_POLLED, &nvmeq->flags) ||
-	    !nvme_cqe_pending(nvmeq))
+	    context_unsafe(!nvme_cqe_pending(nvmeq)))
 		return 0;
 
 	spin_lock(&nvmeq->cq_poll_lock);
@@ -2134,6 +2147,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

[PATCHv2 13/17] 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.

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

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index f77c960f7632..2c55f05f7550 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
@@ -2252,7 +2252,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

[PATCHv2 14/17] nvme: fix Clang context analysis warning in rdma.c

[Nilay Shroff]

After adding Clang lock context annotations in rdma.c, Clang reports
the following warning when context analysis is enabled:

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))
	  |                               ^

The warning is triggered because ctrl->list is annotated as being
protected by nvme_rdma_ctrl_mutex, but list_empty(&ctrl->list) is
invoked without holding that mutex.

Fix the warning by performing the list_empty(&ctrl->list) check while
holding nvme_rdma_ctrl_mutex.

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

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 2c55f05f7550..da7770025059 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -969,10 +969,11 @@ 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);
 
-- 
2.53.0

[PATCHv2 15/17] 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.

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

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 68a1d7640494..c4662ccb5c49 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;
@@ -2910,7 +2911,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

[PATCHv2 16/17] nvme: fix Clang context analysis warning in tcp.c

[Nilay Shroff]

After adding Clang context annotations, compiling tcp.c reports the
following warning while context analysis is enabled:

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))
     |                               ^

The above warning is triggered because ctrl->list is guarded with mutex
nvme_tcp_ctrl_mutex but when list_empty(&ctrl->list) is invoked it
doesn't acquire nvme_tcp_ctrl_mutex.

Fix the warning by performing the list_empty(&ctrl->list) check while
holding nvme_tcp_ctrl_mutex.

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

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index c4662ccb5c49..86ef98aefeb4 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2569,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);
 
-- 
2.53.0

[PATCHv2 17/17] 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