[PATCH 09/23] nvmet: add context annotation for nvmet_ns::size

[Nilay Shroff]

The nvmet_ns::size 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 ns->size without holding subsys->lock.
Unlike ns->bdev, which cannot be updated while I/O is in flight due to
the namespace reference, ns->size can be modified concurrently by
nvmet_ns_revalidate_size_store() or nvmet_file_ns_revalidate().
Consequently, lockless reads may observe a transient value of ns->size,
which is acceptable for these access paths.

Introduce nvmet_ns_size() to perform these lockless reads using
data_race(READ_ONCE()) which would silence KCSAN splat as well as help
avoid clang context analyser warning. Use this accessor at the relevant
call sites where accessing ns->size without holding subsys->lock is
harmless despite concurrent updates.

Pair these lockless reads with WRITE_ONCE() for all updates to ns->size.
Also use READ_ONCE() in nvmet_ns_revalidate() when reading the old and
new size values to detect changes.

Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
---
 drivers/nvme/target/admin-cmd.c   |  3 ++-
 drivers/nvme/target/core.c        |  4 ++--
 drivers/nvme/target/io-cmd-bdev.c |  4 ++--
 drivers/nvme/target/io-cmd-file.c | 10 +++++-----
 drivers/nvme/target/nvmet.h       |  4 +++-
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 4302dba7c000..7cdf13351678 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -891,7 +891,8 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	 * that out from the underlying device.
 	 */
 	id->ncap = id->nsze =
-		cpu_to_le64(req->ns->size >> nvmet_ns_blksize_shift(req->ns));
+		cpu_to_le64(nvmet_ns_size(req->ns) >>
+				nvmet_ns_blksize_shift(req->ns));
 	switch (req->port->ana_state[req->ns->anagrpid]) {
 	case NVME_ANA_INACCESSIBLE:
 	case NVME_ANA_PERSISTENT_LOSS:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 2913ed76bc6a..77e2ef211711 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -591,14 +591,14 @@ static void nvmet_p2pmem_ns_add_p2p(struct nvmet_ctrl *ctrl,
 
 bool nvmet_ns_revalidate(struct nvmet_ns *ns)
 {
-	loff_t oldsize = ns->size;
+	loff_t oldsize = READ_ONCE(ns->size);
 
 	if (ns->bdev)
 		nvmet_bdev_ns_revalidate(ns);
 	else
 		nvmet_file_ns_revalidate(ns);
 
-	return oldsize != ns->size;
+	return oldsize != READ_ONCE(ns->size);
 }
 
 int nvmet_ns_enable(struct nvmet_ns *ns)
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index e950b442ffc6..0ed691387f1c 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -115,7 +115,7 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
 		return ret;
 	}
 	ns->bdev = file_bdev(ns->bdev_file);
-	ns->size = bdev_nr_bytes(ns->bdev);
+	WRITE_ONCE(ns->size, bdev_nr_bytes(ns->bdev));
 	ns->blksize_shift = blksize_bits(bdev_logical_block_size(ns->bdev));
 
 	ns->pi_type = 0;
@@ -136,7 +136,7 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
 
 void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns)
 {
-	ns->size = bdev_nr_bytes(ns->bdev);
+	WRITE_ONCE(ns->size, bdev_nr_bytes(ns->bdev));
 }
 
 u16 blk_to_nvme_status(struct nvmet_req *req, blk_status_t blk_sts)
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 90dbfbd16978..224b7423b780 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -15,7 +15,7 @@
 
 void nvmet_file_ns_revalidate(struct nvmet_ns *ns)
 {
-	ns->size = i_size_read(ns->file->f_mapping->host);
+	WRITE_ONCE(ns->size, i_size_read(ns->file->f_mapping->host));
 }
 
 void nvmet_file_ns_disable(struct nvmet_ns *ns)
@@ -68,7 +68,7 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
 err:
 	fput(ns->file);
 	ns->file = NULL;
-	ns->size = 0;
+	WRITE_ONCE(ns->size, 0);
 	ns->blksize_shift = 0;
 	return ret;
 }
@@ -132,7 +132,7 @@ static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 		is_sync = true;
 
 	pos = le64_to_cpu(req->cmd->rw.slba) << nvmet_ns_blksize_shift(req->ns);
-	if (unlikely(pos + req->transfer_len > req->ns->size)) {
+	if (unlikely(pos + req->transfer_len > nvmet_ns_size(req->ns))) {
 		ret = -ENOSPC;
 		goto complete;
 	}
@@ -301,7 +301,7 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
 				nvmet_ns_blksize_shift(req->ns);
 		len = le32_to_cpu(range.nlb);
 		len <<= nvmet_ns_blksize_shift(req->ns);
-		if (offset + len > req->ns->size) {
+		if (offset + len > nvmet_ns_size(req->ns)) {
 			req->error_slba = le64_to_cpu(range.slba);
 			status = errno_to_nvme_status(req, -ENOSPC);
 			break;
@@ -357,7 +357,7 @@ static void nvmet_file_write_zeroes_work(struct work_struct *w)
 	len = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
 			nvmet_ns_blksize_shift(req->ns));
 
-	if (unlikely(offset + len > req->ns->size)) {
+	if (unlikely(offset + len > nvmet_ns_size(req->ns))) {
 		nvmet_req_complete(req, errno_to_nvme_status(req, -ENOSPC));
 		return;
 	}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 1ea0566b8eea..59adf99470f8 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -161,7 +161,8 @@ struct nvmet_ns {
 		__guarded_by(&subsys->lock);
 	u32			blksize_shift
 		__guarded_by(&subsys->lock);
-	loff_t			size;
+	loff_t			size
+		__guarded_by(&subsys->lock);
 	u8			nguid[16];
 	uuid_t			uuid;
 	u32			anagrpid;
@@ -222,6 +223,7 @@ static inline _type nvmet_ns_##_name(struct nvmet_ns *ns)		\
 }									\
 
 NVMET_NS_ACCESSOR_RACE(bool, readonly)
+NVMET_NS_ACCESSOR_RACE(loff_t, size)
 
 static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
 {
-- 
2.53.0