All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nilay Shroff <nilay@linux.ibm.com>
To: kbusch@kernel.org, hch@lst.de, sagi@grimberg.me, axboe@kernel.dk,
	chaitanyak@nvidia.com
Cc: gjoyce@linux.ibm.com, bvanassche@acm.org, elver@google.com,
	linux-nvme@lists.infradead.org,
	Nilay Shroff <nilay@linux.ibm.com>
Subject: [PATCH 07/23] nvmet: fix nvmet_ns::nsid access using nvmet_subsys::lock
Date: Mon,  9 Nov 2026 08:58:12 +0530	[thread overview]
Message-ID: <20261109033102.982191-32-nilay@linux.ibm.com> (raw)
In-Reply-To: <20261109033102.982191-25-nilay@linux.ibm.com>

nvmet_ns::nsid is protected by nvmet_subsys::lock. However, the Clang
context analyzer reports errors in nvmet_format_ana_group(),
nvmet_execute_identify_endgrp_list() and nvmet_execute_identify_nslist()
because these functions access ns->nsid while iterating over enabled
namespaces without holding the corresponding lock:

drivers/nvme/target/admin-cmd.c:493:44: error: reading variable 'nsid' requires holding mutex 'ns->subsys->lock'
      [-Werror,-Wthread-safety-precise]
  493 |                                 desc->nsids[count++] = cpu_to_le32(ns->nsid);
      |                                                                        ^

drivers/nvme/target/admin-cmd.c:955:11: error: reading variable 'nsid' requires holding mutex 'ns->subsys->lock'
      [-Werror,-Wthread-safety-analysis]
  955 |                 if (ns->nsid <= min_endgid)
      |                         ^

drivers/nvme/target/admin-cmd.c:1002:31: error: reading variable 'nsid' requires holding mutex 'ns->subsys->lock'
      [-Werror,-Wthread-safety-analysis]
 1002 |                 list[i++] = cpu_to_le32(ns->nsid);

Fix above errors, acquiring ctrl->subsys->lock while iterating over
enabled namespaces in these functions to protect accesses to namespace
fields.

