From 8e426c791f06402b3df33fd90e5fcbca322c5321 Mon Sep 17 00:00:00 2001 From: Sean Hefty Date: Thu, 18 Feb 2010 21:49:14 +0000 Subject: [PATCH] dapl: locking cleanup and fixes Cleanup allocated completion channels. Destroy cm_ptr locks before freeing the cm_ptr to avoid memory leaks. And avoid accessing the cm_ptr after queuing it for destruction with the cr_thread to avoid use after free errors. Signed-off-by: Sean Hefty git-svn-id: svn://openib.tc.cornell.edu/gen1@2701 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86 --- trunk/ulp/dapl2/dapl/openib_cma/cm.c | 4 ++++ trunk/ulp/dapl2/dapl/openib_cma/device.c | 10 ++++++++++ trunk/ulp/dapl2/dapl/openib_scm/cm.c | 12 ++++++++---- trunk/ulp/dapl2/dapl/openib_scm/device.c | 10 ++++++++++ trunk/ulp/dapl2/dapl/openib_ucm/cm.c | 4 ++++ trunk/ulp/dapl2/dapl/openib_ucm/device.c | 10 ++++++++++ 6 files changed, 46 insertions(+), 4 deletions(-) diff --git a/trunk/ulp/dapl2/dapl/openib_cma/cm.c b/trunk/ulp/dapl2/dapl/openib_cma/cm.c index b8f71988..bb35c458 100644 --- a/trunk/ulp/dapl2/dapl/openib_cma/cm.c +++ b/trunk/ulp/dapl2/dapl/openib_cma/cm.c @@ -167,6 +167,7 @@ dp_ib_cm_handle_t dapls_ib_cm_create(DAPL_EP *ep) /* create CM_ID, bind to local device, create QP */ if (rdma_create_id(g_cm_events, &cm_id, (void *)conn, RDMA_PS_TCP)) { + dapl_os_lock_destroy(&conn->lock); dapl_os_free(conn, sizeof(*conn)); return NULL; } @@ -221,6 +222,7 @@ void dapls_ib_cm_free(dp_ib_cm_handle_t conn, DAPL_EP *ep) rdma_destroy_id(conn->cm_id); } + dapl_os_lock_destroy(&conn->lock); dapl_os_free(conn, sizeof(*conn)); } @@ -686,6 +688,7 @@ dapls_ib_setup_conn_listener(IN DAPL_IA * ia_ptr, /* create CM_ID, bind to local device, create QP */ if (rdma_create_id (g_cm_events, &conn->cm_id, (void *)conn, RDMA_PS_TCP)) { + dapl_os_lock_destroy(&conn->lock); dapl_os_free(conn, sizeof(*conn)); return (dapl_convert_errno(errno, "setup_listener")); } @@ -734,6 +737,7 @@ dapls_ib_setup_conn_listener(IN DAPL_IA * ia_ptr, bail: rdma_destroy_id(conn->cm_id); + dapl_os_lock_destroy(&conn->lock); dapl_os_free(conn, sizeof(*conn)); return dat_status; } diff --git a/trunk/ulp/dapl2/dapl/openib_cma/device.c b/trunk/ulp/dapl2/dapl/openib_cma/device.c index 32090fbe..c9d3756d 100644 --- a/trunk/ulp/dapl2/dapl/openib_cma/device.c +++ b/trunk/ulp/dapl2/dapl/openib_cma/device.c @@ -502,6 +502,16 @@ DAT_RETURN dapls_ib_close_hca(IN DAPL_HCA * hca_ptr) dapl_os_sleep_usec(1000); } bail: + if (hca_ptr->ib_trans.ib_cq) + ibv_destroy_comp_channel(hca_ptr->ib_trans.ib_cq); + + if (hca_ptr->ib_trans.ib_cq_empty) { + struct ibv_comp_channel *channel; + channel = hca_ptr->ib_trans.ib_cq_empty->channel; + ibv_destroy_cq(hca_ptr->ib_trans.ib_cq_empty); + ibv_destroy_comp_channel(channel); + } + if (hca_ptr->ib_hca_handle != IB_INVALID_HANDLE) { if (rdma_destroy_id(hca_ptr->ib_trans.cm_id)) return (dapl_convert_errno(errno, "ib_close_device")); diff --git a/trunk/ulp/dapl2/dapl/openib_scm/cm.c b/trunk/ulp/dapl2/dapl/openib_scm/cm.c index 356f0fb5..03ba3acf 100644 --- a/trunk/ulp/dapl2/dapl/openib_scm/cm.c +++ b/trunk/ulp/dapl2/dapl/openib_scm/cm.c @@ -317,6 +317,7 @@ void dapls_ib_cm_free(dp_ib_cm_handle_t cm_ptr, DAPL_EP *ep) closesocket(cm_ptr->socket); } dapl_os_unlock(&cm_ptr->lock); + dapl_os_lock_destroy(&cm_ptr->lock); dapl_os_free(cm_ptr, sizeof(*cm_ptr)); return; } @@ -390,15 +391,17 @@ notify_thread: /* queue socket for processing CM work */ static void dapli_cm_queue(struct ib_cm_handle *cm_ptr) { + DAPL_HCA *hca = cm_ptr->hca; + /* add to work queue for cr thread processing */ dapl_llist_init_entry((DAPL_LLIST_ENTRY *) & cm_ptr->entry); - dapl_os_lock(&cm_ptr->hca->ib_trans.lock); - dapl_llist_add_tail(&cm_ptr->hca->ib_trans.list, + dapl_os_lock(&hca->ib_trans.lock); + dapl_llist_add_tail(&hca->ib_trans.list, (DAPL_LLIST_ENTRY *) & cm_ptr->entry, cm_ptr); - dapl_os_unlock(&cm_ptr->hca->ib_trans.lock); + dapl_os_unlock(&hca->ib_trans.lock); /* wakeup CM work thread */ - send(cm_ptr->hca->ib_trans.scm[1], "w", sizeof "w", 0); + send(hca->ib_trans.scm[1], "w", sizeof "w", 0); } /* @@ -1779,6 +1782,7 @@ void cr_thread(void *arg) inet_ntoa(((struct sockaddr_in *) &cr->msg.daddr.so)->sin_addr)); dapl_os_unlock(&cr->lock); + dapl_os_lock_destroy(&cr->lock); dapls_ib_cm_free(cr, cr->ep); continue; } diff --git a/trunk/ulp/dapl2/dapl/openib_scm/device.c b/trunk/ulp/dapl2/dapl/openib_scm/device.c index bb3893a7..04e992a8 100644 --- a/trunk/ulp/dapl2/dapl/openib_scm/device.c +++ b/trunk/ulp/dapl2/dapl/openib_scm/device.c @@ -504,6 +504,16 @@ DAT_RETURN dapls_ib_close_hca(IN DAPL_HCA * hca_ptr) } out: + if (hca_ptr->ib_trans.ib_cq) + ibv_destroy_comp_channel(hca_ptr->ib_trans.ib_cq); + + if (hca_ptr->ib_trans.ib_cq_empty) { + struct ibv_comp_channel *channel; + channel = hca_ptr->ib_trans.ib_cq_empty->channel; + ibv_destroy_cq(hca_ptr->ib_trans.ib_cq_empty); + ibv_destroy_comp_channel(channel); + } + if (hca_ptr->ib_hca_handle != IB_INVALID_HANDLE) { if (ibv_close_device(hca_ptr->ib_hca_handle)) return (dapl_convert_errno(errno, "ib_close_device")); diff --git a/trunk/ulp/dapl2/dapl/openib_ucm/cm.c b/trunk/ulp/dapl2/dapl/openib_ucm/cm.c index 100317bb..03fc7527 100644 --- a/trunk/ulp/dapl2/dapl/openib_ucm/cm.c +++ b/trunk/ulp/dapl2/dapl/openib_ucm/cm.c @@ -728,6 +728,7 @@ void dapls_ib_cm_free(dp_ib_cm_handle_t cm, DAPL_EP *ep) /* cleanup, never made it to work queue */ if (cm->state == DCM_INIT) { dapl_os_unlock(&cm->lock); + dapl_os_lock_destroy(&cm->lock); dapl_os_free(cm, sizeof(*cm)); return; } @@ -1701,6 +1702,7 @@ dapls_ib_remove_conn_listener(IN DAPL_IA *ia, IN DAPL_SP *sp) cm->state = DCM_DESTROY; dapl_os_unlock(&cm->lock); ucm_dequeue_listen(cm); + dapl_os_lock_destroy(&cm->lock); dapl_os_free(cm, sizeof(*cm)); } return DAT_SUCCESS; @@ -1981,6 +1983,7 @@ void cm_thread(void *arg) dapl_llist_remove_entry(&hca->ib_trans.list, (DAPL_LLIST_ENTRY *)&cm->entry); dapl_os_unlock(&cm->lock); + dapl_os_lock_destroy(&cm->lock); dapl_os_free(cm, sizeof(*cm)); continue; } @@ -2052,6 +2055,7 @@ void cm_thread(void *arg) &hca->ib_trans.list, (DAPL_LLIST_ENTRY *)&cm->entry); dapl_os_unlock(&cm->lock); + dapl_os_lock_destroy(&cm->lock); dapl_os_free(cm, sizeof(*cm)); continue; } diff --git a/trunk/ulp/dapl2/dapl/openib_ucm/device.c b/trunk/ulp/dapl2/dapl/openib_ucm/device.c index e890eeff..a9cec73e 100644 --- a/trunk/ulp/dapl2/dapl/openib_ucm/device.c +++ b/trunk/ulp/dapl2/dapl/openib_ucm/device.c @@ -401,6 +401,16 @@ DAT_RETURN dapls_ib_close_hca(IN DAPL_HCA * hca_ptr) destroy_os_signal(hca_ptr); ucm_service_destroy(hca_ptr); + if (hca_ptr->ib_trans.ib_cq) + ibv_destroy_comp_channel(hca_ptr->ib_trans.ib_cq); + + if (hca_ptr->ib_trans.ib_cq_empty) { + struct ibv_comp_channel *channel; + channel = hca_ptr->ib_trans.ib_cq_empty->channel; + ibv_destroy_cq(hca_ptr->ib_trans.ib_cq_empty); + ibv_destroy_comp_channel(channel); + } + if (hca_ptr->ib_hca_handle != IB_INVALID_HANDLE) { if (ibv_close_device(hca_ptr->ib_hca_handle)) return (dapl_convert_errno(errno, "ib_close_device")); -- 2.46.0