Skip to content

Commit b81171c

Browse files
AstralBobteigland
AstralBob
authored and
teigland
committed
DLM: Save and restore socket callbacks properly
This patch fixes the problems with patch b3a5bbf. 1. It removes a return statement from lowcomms_error_report because it needs to call the original error report in all paths through the function. 2. All socket callbacks are saved and restored, not just the sk_error_report, and that's done so with proper locking like sunrpc does. Signed-off-by: Bob Peterson <rpeterso@redhat.com> Signed-off-by: David Teigland <teigland@redhat.com>
1 parent 1a31833 commit b81171c

1 file changed

Lines changed: 59 additions & 11 deletions

File tree

‎fs/dlm/lowcomms.c‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ struct connection {
124124
struct connection *othercon;
125125
struct work_struct rwork; /* Receive workqueue */
126126
struct work_struct swork; /* Send workqueue */
127-
void (*orig_error_report)(struct sock *sk);
127+
void (*orig_error_report)(struct sock *);
128+
void (*orig_data_ready)(struct sock *);
129+
void (*orig_state_change)(struct sock *);
130+
void (*orig_write_space)(struct sock *);
128131
};
129132
#define sock2con(x) ((struct connection *)(x)->sk_user_data)
130133

@@ -467,18 +470,24 @@ int dlm_lowcomms_connect_node(int nodeid)
467470

468471
static void lowcomms_error_report(struct sock *sk)
469472
{
470-
struct connection *con = sock2con(sk);
473+
struct connection *con;
471474
struct sockaddr_storage saddr;
472475
int buflen;
476+
void (*orig_report)(struct sock *) = NULL;
473477

478+
read_lock_bh(&sk->sk_callback_lock);
479+
con = sock2con(sk);
480+
if (con == NULL)
481+
goto out;
482+
483+
orig_report = con->orig_error_report;
474484
if (con->sock == NULL ||
475485
kernel_getpeername(con->sock, (struct sockaddr *)&saddr, &buflen)) {
476486
printk_ratelimited(KERN_ERR "dlm: node %d: socket error "
477487
"sending to node %d, port %d, "
478488
"sk_err=%d/%d\n", dlm_our_nodeid(),
479489
con->nodeid, dlm_config.ci_tcp_port,
480490
sk->sk_err, sk->sk_err_soft);
481-
return;
482491
} else if (saddr.ss_family == AF_INET) {
483492
struct sockaddr_in *sin4 = (struct sockaddr_in *)&saddr;
484493

@@ -501,22 +510,54 @@ static void lowcomms_error_report(struct sock *sk)
501510
dlm_config.ci_tcp_port, sk->sk_err,
502511
sk->sk_err_soft);
503512
}
504-
con->orig_error_report(sk);
513+
out:
514+
read_unlock_bh(&sk->sk_callback_lock);
515+
if (orig_report)
516+
orig_report(sk);
517+
}
518+
519+
/* Note: sk_callback_lock must be locked before calling this function. */
520+
static void save_callbacks(struct connection *con, struct sock *sk)
521+
{
522+
lock_sock(sk);
523+
con->orig_data_ready = sk->sk_data_ready;
524+
con->orig_state_change = sk->sk_state_change;
525+
con->orig_write_space = sk->sk_write_space;
526+
con->orig_error_report = sk->sk_error_report;
527+
release_sock(sk);
528+
}
529+
530+
static void restore_callbacks(struct connection *con, struct sock *sk)
531+
{
532+
write_lock_bh(&sk->sk_callback_lock);
533+
lock_sock(sk);
534+
sk->sk_user_data = NULL;
535+
sk->sk_data_ready = con->orig_data_ready;
536+
sk->sk_state_change = con->orig_state_change;
537+
sk->sk_write_space = con->orig_write_space;
538+
sk->sk_error_report = con->orig_error_report;
539+
release_sock(sk);
540+
write_unlock_bh(&sk->sk_callback_lock);
505541
}
506542

507543
/* Make a socket active */
508544
static void add_sock(struct socket *sock, struct connection *con)
509545
{
546+
struct sock *sk = sock->sk;
547+
548+
write_lock_bh(&sk->sk_callback_lock);
510549
con->sock = sock;
511550

551+
sk->sk_user_data = con;
552+
if (!test_bit(CF_IS_OTHERCON, &con->flags))
553+
save_callbacks(con, sk);
512554
/* Install a data_ready callback */
513-
con->sock->sk->sk_data_ready = lowcomms_data_ready;
514-
con->sock->sk->sk_write_space = lowcomms_write_space;
515-
con->sock->sk->sk_state_change = lowcomms_state_change;
516-
con->sock->sk->sk_user_data = con;
517-
con->sock->sk->sk_allocation = GFP_NOFS;
518-
con->orig_error_report = con->sock->sk->sk_error_report;
519-
con->sock->sk->sk_error_report = lowcomms_error_report;
555+
sk->sk_data_ready = lowcomms_data_ready;
556+
sk->sk_write_space = lowcomms_write_space;
557+
sk->sk_state_change = lowcomms_state_change;
558+
sk->sk_allocation = GFP_NOFS;
559+
sk->sk_error_report = lowcomms_error_report;
560+
write_unlock_bh(&sk->sk_callback_lock);
520561
}
521562

522563
/* Add the port number to an IPv6 or 4 sockaddr and return the address
@@ -551,6 +592,8 @@ static void close_connection(struct connection *con, bool and_other,
551592

552593
mutex_lock(&con->sock_mutex);
553594
if (con->sock) {
595+
if (!test_bit(CF_IS_OTHERCON, &con->flags))
596+
restore_callbacks(con, con->sock->sk);
554597
sock_release(con->sock);
555598
con->sock = NULL;
556599
}
@@ -1192,6 +1235,8 @@ static struct socket *tcp_create_listen_sock(struct connection *con,
11921235
if (result < 0) {
11931236
log_print("Failed to set SO_REUSEADDR on socket: %d", result);
11941237
}
1238+
sock->sk->sk_user_data = con;
1239+
11951240
con->rx_action = tcp_accept_from_sock;
11961241
con->connect_action = tcp_connect_to_sock;
11971242

@@ -1273,13 +1318,16 @@ static int sctp_listen_for_all(void)
12731318
if (result < 0)
12741319
log_print("Could not set SCTP NODELAY error %d\n", result);
12751320

1321+
write_lock_bh(&sock->sk->sk_callback_lock);
12761322
/* Init con struct */
12771323
sock->sk->sk_user_data = con;
12781324
con->sock = sock;
12791325
con->sock->sk->sk_data_ready = lowcomms_data_ready;
12801326
con->rx_action = sctp_accept_from_sock;
12811327
con->connect_action = sctp_connect_to_sock;
12821328

1329+
write_unlock_bh(&sock->sk->sk_callback_lock);
1330+
12831331
/* Bind to all addresses. */
12841332
if (sctp_bind_addrs(con, dlm_config.ci_tcp_port))
12851333
goto create_delsock;

0 commit comments

Comments
 (0)