Skip to content

Commit e05ddb9

Browse files
jsquyresjsquyres
committed
Merge pull request open-mpi#941 from hjelmn/v2.x_ugni_threading
v2.x UGNI threading fixes
2 parents 52e424c + 4d79c93 commit e05ddb9

File tree

4 files changed

+57
-39
lines changed

4 files changed

+57
-39
lines changed

‎opal/mca/btl/ugni/btl_ugni_component.c‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
22
/*
3-
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
3+
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
44
* reserved.
55
* Copyright (c) 2011 UT-Battelle, LLC. All rights reserved.
66
* $COPYRIGHT$
@@ -589,32 +589,21 @@ mca_btl_ugni_progress_wait_list (mca_btl_ugni_module_t *ugni_module)
589589
int count;
590590

591591
OPAL_THREAD_LOCK(&ugni_module->ep_wait_list_lock);
592-
count = opal_list_get_size(&ugni_module->ep_wait_list);
593-
OPAL_THREAD_UNLOCK(&ugni_module->ep_wait_list_lock);
592+
count = opal_list_get_size(&ugni_module->ep_wait_list);
594593

595594
do {
596-
OPAL_THREAD_LOCK(&ugni_module->ep_wait_list_lock);
597595
endpoint = (mca_btl_base_endpoint_t *) opal_list_remove_first (&ugni_module->ep_wait_list);
598-
OPAL_THREAD_UNLOCK(&ugni_module->ep_wait_list_lock);
599596
if (endpoint != NULL) {
600-
601-
endpoint->wait_listed = false;
602-
603597
rc = mca_btl_ugni_progress_send_wait_list (endpoint);
604598

605-
if (OPAL_SUCCESS != rc && false == endpoint->wait_listed) {
606-
607-
endpoint->wait_listed = true;
608-
OPAL_THREAD_LOCK(&ugni_module->ep_wait_list_lock);
599+
if (OPAL_SUCCESS != rc) {
609600
opal_list_append (&ugni_module->ep_wait_list, &endpoint->super);
610-
OPAL_THREAD_UNLOCK(&ugni_module->ep_wait_list_lock);
601+
} else {
602+
endpoint->wait_listed = false;
611603
}
612604
}
613-
614-
--count;
615-
if (count == 0) break;
616-
617-
} while (endpoint != NULL) ;
605+
} while (endpoint != NULL && --count > 0) ;
606+
OPAL_THREAD_UNLOCK(&ugni_module->ep_wait_list_lock);
618607

619608
return rc;
620609
}

‎opal/mca/btl/ugni/btl_ugni_endpoint.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
22
/*
3-
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
3+
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
44
* reserved.
55
* Copyright (c) 2011-2013 UT-Battelle, LLC. All rights reserved.
66
* $COPYRIGHT$
@@ -158,9 +158,11 @@ static inline int mca_btl_ugni_ep_connect_finish (mca_btl_base_endpoint_t *ep) {
158158

159159
rc = mca_btl_ugni_progress_send_wait_list (ep);
160160
if (OPAL_UNLIKELY(OPAL_SUCCESS != rc)) {
161-
ep->wait_listed = true;
162161
OPAL_THREAD_LOCK(&ep->btl->ep_wait_list_lock);
163-
opal_list_append (&ep->btl->ep_wait_list, &ep->super);
162+
if (false == ep->wait_listed) {
163+
opal_list_append (&ep->btl->ep_wait_list, &ep->super);
164+
ep->wait_listed = true;
165+
}
164166
OPAL_THREAD_UNLOCK(&ep->btl->ep_wait_list_lock);
165167
}
166168

‎opal/mca/btl/ugni/btl_ugni_frag.h‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
22
/*
3-
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
3+
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
44
* reserved.
55
* Copyright (c) 2011 UT-Battelle, LLC. All rights reserved.
66
* Copyright (c) 2013 The University of Tennessee and The University
@@ -66,6 +66,7 @@ struct mca_btl_ugni_base_frag_t;
6666

6767
typedef struct mca_btl_ugni_base_frag_t {
6868
mca_btl_base_descriptor_t base;
69+
volatile int32_t ref_cnt;
6970
uint32_t msg_id;
7071
uint16_t hdr_size;
7172
uint16_t flags;
@@ -148,6 +149,7 @@ static inline int mca_btl_ugni_frag_alloc (mca_btl_base_endpoint_t *ep,
148149
if (OPAL_LIKELY(NULL != *frag)) {
149150
(*frag)->my_list = list;
150151
(*frag)->endpoint = ep;
152+
(*frag)->ref_cnt = 1;
151153
return OPAL_SUCCESS;
152154
}
153155

@@ -169,10 +171,16 @@ static inline int mca_btl_ugni_frag_return (mca_btl_ugni_base_frag_t *frag)
169171
return OPAL_SUCCESS;
170172
}
171173

172-
static inline void mca_btl_ugni_frag_complete (mca_btl_ugni_base_frag_t *frag, int rc) {
173-
frag->flags |= MCA_BTL_UGNI_FRAG_COMPLETE;
174+
static inline bool mca_btl_ugni_frag_del_ref (mca_btl_ugni_base_frag_t *frag, int rc) {
175+
int32_t ref_cnt;
174176

175-
BTL_VERBOSE(("frag complete. flags = %d", frag->base.des_flags));
177+
opal_atomic_mb ();
178+
179+
ref_cnt = OPAL_THREAD_ADD32(&frag->ref_cnt, -1);
180+
if (ref_cnt) {
181+
assert (ref_cnt > 0);
182+
return false;
183+
}
176184

177185
/* call callback if specified */
178186
if (frag->base.des_flags & MCA_BTL_DES_SEND_ALWAYS_CALLBACK) {
@@ -182,6 +190,20 @@ static inline void mca_btl_ugni_frag_complete (mca_btl_ugni_base_frag_t *frag, i
182190
if (frag->base.des_flags & MCA_BTL_DES_FLAGS_BTL_OWNERSHIP) {
183191
mca_btl_ugni_frag_return (frag);
184192
}
193+
194+
return true;
195+
}
196+
197+
static inline void mca_btl_ugni_frag_complete (mca_btl_ugni_base_frag_t *frag, int rc) {
198+
BTL_VERBOSE(("frag complete. flags = %d", frag->base.des_flags));
199+
200+
frag->flags |= MCA_BTL_UGNI_FRAG_COMPLETE;
201+
202+
mca_btl_ugni_frag_del_ref (frag, rc);
203+
}
204+
205+
static inline bool mca_btl_ugni_frag_check_complete (mca_btl_ugni_base_frag_t *frag) {
206+
return !!(MCA_BTL_UGNI_FRAG_COMPLETE & frag->flags);
185207
}
186208

187209
#define MCA_BTL_UGNI_FRAG_ALLOC_SMSG(ep, frag) \

‎opal/mca/btl/ugni/btl_ugni_send.c‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
22
/*
3-
* Copyright (c) 2011-2014 Los Alamos National Security, LLC. All rights
3+
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
44
* reserved.
55
* Copyright (c) 2011 UT-Battelle, LLC. All rights reserved.
66
* Copyright (c) 2014 Research Organization for Information Science
@@ -25,7 +25,6 @@ int mca_btl_ugni_send (struct mca_btl_base_module_t *btl,
2525
mca_btl_ugni_base_frag_t *frag = (mca_btl_ugni_base_frag_t *) descriptor;
2626
size_t size = frag->segments[0].seg_len + frag->segments[1].seg_len;
2727
mca_btl_ugni_module_t *ugni_module = (mca_btl_ugni_module_t *) btl;
28-
int flags_save = frag->base.des_flags;
2928
int rc;
3029

3130
/* tag and len are at the same location in eager and smsg frag hdrs */
@@ -43,42 +42,48 @@ int mca_btl_ugni_send (struct mca_btl_base_module_t *btl,
4342
BTL_VERBOSE(("btl/ugni sending descriptor %p from %d -> %d. length = %" PRIu64, (void *)descriptor,
4443
OPAL_PROC_MY_NAME.vpid, endpoint->common->ep_rem_id, size));
4544

46-
/* temporarily disable ownership and callback flags so we can reliably check the complete flag */
47-
frag->base.des_flags &= ~(MCA_BTL_DES_FLAGS_BTL_OWNERSHIP | MCA_BTL_DES_SEND_ALWAYS_CALLBACK);
45+
/* add a reference to prevent the fragment from being returned until after the
46+
* completion flag is checked. */
47+
++frag->ref_cnt;
4848
frag->flags &= ~MCA_BTL_UGNI_FRAG_COMPLETE;
4949

5050
rc = mca_btl_ugni_send_frag (endpoint, frag);
51-
52-
if (OPAL_LIKELY(frag->flags & MCA_BTL_UGNI_FRAG_COMPLETE)) {
51+
if (OPAL_LIKELY(mca_btl_ugni_frag_check_complete (frag))) {
5352
/* fast path: remote side has received the frag */
54-
frag->base.des_flags = flags_save;
55-
mca_btl_ugni_frag_complete (frag, OPAL_SUCCESS);
53+
(void) mca_btl_ugni_frag_del_ref (frag, OPAL_SUCCESS);
5654

5755
return 1;
5856
}
5957

60-
if ((OPAL_SUCCESS == rc) && (frag->flags & MCA_BTL_UGNI_FRAG_BUFFERED) && (flags_save & MCA_BTL_DES_FLAGS_BTL_OWNERSHIP)) {
58+
if ((OPAL_SUCCESS == rc) && (frag->flags & MCA_BTL_UGNI_FRAG_BUFFERED) && (frag->flags & MCA_BTL_DES_FLAGS_BTL_OWNERSHIP)) {
6159
/* fast(ish) path: btl owned buffered frag. report send as complete */
62-
frag->base.des_flags = flags_save & ~MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
60+
bool call_callback = !!(frag->flags & MCA_BTL_DES_SEND_ALWAYS_CALLBACK);
61+
frag->flags &= ~MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
6362

64-
if (OPAL_LIKELY(flags_save & MCA_BTL_DES_SEND_ALWAYS_CALLBACK)) {
63+
if (call_callback) {
6564
frag->base.des_cbfunc(&frag->endpoint->btl->super, frag->endpoint, &frag->base, rc);
6665
}
6766

67+
(void) mca_btl_ugni_frag_del_ref (frag, OPAL_SUCCESS);
68+
6869
return 1;
6970
}
7071

7172
/* slow(ish) path: remote side hasn't received the frag. call the frag's callback when
7273
we get the local smsg/msgq or remote rdma completion */
73-
frag->base.des_flags = flags_save | MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
74+
frag->base.des_flags |= MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
75+
76+
mca_btl_ugni_frag_del_ref (frag, OPAL_SUCCESS);
7477

7578
if (OPAL_UNLIKELY(OPAL_ERR_OUT_OF_RESOURCE == rc)) {
7679
/* queue up request */
7780
if (false == endpoint->wait_listed) {
7881
OPAL_THREAD_LOCK(&ugni_module->ep_wait_list_lock);
79-
opal_list_append (&ugni_module->ep_wait_list, &endpoint->super);
82+
if (false == endpoint->wait_listed) {
83+
opal_list_append (&ugni_module->ep_wait_list, &endpoint->super);
84+
endpoint->wait_listed = true;
85+
}
8086
OPAL_THREAD_UNLOCK(&ugni_module->ep_wait_list_lock);
81-
endpoint->wait_listed = true;
8287
}
8388

8489
OPAL_THREAD_LOCK(&endpoint->lock);

0 commit comments

Comments
 (0)