[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 at 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