Skip to contentSkip to conversation

DLM: Don't save callbacks after accept

PATCH New Bob Peterson 1 participant 2 messages Sep 23, 2016 – Oct 19, 2016 1 message not in the archive Distributed lock manager (DLM)

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Bob Peterson

author
#1 lore raw

PATCH 1/1 DLM: Don't save callbacks after accept

Hi,

When DLM calls accept() on a socket, the comm code copies the sk after we've saved its callbacks. Afterward, it calls add_sock which saves the callbacks a second time. Since the error reporting function lowcomms_error_report calls the previous callback too, this results in a recursive call to itself. This patch adds a new parameter to function add_sock to tell whether to save the callbacks. Function tcp_accept_from_sock (and its sctp counterpart) then calls it with false to avoid the recursion.

Signed-off-by Bob Peterson <[email protected]>

---

fs/dlm/lowcomms.c +12 -9
index 963016c..948575a 100644
@@ -541,7 +541,7 @@ static void restore_callbacks(struct connection *con, struct sock *sk)
 }
 
 /* Make a socket active */
-static void add_sock(struct socket *sock, struct connection *con)
+static void add_sock(struct socket *sock, struct connection *con, bool save_cb)
 {
 	struct sock *sk = sock->sk;
 

@@ -549,7 +549,7 @@ static void add_sock(struct socket *sock, struct connection *con)
 	con->sock = sock;
 
 	sk->sk_user_data = con;
-	if (!test_bit(CF_IS_OTHERCON, &con->flags))
+	if (save_cb)
 		save_callbacks(con, sk);
 	/* Install a data_ready callback */
 	sk->sk_data_ready = lowcomms_data_ready;

@@ -806,7 +806,7 @@ static int tcp_accept_from_sock(struct connection *con)
 			newcon->othercon = othercon;
 			othercon->sock = newsock;
 			newsock->sk->sk_user_data = othercon;
-			add_sock(newsock, othercon);
+			add_sock(newsock, othercon, false);
 			addcon = othercon;
 		}
 		else {

@@ -819,7 +819,10 @@ static int tcp_accept_from_sock(struct connection *con)
 	else {
 		newsock->sk->sk_user_data = newcon;
 		newcon->rx_action = receive_from_sock;
-		add_sock(newsock, newcon);
+		/* accept copies the sk after we've saved the callbacks, so we
+		   don't want to save them a second time or comm errors will
+		   result in calling sk_error_report recursively. */
+		add_sock(newsock, newcon, false);
 		addcon = newcon;
 	}
 

@@ -919,7 +922,7 @@ static int sctp_accept_from_sock(struct connection *con)
 			newcon->othercon = othercon;
 			othercon->sock = newsock;
 			newsock->sk->sk_user_data = othercon;
-			add_sock(newsock, othercon);
+			add_sock(newsock, othercon, false);
 			addcon = othercon;
 		} else {
 			printk("Extra connection from node %d attempted\n", nodeid);

@@ -930,7 +933,7 @@ static int sctp_accept_from_sock(struct connection *con)
 	} else {
 		newsock->sk->sk_user_data = newcon;
 		newcon->rx_action = receive_from_sock;
-		add_sock(newsock, newcon);
+		add_sock(newsock, newcon, false);
 		addcon = newcon;
 	}
 

@@ -1058,7 +1061,7 @@ static void sctp_connect_to_sock(struct connection *con)
 	sock->sk->sk_user_data = con;
 	con->rx_action = receive_from_sock;
 	con->connect_action = sctp_connect_to_sock;
-	add_sock(sock, con);
+	add_sock(sock, con, true);
 
 	/* Bind to all addresses. */
 	if (sctp_bind_addrs(con, 0))

@@ -1146,7 +1149,7 @@ static void tcp_connect_to_sock(struct connection *con)
 	sock->sk->sk_user_data = con;
 	con->rx_action = receive_from_sock;
 	con->connect_action = tcp_connect_to_sock;
-	add_sock(sock, con);
+	add_sock(sock, con, true);
 
 	/* Bind to our cluster-known address connecting to avoid
 	   routing problems */

@@ -1366,7 +1369,7 @@ static int tcp_listen_for_all(void)
 
 	sock = tcp_create_listen_sock(con, dlm_local_addr[0]);
 	if (sock) {
-		add_sock(sock, con);
+		add_sock(sock, con, true);
 		result = 0;
 	}
 	else {
author
#2 lore raw

PATCH 1/1 DLM: Don't save callbacks after accept

----- Original Message -----

| Hi,
| 
| When DLM calls accept() on a socket, the comm code copies the sk
| after we've saved its callbacks. Afterward, it calls add_sock which
| saves the callbacks a second time. Since the error reporting function
| lowcomms_error_report calls the previous callback too, this results
| in a recursive call to itself. This patch adds a new parameter to
| function add_sock to tell whether to save the callbacks. Function
| tcp_accept_from_sock (and its sctp counterpart) then calls it with
| false to avoid the recursion.
| 
| Signed-off-by: Bob Peterson <[email protected]>
| ---

Hi Dave,

Any word on this patch from 23 September?

Regards,

Bob Peterson
Red Hat File Systems