Hi,
On 10/02/16 18:55, Bob Peterson wrote:
Before this patch, DLM was saving off the original error report
callback before setting its own, but it never restored it. Instead,
we should be saving off all four socket callbacks before changing
them, and then restore them once we're done.
Signed-off-by: Bob Peterson <rpete...@redhat.com>
---
fs/dlm/lowcomms.c | 40 +++++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 4e82285..c196c16 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -124,7 +124,10 @@ struct connection {
struct connection *othercon;
struct work_struct rwork; /* Receive workqueue */
struct work_struct swork; /* Send workqueue */
- void (*orig_error_report)(struct sock *sk);
+ void (*orig_error_report)(struct sock *);
+ void (*orig_data_ready)(struct sock *);
+ void (*orig_state_change)(struct sock *);
+ void (*orig_write_space)(struct sock *);
};
#define sock2con(x) ((struct connection *)(x)->sk_user_data)
@@ -513,6 +516,34 @@ out:
orig_report(sk);
}
+/* Note: sk_callback_lock must be locked before calling this function. */
+static void save_callbacks(struct connection *con, struct sock *sk)
+{
+ if (test_bit(CF_IS_OTHERCON, &con->flags))
+ return;
+ lock_sock(sk);
+ con->orig_data_ready = sk->sk_data_ready;
+ con->orig_state_change = sk->sk_state_change;
+ con->orig_write_space = sk->sk_write_space;
+ con->orig_error_report = sk->sk_error_report;
+ release_sock(sk);
+}
+
+static void restore_callbacks(struct connection *con, struct sock *sk)
+{
+ if (test_bit(CF_IS_OTHERCON, &con->flags))
+ return;
+ write_lock_bh(&sk->sk_callback_lock);
+ lock_sock(sk);
+ sk->sk_user_data = NULL;
+ sk->sk_data_ready = con->orig_data_ready;
+ sk->sk_state_change = con->orig_state_change;
+ sk->sk_write_space = con->orig_write_space;
+ sk->sk_error_report = con->orig_error_report;
+ release_sock(sk);
+ write_unlock_bh(&sk->sk_callback_lock);
+}
+
Might be clearer to move the test for CF_IS_OTHERCON outside of these
functions and into the callers?
Otherwise these patches look like a good set of clean ups,
Steve.
/* Make a socket active */
static void add_sock(struct socket *sock, struct connection *con)
{
@@ -521,13 +552,13 @@ static void add_sock(struct socket *sock, struct
connection *con)
write_lock_bh(&sk->sk_callback_lock);
con->sock = sock;
+ sk->sk_user_data = con;
+ save_callbacks(con, sk);
/* Install a data_ready callback */
sk->sk_data_ready = lowcomms_data_ready;
sk->sk_write_space = lowcomms_write_space;
sk->sk_state_change = lowcomms_state_change;
- sk->sk_user_data = con;
sk->sk_allocation = GFP_NOFS;
- con->orig_error_report = sk->sk_error_report;
sk->sk_error_report = lowcomms_error_report;
write_unlock_bh(&sk->sk_callback_lock);
}
@@ -564,6 +595,7 @@ static void close_connection(struct connection *con, bool
and_other,
mutex_lock(&con->sock_mutex);
if (con->sock) {
+ restore_callbacks(con, con->sock->sk);
sock_release(con->sock);
con->sock = NULL;
}
@@ -1205,6 +1237,8 @@ static struct socket *tcp_create_listen_sock(struct
connection *con,
if (result < 0) {
log_print("Failed to set SO_REUSEADDR on socket: %d", result);
}
+ sock->sk->sk_user_data = con;
+
con->rx_action = tcp_accept_from_sock;
con->connect_action = tcp_connect_to_sock;
