[PATCH 05/23] nvmet: add context annotation for nvmet_ns::readonly

[Nilay Shroff]

The nvmet_ns::readonly 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 admin command handlers nvmet_get_feat_write_protect() and
nvmet_set_feat_write_protect() access nvmet_ns::readonly while
holding ns->subsys->lock. However, the context analyzer cannot
recognize that subsys->lock and ns->subsys->lock refer to the
same mutex. Use ns->subsys->lock directly in these functions
to avoid false positive warnings.

The I/O hotpath functions, nvmet_execute_identify_ns(),
nvmet_execute_id_cs_indep() and nvmet_io_cmd_check_access(),
read nvmet_ns::readonly without holding the lock. These lockless
reads may race with updates to the field, potentially observing a
transient value. Such races are acceptable for these accesses.

Introduce nvmet_ns_readonly() to perform these lockless reads
using data_race(READ_ONCE()). This suppresses KCSAN reports for
the intentional races and informs the context analyzer that the
access is intentionally performed without holding the lock.

Use WRITE_ONCE() for updates to nvmet_ns::readonly in
nvmet_set_feat_write_protect() to pair with the lockless
READ_ONCE() accesses.

Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
---
 drivers/nvme/target/admin-cmd.c | 15 +++++++--------
 drivers/nvme/target/core.c      |  2 +-
 drivers/nvme/target/nvmet.h     | 16 +++++++++++++++-
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 517f8cc78473..6b0ba383106d 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -938,7 +938,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 		id->lbaf[0].ms = cpu_to_le16(req->ns->metadata_size);
 	}
 
-	if (req->ns->readonly)
+	if (nvmet_ns_readonly(req->ns))
 		id->nsattr |= NVME_NS_ATTR_RO;
 done:
 	if (!status)
@@ -1133,7 +1133,7 @@ static void nvmet_execute_id_cs_indep(struct nvmet_req *req)
 	id->nstat = NVME_NSTAT_NRDY;
 	id->anagrpid = cpu_to_le32(req->ns->anagrpid);
 	id->nmic = NVME_NS_NMIC_SHARED;
-	if (req->ns->readonly)
+	if (nvmet_ns_readonly(req->ns))
 		id->nsattr |= NVME_NS_ATTR_RO;
 	if (nvmet_ns_bdev(req->ns) && bdev_rot(nvmet_ns_bdev(req->ns)))
 		id->nsfeat |= NVME_NS_ROTATIONAL;
@@ -1255,13 +1255,13 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req)
 	mutex_lock(&req->ns->subsys->lock);
 	switch (write_protect) {
 	case NVME_NS_WRITE_PROTECT:
-		req->ns->readonly = true;
+		WRITE_ONCE(req->ns->readonly, true);
 		status = nvmet_write_protect_flush_sync(req);
 		if (status)
-			req->ns->readonly = false;
+			WRITE_ONCE(req->ns->readonly, false);
 		break;
 	case NVME_NS_NO_WRITE_PROTECT:
-		req->ns->readonly = false;
+		WRITE_ONCE(req->ns->readonly, false);
 		status = 0;
 		break;
 	default:
@@ -1451,20 +1451,19 @@ void nvmet_execute_set_features(struct nvmet_req *req)
 
 static u16 nvmet_get_feat_write_protect(struct nvmet_req *req)
 {
-	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 	u32 result;
 
 	result = nvmet_req_find_ns(req);
 	if (result)
 		return result;
 
-	mutex_lock(&subsys->lock);
+	mutex_lock(&req->ns->subsys->lock);
 	if (req->ns->readonly == true)
 		result = NVME_NS_WRITE_PROTECT;
 	else
 		result = NVME_NS_NO_WRITE_PROTECT;
 	nvmet_set_result(req, result);
-	mutex_unlock(&subsys->lock);
+	mutex_unlock(&req->ns->subsys->lock);
 
 	return 0;
 }
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index ef99c45f0f37..116b1b500790 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1090,7 +1090,7 @@ static inline u16 nvmet_check_ana_state(struct nvmet_port *port,
 
 static inline u16 nvmet_io_cmd_check_access(struct nvmet_req *req)
 {
-	if (unlikely(req->ns->readonly)) {
+	if (unlikely(nvmet_ns_readonly(req->ns))) {
 		switch (req->cmd->common.opcode) {
 		case nvme_cmd_read:
 		case nvme_cmd_flush:
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index b63664250ef4..b76671672aca 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -155,7 +155,8 @@ struct nvmet_ns {
 		__guarded_by(&subsys->lock);
 	struct file		*file
 		__guarded_by(&subsys->lock);
-	bool			readonly;
+	bool			readonly
+		__guarded_by(&subsys->lock);
 	u32			nsid;
 	u32			blksize_shift;
 	loff_t			size;
@@ -205,6 +206,19 @@ static inline _type nvmet_ns_##_name(struct nvmet_ns *ns)		    \
 NVMET_NS_ACCESSOR(struct block_device *, bdev)
 NVMET_NS_ACCESSOR(struct file *, file)
 
+/*
+ * These accessors use data_race() and READ_ONCE() to allow lockless access to
+ * the corresponding fields. Use them only in code paths where a concurrent
+ * update to the field is harmless and the resulting data race is acceptable.
+ */
+#define NVMET_NS_ACCESSOR_RACE(_type, _name)				\
+static inline _type nvmet_ns_##_name(struct nvmet_ns *ns)		\
+{									\
+	return data_race(READ_ONCE(ns->_name));				\
+}									\
+
+NVMET_NS_ACCESSOR_RACE(bool, readonly)
+
 static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
 {
 	return container_of(to_config_group(item), struct nvmet_ns, group);
-- 
2.53.0