[Cluster-devel] [DLM PATCH 6/6] DLM: save / restore all socket callbacks
Steven Whitehouse
swhiteho at redhat.com
Thu Feb 11 15:31:04 UTC 2016
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 <rpeterso at 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;
>
More information about the Cluster-devel
mailing list