From: maxim Date: Wed, 15 Apr 2009 19:38:54 +0000 (+0000) Subject: b=18844 X-Git-Tag: v1_9_170~48 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=66c7ab34ef607318e527140402e9c2d0e2a4b6c3 b=18844 i=isaac Landing a patch fixing deadlock in usocklnd on HEAD. The patch also includes: - minor code cleanup suggested by Isaac in comment #6 of bug #18844 - trivial cleanup patch for acceptor.c (att #21983 of bug #14132) - minor one-line fix for handling EINTR error of poll(2) - inspected by Shadow. --- diff --git a/lnet/ChangeLog b/lnet/ChangeLog index 36e3dbd..578ab40 100644 --- a/lnet/ChangeLog +++ b/lnet/ChangeLog @@ -17,6 +17,14 @@ Bugzilla : Description: Details : +Severity : normal +Bugzilla : 18844 +Description: Fixing deadlock in usocklnd +Details : A deadlock was possible in usocklnd due to race condition while + tearing connection down. The problem resulted from erroneous + assumption that lnet_finalize() could have been called holding + some lnd-level locks. + Severity : major Bugzilla : 13621, 15983 Description: Protocol V2 of o2iblnd diff --git a/lnet/lnet/acceptor.c b/lnet/lnet/acceptor.c index c852f16..5b143a9 100644 --- a/lnet/lnet/acceptor.c +++ b/lnet/lnet/acceptor.c @@ -87,6 +87,18 @@ CFS_MODULE_PARM(accept_backlog, "i", int, 0444, CFS_MODULE_PARM(accept_timeout, "i", int, 0644, "Acceptor's timeout (seconds)"); +static char *accept_type = NULL; + +int +lnet_acceptor_get_tunables(void) +{ + /* Userland acceptor uses 'accept_type' instead of 'accept', due to + * conflict with 'accept(2)', but kernel acceptor still uses 'accept' + * for compatibility. Hence the trick. */ + accept_type = accept; + return 0; +} + int lnet_acceptor_timeout(void) { @@ -221,7 +233,7 @@ EXPORT_SYMBOL(lnet_connect); #else /* below is multi-threaded user-space code */ -static char *accept_type = "secure"; +static char *accept_type = "secure"; int lnet_acceptor_get_tunables() @@ -418,11 +430,7 @@ lnet_acceptor(void *arg) lnet_acceptor_state.pta_sock = NULL; } else { -#ifdef __KERNEL__ - LCONSOLE(0, "Accept %s, port %d\n", accept, accept_port); -#else LCONSOLE(0, "Accept %s, port %d\n", accept_type, accept_port); -#endif } /* set init status and unblock parent */ @@ -517,23 +525,18 @@ lnet_acceptor_start(void) LASSERT (lnet_acceptor_state.pta_sock == NULL); -#ifndef __KERNEL__ - /* kernel version uses CFS_MODULE_PARM */ rc = lnet_acceptor_get_tunables(); if (rc != 0) return rc; +#ifndef __KERNEL__ /* Do nothing if we're liblustre clients */ if ((the_lnet.ln_pid & LNET_PID_USERFLAG) != 0) return 0; #endif cfs_init_completion(&lnet_acceptor_state.pta_signal); -#ifdef __KERNEL__ - rc = accept2secure(accept, &secure); -#else rc = accept2secure(accept_type, &secure); -#endif if (rc <= 0) { cfs_fini_completion(&lnet_acceptor_state.pta_signal); return rc; diff --git a/lnet/ulnds/socklnd/conn.c b/lnet/ulnds/socklnd/conn.c index 4d6e01d..48c9a1e 100644 --- a/lnet/ulnds/socklnd/conn.c +++ b/lnet/ulnds/socklnd/conn.c @@ -128,6 +128,8 @@ usocklnd_tear_peer_conn(usock_conn_t *conn) lnet_process_id_t id; int decref_flag = 0; int killall_flag = 0; + void *rx_lnetmsg = NULL; + CFS_LIST_HEAD (zombie_txs); if (peer == NULL) /* nothing to tear */ return; @@ -142,11 +144,12 @@ usocklnd_tear_peer_conn(usock_conn_t *conn) if (conn->uc_rx_state == UC_RX_LNET_PAYLOAD) { /* change state not to finalize twice */ conn->uc_rx_state = UC_RX_KSM_HEADER; - lnet_finalize(peer->up_ni, conn->uc_rx_lnetmsg, -EIO); + /* stash lnetmsg while holding locks */ + rx_lnetmsg = conn->uc_rx_lnetmsg; } - usocklnd_destroy_txlist(peer->up_ni, - &conn->uc_tx_list); + /* we cannot finilize txs right now (bug #18844) */ + list_splice_init(&conn->uc_tx_list, &zombie_txs); peer->up_conns[idx] = NULL; conn->uc_peer = NULL; @@ -154,6 +157,9 @@ usocklnd_tear_peer_conn(usock_conn_t *conn) if(conn->uc_errored && !peer->up_errored) peer->up_errored = killall_flag = 1; + + /* prevent queueing new txs to this conn */ + conn->uc_errored = 1; } pthread_mutex_unlock(&conn->uc_lock); @@ -166,6 +172,11 @@ usocklnd_tear_peer_conn(usock_conn_t *conn) if (!decref_flag) return; + if (rx_lnetmsg != NULL) + lnet_finalize(ni, rx_lnetmsg, -EIO); + + usocklnd_destroy_txlist(ni, &zombie_txs); + usocklnd_conn_decref(conn); usocklnd_peer_decref(peer); @@ -850,6 +861,13 @@ usocklnd_find_or_create_conn(usock_peer_t *peer, int type, LASSERT(tx == NULL || zc_ack == NULL); if (tx != NULL) { + /* usocklnd_tear_peer_conn() could signal us stop queueing */ + if (conn->uc_errored) { + rc = -EIO; + pthread_mutex_unlock(&conn->uc_lock); + goto find_or_create_conn_failed; + } + usocklnd_enqueue_tx(conn, tx, send_immediately); } else { rc = usocklnd_enqueue_zcack(conn, zc_ack); diff --git a/lnet/ulnds/socklnd/handlers.c b/lnet/ulnds/socklnd/handlers.c index e9eda46..521e5b2 100644 --- a/lnet/ulnds/socklnd/handlers.c +++ b/lnet/ulnds/socklnd/handlers.c @@ -1000,7 +1000,8 @@ usocklnd_read_data(usock_conn_t *conn) nob = libcfs_sock_readv(conn->uc_sock, conn->uc_rx_iov, conn->uc_rx_niov); if (nob <= 0) {/* read nothing or error */ - conn->uc_errored = 1; + if (nob < 0) + conn->uc_errored = 1; return nob; } diff --git a/lnet/ulnds/socklnd/poll.c b/lnet/ulnds/socklnd/poll.c index 2f15f1b..01ea259 100644 --- a/lnet/ulnds/socklnd/poll.c +++ b/lnet/ulnds/socklnd/poll.c @@ -113,7 +113,7 @@ usocklnd_poll_thread(void *arg) pt_data->upt_nfds, usock_tuns.ut_poll_timeout * 1000); - if (rc < 0) { + if (rc < 0 && errno != EINTR) { CERROR("Cannot poll(2): errno=%d\n", errno); break; }