[PATCH 06/23] nvmet: add context annotation for nvmet_ns::nsid

[Nilay Shroff]

The nvmet_ns::nsid field is protected by nvmet_subsys::lock.
Annotate it with __guarded_by(&subsys->lock) to allow the Clang
context analyzer to validate accesses to this field.

The I/O hotpath also accesses nvmet_ns::nsid without holding
subsys->lock. This is safe because the namespace reference held by
the I/O request ensures that the namespace cannot be modified while
the I/O is in flight.

Since the context analyzer cannot distinguish between accesses from
the I/O hotpath and the control path, introduce nvmet_ns_nsid(),
annotated with context_unsafe, to allow the I/O hotpath to access
nvmet_ns::nsid without triggering context analyzer warnings. Use
this accessor for accesses from the I/O hotpath.

Additionally, nvmet_max_nsid() and nvmet_ns_alloc() access
nvmet_ns::nsid while holding subsys->lock. Although subsys->lock
and ns->subsys->lock refer to the same lock, the context analyzer
cannot recognize this relationship. Explicitly assert the
ns->subsys->lock capability around these accesses to avoid false
positive warnings.

Annotate nvmet_ns_changed() and nvmet_debugfs_ns_setup() with
__must_hold() to make their locking requirements explicit.

Finally, nvmet_set_feat_write_protect() calls nvmet_ns_changed()
while holding ns->subsys->lock. Since the context analyzer
cannot recognize that this is the same lock as subsys->lock,
explicitly assert the subsys->lock capability before calling
nvmet_ns_changed().

Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
---
 drivers/nvme/target/admin-cmd.c | 12 ++++++++++--
 drivers/nvme/target/core.c      | 21 +++++++++++++++++++--
 drivers/nvme/target/debugfs.h   |  3 ++-
 drivers/nvme/target/nvmet.h     |  7 +++++--
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 6b0ba383106d..590ca68588f8 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -922,7 +922,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	 * Since we don't know any better, every namespace is its own endurance
 	 * group.
 	 */
-	id->endgid = cpu_to_le16(req->ns->nsid);
+	id->endgid = cpu_to_le16(nvmet_ns_nsid(req->ns));
 
 	memcpy(&id->nguid, &req->ns->nguid, sizeof(id->nguid));
 
@@ -1268,8 +1268,16 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req)
 		break;
 	}
 
-	if (!status)
+	if (!status) {
+		/*
+		 * Both ns->subsys->lock and subsys->lock refers to the same
+		 * lock. However there's no way for us to annotate such locking
+		 * semantics using clang context annotation. So explicitly
+		 * assert the capability.
+		 */
+		__assume_ctx_lock(&subsys->lock);
 		nvmet_ns_changed(subsys, req->ns->nsid);
+	}
 	mutex_unlock(&req->ns->subsys->lock);
 	return status;
 }
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 116b1b500790..2913ed76bc6a 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -118,13 +118,22 @@ u16 nvmet_zero_sgl(struct nvmet_req *req, off_t off, size_t len)
 }
 
 static u32 nvmet_max_nsid(struct nvmet_subsys *subsys)
+	__must_hold(&subsys->lock)
 {
 	struct nvmet_ns *cur;
 	unsigned long idx;
 	u32 nsid = 0;
 
-	nvmet_for_each_enabled_ns(&subsys->namespaces, idx, cur)
+	nvmet_for_each_enabled_ns(&subsys->namespaces, idx, cur) {
+		/*
+		 * Both ns->subsys->lock and subsys->lock refers to the same
+		 * lock. However there's no way for us to annotate such locking
+		 * semantics using clang context annotation. So explicitly
+		 * assert the capability.
+		 */
+		__assume_ctx_lock(&cur->subsys->lock);
 		nsid = cur->nsid;
+	}
 
 	return nsid;
 }
@@ -746,6 +755,12 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
 
 	init_completion(&ns->disable_done);
 
+	/*
+	 * Both subsys->lock and ns->subsys->lock refers to the same lock.
+	 * However there's no way for us to annotate such locking semantics
+	 * using clang context annotation. So explicitly assert the capability.
+	 */
+	__assume_ctx_lock(&ns->subsys->lock);
 	ns->nsid = nsid;
 	ns->subsys = subsys;
 
@@ -757,6 +772,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
 
 	subsys->nr_namespaces++;
 
+	__release_ctx_lock(&ns->subsys->lock);
 	mutex_unlock(&subsys->lock);
 
 	down_write(&nvmet_ana_sem);
@@ -1361,7 +1377,8 @@ static struct pci_dev *nvmet_req_find_p2p_dev(struct nvmet_req *req)
 	if (!IS_ENABLED(CONFIG_PCI_P2PDMA) ||
 	    !req->sq->ctrl || !req->sq->qid || !req->ns)
 		return NULL;
-	return radix_tree_lookup(&req->sq->ctrl->p2p_ns_map, req->ns->nsid);
+	return radix_tree_lookup(&req->sq->ctrl->p2p_ns_map,
+			nvmet_ns_nsid(req->ns));
 }
 
 int nvmet_req_alloc_sgls(struct nvmet_req *req)
diff --git a/drivers/nvme/target/debugfs.h b/drivers/nvme/target/debugfs.h
index b559d254fc2a..2dde79e8a2ab 100644
--- a/drivers/nvme/target/debugfs.h
+++ b/drivers/nvme/target/debugfs.h
@@ -14,7 +14,8 @@ int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys);
 void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys);
 int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl);
 void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl);
-void nvmet_debugfs_ns_setup(struct nvmet_ns *ns);
+void nvmet_debugfs_ns_setup(struct nvmet_ns *ns)
+	__must_hold(&ns->subsys->lock);
 void nvmet_debugfs_ns_free(struct nvmet_ns *ns);
 
 int __init nvmet_init_debugfs(void);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index b76671672aca..d03e059d6d16 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -157,7 +157,8 @@ struct nvmet_ns {
 		__guarded_by(&subsys->lock);
 	bool			readonly
 		__guarded_by(&subsys->lock);
-	u32			nsid;
+	u32			nsid
+		__guarded_by(&subsys->lock);
 	u32			blksize_shift;
 	loff_t			size;
 	u8			nguid[16];
@@ -205,6 +206,7 @@ static inline _type nvmet_ns_##_name(struct nvmet_ns *ns)		    \
 
 NVMET_NS_ACCESSOR(struct block_device *, bdev)
 NVMET_NS_ACCESSOR(struct file *, file)
+NVMET_NS_ACCESSOR(u32, nsid)
 
 /*
  * These accessors use data_race() and READ_ONCE() to allow lockless access to
@@ -784,7 +786,8 @@ u16 nvmet_bdev_flush(struct nvmet_req *req)
 	__must_hold(&req->ns->subsys->lock);
 u16 nvmet_file_flush(struct nvmet_req *req)
 	__must_hold(&req->ns->subsys->lock);
-void nvmet_ns_changed(struct nvmet_subsys *subsys, u32 nsid);
+void nvmet_ns_changed(struct nvmet_subsys *subsys, u32 nsid)
+	__must_hold(&subsys->lock);
 void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
 	__must_hold(&ns->subsys->lock);
 void nvmet_file_ns_revalidate(struct nvmet_ns *ns)
-- 
2.53.0