From ddcade952026343bb0d2a56745558dca1cbdafa3 Mon Sep 17 00:00:00 2001 From: Liang Zhen Date: Fri, 14 Nov 2014 23:48:07 +0800 Subject: [PATCH] LU-7099 lnet: lock improvement for ko2iblnd kiblnd_check_sends() takes conn::ibc_lock at the begin and release this lock at the end, this is inefficient because most use-case needs to explicitly release ibc_lock before caling this function. This patches changes it to kiblnd_check_sends_locked() and avoid unnecessary lock dances. Test-Parameters: trivial Signed-off-by: Doug Oucharek Change-Id: I3b348029dace70fd27beafa495f7a9e0bde442ed Reviewed-on: http://review.whamcloud.com/20322 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Amir Shehata Reviewed-by: Dmitry Eremin Reviewed-by: Oleg Drokin --- lnet/klnds/o2iblnd/o2iblnd_cb.c | 57 +++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/lnet/klnds/o2iblnd/o2iblnd_cb.c b/lnet/klnds/o2iblnd/o2iblnd_cb.c index b211f95..d5d6aec 100644 --- a/lnet/klnds/o2iblnd/o2iblnd_cb.c +++ b/lnet/klnds/o2iblnd/o2iblnd_cb.c @@ -42,7 +42,6 @@ static void kiblnd_peer_alive(kib_peer_t *peer); static void kiblnd_peer_connect_failed(kib_peer_t *peer, int active, int error); -static void kiblnd_check_sends(kib_conn_t *conn); static void kiblnd_init_tx_msg(lnet_ni_t *ni, kib_tx_t *tx, int type, int body_nob); static int kiblnd_init_rdma(kib_conn_t *conn, kib_tx_t *tx, int type, @@ -50,8 +49,9 @@ static int kiblnd_init_rdma(kib_conn_t *conn, kib_tx_t *tx, int type, static void kiblnd_queue_tx_locked(kib_tx_t *tx, kib_conn_t *conn); static void kiblnd_queue_tx(kib_tx_t *tx, kib_conn_t *conn); static void kiblnd_unmap_tx(lnet_ni_t *ni, kib_tx_t *tx); +static void kiblnd_check_sends_locked(kib_conn_t *conn); -static void +void kiblnd_tx_done (lnet_ni_t *ni, kib_tx_t *tx) { lnet_msg_t *lntmsg[2]; @@ -215,9 +215,9 @@ kiblnd_post_rx (kib_rx_t *rx, int credit) conn->ibc_outstanding_credits++; else conn->ibc_reserved_credits++; + kiblnd_check_sends_locked(conn); spin_unlock(&conn->ibc_lock); - kiblnd_check_sends(conn); out: kiblnd_conn_decref(conn); return rc; @@ -350,8 +350,8 @@ kiblnd_handle_rx (kib_rx_t *rx) !IBLND_OOB_CAPABLE(conn->ibc_version)) /* v1 only */ conn->ibc_outstanding_credits++; + kiblnd_check_sends_locked(conn); spin_unlock(&conn->ibc_lock); - kiblnd_check_sends(conn); } switch (msg->ibm_type) { @@ -803,9 +803,9 @@ __must_hold(&conn->ibc_lock) (!kiblnd_need_noop(conn) || /* redundant NOOP */ (IBLND_OOB_CAPABLE(ver) && /* posted enough NOOP */ conn->ibc_noops_posted == IBLND_OOB_MSGS(ver)))) { - /* OK to drop when posted enough NOOPs, since - * kiblnd_check_sends will queue NOOP again when - * posted NOOPs complete */ + /* OK to drop when posted enough NOOPs, since + * kiblnd_check_sends_locked will queue NOOP again when + * posted NOOPs complete */ spin_unlock(&conn->ibc_lock); kiblnd_tx_done(peer->ibp_ni, tx); spin_lock(&conn->ibc_lock); @@ -905,7 +905,7 @@ __must_hold(&conn->ibc_lock) } static void -kiblnd_check_sends (kib_conn_t *conn) +kiblnd_check_sends_locked(kib_conn_t *conn) { int ver = conn->ibc_version; lnet_ni_t *ni = conn->ibc_peer->ibp_ni; @@ -918,8 +918,6 @@ kiblnd_check_sends (kib_conn_t *conn) return; } - spin_lock(&conn->ibc_lock); - LASSERT(conn->ibc_nsends_posted <= kiblnd_concurrent_sends(ver, ni)); LASSERT (!IBLND_OOB_CAPABLE(ver) || @@ -969,8 +967,6 @@ kiblnd_check_sends (kib_conn_t *conn) if (kiblnd_post_tx_locked(conn, tx, credit) != 0) break; } - - spin_unlock(&conn->ibc_lock); } static void @@ -1016,16 +1012,11 @@ kiblnd_tx_complete (kib_tx_t *tx, int status) if (idle) list_del(&tx->tx_list); - kiblnd_conn_addref(conn); /* 1 ref for me.... */ - + kiblnd_check_sends_locked(conn); spin_unlock(&conn->ibc_lock); if (idle) kiblnd_tx_done(conn->ibc_peer->ibp_ni, tx); - - kiblnd_check_sends(conn); - - kiblnd_conn_decref(conn); /* ...until here */ } static void @@ -1211,9 +1202,8 @@ kiblnd_queue_tx (kib_tx_t *tx, kib_conn_t *conn) { spin_lock(&conn->ibc_lock); kiblnd_queue_tx_locked(tx, conn); + kiblnd_check_sends_locked(conn); spin_unlock(&conn->ibc_lock); - - kiblnd_check_sends(conn); } static int kiblnd_resolve_addr(struct rdma_cm_id *cmid, @@ -2184,13 +2174,10 @@ kiblnd_connreq_done(kib_conn_t *conn, int status) return; } - /* refcount taken by cmid is not reliable after I released the glock - * because this connection is visible to other threads now, another - * thread can find and close this connection right after I released - * the glock, if kiblnd_cm_callback for RDMA_CM_EVENT_DISCONNECTED is - * called, it can release the connection refcount taken by cmid. - * It means the connection could be destroyed before I finish my - * operations on it. + /* +1 ref for myself, this connection is visible to other threads + * now, refcount of peer:ibp_conns can be released by connection + * close from either a different thread, or the calling of + * kiblnd_check_sends_locked() below. See bz21911 for details. */ kiblnd_conn_addref(conn); write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags); @@ -2203,13 +2190,11 @@ kiblnd_connreq_done(kib_conn_t *conn, int status) kiblnd_queue_tx_locked(tx, conn); } + kiblnd_check_sends_locked(conn); spin_unlock(&conn->ibc_lock); - kiblnd_check_sends(conn); - /* schedule blocked rxs */ kiblnd_handle_early_rxs(conn); - kiblnd_conn_decref(conn); } @@ -3156,9 +3141,9 @@ kiblnd_check_conns (int idx) struct list_head *ctmp; unsigned long flags; - /* NB. We expect to have a look at all the peers and not find any - * RDMAs to time out, so we just use a shared lock while we - * take a look... */ + /* NB. We expect to have a look at all the peers and not find any + * RDMAs to time out, so we just use a shared lock while we + * take a look... */ read_lock_irqsave(&kiblnd_data.kib_global_lock, flags); list_for_each(ptmp, peers) { @@ -3221,7 +3206,11 @@ kiblnd_check_conns (int idx) conn = list_entry(checksends.next, kib_conn_t, ibc_connd_list); list_del(&conn->ibc_connd_list); - kiblnd_check_sends(conn); + + spin_lock(&conn->ibc_lock); + kiblnd_check_sends_locked(conn); + spin_unlock(&conn->ibc_lock); + kiblnd_conn_decref(conn); } } -- 1.8.3.1