Since the context analyzer cannot infer that ctrl->subsys->lock and
ns->subsys->lock refer to the same mutex, explicitly assert the
ns->subsys->lock capability using __assume_ctx_lock() inside each
iteration.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/target/admin-cmd.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 590ca68588f8..761caac84255 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -512,10 +512,19 @@ static u32 nvmet_format_ana_group(struct nvmet_req *req, u32 grpid,
 	u32 count = 0;
 
 	if (!(req->cmd->get_log_page.lsp & NVME_ANA_LOG_RGO)) {
+		mutex_lock(&ctrl->subsys->lock);
 		nvmet_for_each_enabled_ns(&ctrl->subsys->namespaces, idx, ns) {
+			/*
+			 * Both ctrl->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);
 			if (ns->anagrpid == grpid)
 				desc->nsids[count++] = cpu_to_le32(ns->nsid);
 		}
+		mutex_unlock(&ctrl->subsys->lock);
 	}
 
 	desc->grpid = cpu_to_le32(grpid);
@@ -966,7 +975,16 @@ static void nvmet_execute_identify_endgrp_list(struct nvmet_req *req)
 		goto out;
 	}
 
+	mutex_lock(&ctrl->subsys->lock);
 	nvmet_for_each_enabled_ns(&ctrl->subsys->namespaces, idx, ns) {
+		/*
+		 * Both ctrl->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);
+
 		if (ns->nsid <= min_endgid)
 			continue;
 
@@ -974,6 +992,7 @@ static void nvmet_execute_identify_endgrp_list(struct nvmet_req *req)
 		if (i == buf_size / sizeof(__le16))
 			break;
 	}
+	mutex_unlock(&ctrl->subsys->lock);
 
 	list[0] = cpu_to_le16(i - 1);
 	status = nvmet_copy_to_sgl(req, 0, list, buf_size);
@@ -1009,7 +1028,16 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req, bool match_css)
 		goto out;
 	}
 
+	mutex_lock(&ctrl->subsys->lock);
 	nvmet_for_each_enabled_ns(&ctrl->subsys->namespaces, idx, ns) {
+		/*
+		 * Both ctrl->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);
+
 		if (ns->nsid <= min_nsid)
 			continue;
 		if (match_css && ns->csi != req->cmd->identify.csi)
@@ -1018,6 +1046,7 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req, bool match_css)
 		if (i == buf_size / sizeof(__le32))
 			break;
 	}
+	mutex_unlock(&ctrl->subsys->lock);
 
 	status = nvmet_copy_to_sgl(req, 0, list, buf_size);
 
-- 
2.53.0



  parent reply	other threads:[~2026-11-09  3:32 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  3:28 [PATCH 00/23] nvmet: add context annotation for params in struct nvmet_ns Nilay Shroff
2026-11-09  3:28 ` [PATCH 01/23] nvmet: add context annotation for nvmet_ns::bdev Nilay Shroff
2026-11-09 20:09   ` sashiko-bot
2026-11-09 21:46   ` Bart Van Assche
2026-11-10  6:38     ` Nilay Shroff
2026-11-10 17:29       ` Bart Van Assche
2026-11-11  7:25         ` Nilay Shroff
2026-11-11 18:00           ` Bart Van Assche
2026-11-12  9:04             ` Nilay Shroff
2026-11-12 17:35               ` Bart Van Assche
2026-11-09  3:28 ` [PATCH 02/23] nvmet: fix nvmet_ns::bdev access using nvmet_subsys::lock Nilay Shroff
2026-11-09 20:15   ` sashiko-bot
2026-11-09 21:47   ` Bart Van Assche
2026-11-10  6:49     ` Nilay Shroff
2026-11-09  3:28 ` [PATCH 03/23] nvmet: add context annotation for nvmet_ns::file Nilay Shroff
2026-11-09 20:20   ` sashiko-bot
2026-11-09 21:53   ` Bart Van Assche
2026-11-10  7:12     ` Nilay Shroff
2026-11-09  3:28 ` [PATCH 04/23] nvmet: add context annotation for nvmet_ns::bdev_file Nilay Shroff
2026-11-09  3:28 ` [PATCH 05/23] nvmet: add context annotation for nvmet_ns::readonly Nilay Shroff
2026-11-09  3:28 ` [PATCH 06/23] nvmet: add context annotation for nvmet_ns::nsid Nilay Shroff
2026-11-09 21:56   ` Bart Van Assche
2026-11-10  8:50     ` Nilay Shroff
2026-11-09  3:28 ` Nilay Shroff [this message]
2026-11-09  3:28 ` [PATCH 08/23] nvmet: add context annotation for nvmet_ns::blksize_shift Nilay Shroff
2026-11-09  3:28 ` [PATCH 09/23] nvmet: add context annotation for nvmet_ns::size Nilay Shroff
2026-11-09  3:28 ` [PATCH 10/23] nvmet: add context annotation for nvmet_ns::nguid Nilay Shroff
2026-11-09  3:28 ` [PATCH 11/23] nvmet: add context annotation for nvmet_ns::uuid Nilay Shroff
2026-11-09 21:58   ` Bart Van Assche
2026-11-10  9:01     ` Nilay Shroff
2026-11-09  3:28 ` [PATCH 12/23] nvmet: fix nvmet_ns::uuid access using nvmet_subsys::lock Nilay Shroff
2026-11-09  3:28 ` [PATCH 13/23] nvmet: add context annotation for nvmet_ns::buffered_io Nilay Shroff
2026-11-09  3:28 ` [PATCH 14/23] nvmet: fix nvmet_ns::buffered_io access using nvmet_subsys::lock Nilay Shroff
2026-11-09  3:28 ` [PATCH 15/23] nvmet: add context annotation for nvmet_ns::enabled Nilay Shroff
2026-11-09  3:28 ` [PATCH 16/23] nvmet: fix access to nvmet_ns::enable using nvmet_subsys::lock Nilay Shroff
2026-11-09  3:28 ` [PATCH 17/23] nvmet: add context annotation for nvmet_ns::device_path Nilay Shroff
2026-11-09  3:28 ` [PATCH 18/23] nvmet: fix nvmet_ns::device_path access using nvmet_subsys::lock Nilay Shroff
2026-11-09 21:57   ` sashiko-bot
2026-11-09  3:28 ` [PATCH 19/23] nvmet: add context annotation for PCI P2P parameters in struct nvmet_ns Nilay Shroff
2026-11-09  3:28 ` [PATCH 20/23] nvmet: fix PCI P2P parameters access using nvmet_subsys::lock Nilay Shroff
2026-11-09 22:12   ` sashiko-bot
2026-11-09  3:28 ` [PATCH 21/23] nvmet: add context annotation for integrity params in struct nvmet_ns Nilay Shroff
2026-11-09  3:28 ` [PATCH 22/23] nvmet: add context annotation for nvmet_ns::csi Nilay Shroff
2026-11-09  3:28 ` [PATCH 23/23] nvmet: add context annotation for nvmet_ns::debugfs_dir Nilay Shroff

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261109033102.982191-32-nilay@linux.ibm.com \
    --to=nilay@linux.ibm.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=chaitanyak@nvidia.com \
    --cc=elver@google.com \
    --cc=gjoyce@linux.ibm.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.