drivers: nvme: target: core: Pass lockdep expression to RCU lists
Subsystems (2)
PATCH 1/1 drivers: nvme: target: core: Pass lockdep expression to RCU lists
ctrl->subsys->namespaces and subsys->namespaces are traversed with hlist_for_each_entry_rcu outside an RCU read-side critical section but under the protection of subsys->lock.
Hence, add the corresponding lockdep expression to the list traversal primitive to silence false-positive lockdep warnings, and harden RCU lists.
Add macro for the corresponding lockdep expression to make the code clean and concise.
--- drivers/nvme/target/core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/nvme/target/core.c +7 -2
@@ -15,6 +15,9 @@#include "nvmet.h" +#define subsys_lock_held () \ + lockdep_is_held(&subsys->lock) +struct workqueue_struct *buffered_io_wq;static const struct nvmet_fabrics_ops *nvmet_transports[NVMF_TRTYPE_MAX ];static DEFINE_IDA (cntlid_ida);
@@ -555,7 +558,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) }else {struct nvmet_ns *old; -list_for_each_entry_rcu (old, &subsys->namespaces, dev_link) { +list_for_each_entry_rcu (old, &subsys->namespaces, dev_link, +subsys_lock_held ()) {BUG_ON (ns->nsid == old->nsid);if (ns->nsid < old->nsid)break ;
@@ -1172,7 +1176,8 @@ static void nvmet_setup_p2p_ns_map(struct nvmet_ctrl *ctrl, ctrl->p2p_client =get_device (req->p2p_client); -list_for_each_entry_rcu (ns, &ctrl->subsys->namespaces, dev_link) +list_for_each_entry_rcu (ns, &ctrl->subsys->namespaces, dev_link, +subsys_lock_held ())nvmet_p2pmem_ns_add_p2p (ctrl, ns); }
Signature · 6 lines
2.24.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 10 lines · Amol Grover
ctrl->subsys->namespacesandsubsys->namespacesare traversed withhlist_for_each_entry_rcuoutside an RCU read-side critical section but under the protection ofsubsys->lock.Hence, add the corresponding lockdep expression to the list traversal primitive to silence false-positive lockdep warnings, and harden RCU lists.
Add macro for the corresponding lockdep expression to make the code clean and concise.
Amol, thanks. Could you fix this checkpatch warnings? hint: Use --fix-inplace
CHECK: Alignment should match open parenthesis
#50: FILE: drivers/nvme/target/core.c:562:
+ list_for_each_entry_rcu(old, &subsys->namespaces, dev_link,
+ subsys_lock_held()) {
CHECK: Alignment should match open parenthesis
#60: FILE: drivers/nvme/target/core.c:1180:
+ list_for_each_entry_rcu(ns, &ctrl->subsys->namespaces, dev_link,
+ subsys_lock_held())
Otherwise,
thanks,
- Joel
Quoted 43 lines
Signed-off-by Amol Grover <[email protected]>--- drivers/nvme/target/core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/nvme/target/core.c +7
-2@@ -15,6 +15,9 @@#include "nvmet.h" +#define subsys_lock_held () \ + lockdep_is_held(&subsys->lock) +struct workqueue_struct *buffered_io_wq;static const struct nvmet_fabrics_ops *nvmet_transports[NVMF_TRTYPE_MAX ];static DEFINE_IDA (cntlid_ida);@@ -555,7 +558,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) }else {struct nvmet_ns *old; -list_for_each_entry_rcu (old, &subsys->namespaces, dev_link) { +list_for_each_entry_rcu (old, &subsys->namespaces, dev_link, +subsys_lock_held ()) {BUG_ON (ns->nsid == old->nsid);if (ns->nsid < old->nsid)break ;@@ -1172,7 +1176,8 @@ static void nvmet_setup_p2p_ns_map(struct nvmet_ctrl *ctrl, ctrl->p2p_client =get_device (req->p2p_client); -list_for_each_entry_rcu (ns, &ctrl->subsys->namespaces, dev_link) +list_for_each_entry_rcu (ns, &ctrl->subsys->namespaces, dev_link, +subsys_lock_held ())nvmet_p2pmem_ns_add_p2p (ctrl, ns); }Signature · 1 line
2.24.1
Keith Busch
maintainerdrivers/nvme/target/core.c line 19
Quoted 2 lines · Amol Grover
drivers/nvme/target/core.c +2
-0+#define subsys_lock_held () \ + lockdep_is_held(&subsys->lock)
This macro requires "struct nvmet_subsys *subsys" was previously declared in the function using it, which isn't obvious when looking at the users. I don't think that's worth the conciseness.
Quoted 10 lines
drivers/nvme/target/core.c +2
-1@@ -555,7 +558,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) }else {struct nvmet_ns *old; -list_for_each_entry_rcu (old, &subsys->namespaces, dev_link) { +list_for_each_entry_rcu (old, &subsys->namespaces, dev_link, +subsys_lock_held ()) {BUG_ON (ns->nsid == old->nsid);if (ns->nsid < old->nsid)break ;
Quoted 8 lines · Keith Busch
drivers/nvme/target/core.c line 19
Quoted 2 lines · Amol Grover
drivers/nvme/target/core.c +2
-0+#define subsys_lock_held () \ + lockdep_is_held(&subsys->lock)This macro requires "struct
nvmet_subsys*subsys" was previously declared in the function using it, which isn't obvious when looking at the users. I don't think that's worth the conciseness.
Hey Keith,
If I understand correctly, you're saying *subsys is always declared in the function using it, right? I too think, this could cause confusion to the person going through the code, I'll fix it right away.
Thanks
Amol
Quoted 10 lines
Quoted 10 lines
drivers/nvme/target/core.c +2
-1@@ -555,7 +558,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns) }else {struct nvmet_ns *old; -list_for_each_entry_rcu (old, &subsys->namespaces, dev_link) { +list_for_each_entry_rcu (old, &subsys->namespaces, dev_link, +subsys_lock_held ()) {BUG_ON (ns->nsid == old->nsid);if (ns->nsid < old->nsid)break ;