1. Patchew
  2. linux
  3. View series

[PATCH] dlm: use hlist_for_each_entry_srcu for SRCU protected lists

lirongqing posted 1 patch 1 day, 12 hours ago 
fs/dlm/lowcomms.c | 12 ++++++++----
fs/dlm/midcomms.c | 15 ++++++++++-----
2 files changed, 18 insertions(+), 9 deletions(-)
[PATCH] dlm: use hlist_for_each_entry_srcu for SRCU protected lists
Posted by lirongqing 1 day, 12 hours ago 
From: Li RongQing <lirongqing@baidu.com>

The connection and node hash tables in DLM are protected by SRCU, but
the code currently uses hlist_for_each_entry_rcu() for traversal.
While this works functionally, it is semantically incorrect and triggers
warnings when RCU lockdep debugging is enabled, as it expects regular
RCU read-side critical sections.

This patch replaces the incorrect macros with hlist_for_each_entry_srcu()
and adds the appropriate lockdep expressions using srcu_read_lock_held()
to ensure consistency with the underlying locking mechanism.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 fs/dlm/lowcomms.c | 12 ++++++++----
 fs/dlm/midcomms.c | 15 ++++++++++-----
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index b395800..59e91a6 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -271,7 +271,8 @@ static struct connection *__find_con(int nodeid, int r)
 {
 	struct connection *con;
 
-	hlist_for_each_entry_rcu(con, &connection_hash[r], list) {
+	hlist_for_each_entry_srcu(con, &connection_hash[r], list,
+			srcu_read_lock_held(&connections_srcu)) {
 		if (con->nodeid == nodeid)
 			return con;
 	}
@@ -426,7 +427,8 @@ static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid,
 
 	idx = srcu_read_lock(&connections_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(con, &connection_hash[i], list) {
+		hlist_for_each_entry_srcu(con, &connection_hash[i], list,
+				srcu_read_lock_held(&connections_srcu)) {
 			WARN_ON_ONCE(!con->addr_count);
 
 			spin_lock(&con->addrs_lock);
@@ -1729,7 +1731,8 @@ void dlm_lowcomms_shutdown(void)
 
 	idx = srcu_read_lock(&connections_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(con, &connection_hash[i], list) {
+		hlist_for_each_entry_srcu(con, &connection_hash[i], list,
+				srcu_read_lock_held(&connections_srcu)) {
 			shutdown_connection(con, true);
 			stop_connection_io(con);
 			flush_workqueue(process_workqueue);
@@ -1968,7 +1971,8 @@ void dlm_lowcomms_exit(void)
 
 	idx = srcu_read_lock(&connections_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(con, &connection_hash[i], list) {
+		hlist_for_each_entry_srcu(con, &connection_hash[i], list,
+				srcu_read_lock_held(&connections_srcu)) {
 			spin_lock(&connections_lock);
 			hlist_del_rcu(&con->list);
 			spin_unlock(&connections_lock);
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 2c101bb..e08765b 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -275,7 +275,8 @@ static struct midcomms_node *__find_node(int nodeid, int r)
 {
 	struct midcomms_node *node;
 
-	hlist_for_each_entry_rcu(node, &node_hash[r], hlist) {
+	hlist_for_each_entry_srcu(node, &node_hash[r], hlist,
+			srcu_read_lock_held(&nodes_srcu)) {
 		if (node->nodeid == nodeid)
 			return node;
 	}
@@ -1165,7 +1166,8 @@ void dlm_midcomms_exit(void)
 
 	idx = srcu_read_lock(&nodes_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+		hlist_for_each_entry_srcu(node, &node_hash[i], hlist,
+				srcu_read_lock_held(&nodes_srcu)) {
 			dlm_delete_debug_comms_file(node->debugfs);
 
 			spin_lock(&nodes_lock);
@@ -1325,7 +1327,8 @@ void dlm_midcomms_version_wait(void)
 
 	idx = srcu_read_lock(&nodes_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+		hlist_for_each_entry_srcu(node, &node_hash[i], hlist,
+				srcu_read_lock_held(&nodes_srcu)) {
 			ret = wait_event_timeout(node->shutdown_wait,
 						 node->version != DLM_VERSION_NOT_SET ||
 						 node->state == DLM_CLOSED ||
@@ -1396,7 +1399,8 @@ void dlm_midcomms_shutdown(void)
 	mutex_lock(&close_lock);
 	idx = srcu_read_lock(&nodes_srcu);
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+		hlist_for_each_entry_srcu(node, &node_hash[i], hlist,
+				srcu_read_lock_held(&nodes_srcu)) {
 			midcomms_shutdown(node);
 		}
 	}
@@ -1404,7 +1408,8 @@ void dlm_midcomms_shutdown(void)
 	dlm_lowcomms_shutdown();
 
 	for (i = 0; i < CONN_HASH_SIZE; i++) {
-		hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+		hlist_for_each_entry_srcu(node, &node_hash[i], hlist,
+				srcu_read_lock_held(&nodes_srcu)) {
 			midcomms_node_reset(node);
 		}
 	}
-- 
2.9.4
Re: [PATCH] dlm: use hlist_for_each_entry_srcu for SRCU protected lists
Posted by Alexander Aring 1 day, 2 hours ago 
Hi,

On Fri, Feb 6, 2026 at 12:19 AM lirongqing <lirongqing@baidu.com> wrote:
>
> From: Li RongQing <lirongqing@baidu.com>
>
> The connection and node hash tables in DLM are protected by SRCU, but
> the code currently uses hlist_for_each_entry_rcu() for traversal.
> While this works functionally, it is semantically incorrect and triggers
> warnings when RCU lockdep debugging is enabled, as it expects regular
> RCU read-side critical sections.
>

I noticed this as well and came to the same conclusion that it doesn't
matter, but I do agree we should change that because the API can be
changed even in the non debug case.

So thanks for doing this...

Acked-by: Alexander Aring <aahringo@redhat.com>

- Alex

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.