Commit cc661fc9 authored by Bob Peterson's avatar Bob Peterson Committed by David Teigland

DLM: Fix saving of NULL callbacks

In a previous patch I noted that accept() often copies the struct
sock (sk) which overwrites the sock callbacks. However, in testing
we discovered that the dlm connection structures (con) are sometimes
deleted and recreated as connections come and go, and since they're
zeroed out by kmem_cache_zalloc, the saved callback pointers are
also initialized to zero. But with today's DLM code, the callbacks
are only saved when a socket is added.

During recovery testing, we discovered a common situation in which
the new con is initialized to zero, then a socket is added after
accept(). In this case, the sock's saved values are all NULL, but
the saved values are wiped out, due to accept(). Therefore, we
don't have a known good copy of the callbacks from which we can
restore.

Since the struct sock callbacks are always good after listen(),
this patch saves the known good values after listen(). These good
values are then used for subsequent restores.
Signed-off-by: default avatarBob Peterson <rpeterso@redhat.com>
Reviewed-by: default avatarTadashi Miyauchi <miyauchi@toshiba-tops.co.jp>
Signed-off-by: default avatarDavid Teigland <teigland@redhat.com>
parent 01da24d3
...@@ -122,10 +122,6 @@ struct connection { ...@@ -122,10 +122,6 @@ struct connection {
struct connection *othercon; struct connection *othercon;
struct work_struct rwork; /* Receive workqueue */ struct work_struct rwork; /* Receive workqueue */
struct work_struct swork; /* Send workqueue */ struct work_struct swork; /* Send workqueue */
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) #define sock2con(x) ((struct connection *)(x)->sk_user_data)
...@@ -148,6 +144,13 @@ struct dlm_node_addr { ...@@ -148,6 +144,13 @@ struct dlm_node_addr {
struct sockaddr_storage *addr[DLM_MAX_ADDR_COUNT]; struct sockaddr_storage *addr[DLM_MAX_ADDR_COUNT];
}; };
static struct listen_sock_callbacks {
void (*sk_error_report)(struct sock *);
void (*sk_data_ready)(struct sock *);
void (*sk_state_change)(struct sock *);
void (*sk_write_space)(struct sock *);
} listen_sock;
static LIST_HEAD(dlm_node_addrs); static LIST_HEAD(dlm_node_addrs);
static DEFINE_SPINLOCK(dlm_node_addrs_spin); static DEFINE_SPINLOCK(dlm_node_addrs_spin);
...@@ -477,7 +480,7 @@ static void lowcomms_error_report(struct sock *sk) ...@@ -477,7 +480,7 @@ static void lowcomms_error_report(struct sock *sk)
if (con == NULL) if (con == NULL)
goto out; goto out;
orig_report = con->orig_error_report; orig_report = listen_sock.sk_error_report;
if (con->sock == NULL || if (con->sock == NULL ||
kernel_getpeername(con->sock, (struct sockaddr *)&saddr, &buflen)) { kernel_getpeername(con->sock, (struct sockaddr *)&saddr, &buflen)) {
printk_ratelimited(KERN_ERR "dlm: node %d: socket error " printk_ratelimited(KERN_ERR "dlm: node %d: socket error "
...@@ -514,22 +517,26 @@ static void lowcomms_error_report(struct sock *sk) ...@@ -514,22 +517,26 @@ static void lowcomms_error_report(struct sock *sk)
} }
/* Note: sk_callback_lock must be locked before calling this function. */ /* Note: sk_callback_lock must be locked before calling this function. */
static void save_callbacks(struct connection *con, struct sock *sk) static void save_listen_callbacks(struct socket *sock)
{ {
con->orig_data_ready = sk->sk_data_ready; struct sock *sk = sock->sk;
con->orig_state_change = sk->sk_state_change;
con->orig_write_space = sk->sk_write_space; listen_sock.sk_data_ready = sk->sk_data_ready;
con->orig_error_report = sk->sk_error_report; listen_sock.sk_state_change = sk->sk_state_change;
listen_sock.sk_write_space = sk->sk_write_space;
listen_sock.sk_error_report = sk->sk_error_report;
} }
static void restore_callbacks(struct connection *con, struct sock *sk) static void restore_callbacks(struct socket *sock)
{ {
struct sock *sk = sock->sk;
write_lock_bh(&sk->sk_callback_lock); write_lock_bh(&sk->sk_callback_lock);
sk->sk_user_data = NULL; sk->sk_user_data = NULL;
sk->sk_data_ready = con->orig_data_ready; sk->sk_data_ready = listen_sock.sk_data_ready;
sk->sk_state_change = con->orig_state_change; sk->sk_state_change = listen_sock.sk_state_change;
sk->sk_write_space = con->orig_write_space; sk->sk_write_space = listen_sock.sk_write_space;
sk->sk_error_report = con->orig_error_report; sk->sk_error_report = listen_sock.sk_error_report;
write_unlock_bh(&sk->sk_callback_lock); write_unlock_bh(&sk->sk_callback_lock);
} }
...@@ -542,8 +549,6 @@ static void add_sock(struct socket *sock, struct connection *con, bool save_cb) ...@@ -542,8 +549,6 @@ static void add_sock(struct socket *sock, struct connection *con, bool save_cb)
con->sock = sock; con->sock = sock;
sk->sk_user_data = con; sk->sk_user_data = con;
if (save_cb)
save_callbacks(con, sk);
/* Install a data_ready callback */ /* Install a data_ready callback */
sk->sk_data_ready = lowcomms_data_ready; sk->sk_data_ready = lowcomms_data_ready;
sk->sk_write_space = lowcomms_write_space; sk->sk_write_space = lowcomms_write_space;
...@@ -583,8 +588,7 @@ static void close_connection(struct connection *con, bool and_other, ...@@ -583,8 +588,7 @@ static void close_connection(struct connection *con, bool and_other,
mutex_lock(&con->sock_mutex); mutex_lock(&con->sock_mutex);
if (con->sock) { if (con->sock) {
if (!test_bit(CF_IS_OTHERCON, &con->flags)) restore_callbacks(con->sock);
restore_callbacks(con, con->sock->sk);
sock_release(con->sock); sock_release(con->sock);
con->sock = NULL; con->sock = NULL;
} }
...@@ -1226,7 +1230,7 @@ static struct socket *tcp_create_listen_sock(struct connection *con, ...@@ -1226,7 +1230,7 @@ static struct socket *tcp_create_listen_sock(struct connection *con,
log_print("Failed to set SO_REUSEADDR on socket: %d", result); log_print("Failed to set SO_REUSEADDR on socket: %d", result);
} }
sock->sk->sk_user_data = con; sock->sk->sk_user_data = con;
save_listen_callbacks(sock);
con->rx_action = tcp_accept_from_sock; con->rx_action = tcp_accept_from_sock;
con->connect_action = tcp_connect_to_sock; con->connect_action = tcp_connect_to_sock;
...@@ -1310,6 +1314,7 @@ static int sctp_listen_for_all(void) ...@@ -1310,6 +1314,7 @@ static int sctp_listen_for_all(void)
write_lock_bh(&sock->sk->sk_callback_lock); write_lock_bh(&sock->sk->sk_callback_lock);
/* Init con struct */ /* Init con struct */
sock->sk->sk_user_data = con; sock->sk->sk_user_data = con;
save_listen_callbacks(sock);
con->sock = sock; con->sock = sock;
con->sock->sk->sk_data_ready = lowcomms_data_ready; con->sock->sk->sk_data_ready = lowcomms_data_ready;
con->rx_action = sctp_accept_from_sock; con->rx_action = sctp_accept_from_sock;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment