From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Tue, 26 Jan 2016 20:04:24 +0000
Subject: [Cluster-devel] [DLM PATCH] DLM: save / restore all socket
 callbacks
In-Reply-To: <1244929267.15846120.1453831061604.JavaMail.zimbra@redhat.com>
References: <1244929267.15846120.1453831061604.JavaMail.zimbra@redhat.com>
Message-ID: <56A7D148.9080309@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On 26/01/16 17:57, Bob Peterson wrote:
> Hi,
>
> 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.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index dc9ae6d..f9eb366 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)
>   
> @@ -465,12 +468,43 @@ int dlm_lowcomms_connect_node(int nodeid)
>   	return 0;
>   }
>   
> -static void lowcomms_error_report(struct sock *sk)
> +static void save_old_callbacks(struct sock *sk)
> +{
> +	struct connection *con = sock2con(sk);
> +
> +	if (sk->sk_error_report) {
> +		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;
> +	}
> +}
> +
This should not need to be conditional, because sk->sk_error_report() 
should never be NULL, so I think that needs looking at again - where did 
that NULL pointer come from?

Also the other callbacks, data_ready, etc should also have the same 
locking pattern of read_lock_bh(), con = sock2con(sk); if (con) { /* 
original code here */ } read_unlock_bh(); as per the error_report one,

Steve.