From df6dd69db8ab828c020eb97e534545afccea72dc Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Tue, 2 Oct 2018 15:52:45 -0600 Subject: [PATCH 1/2] btl/vader: ensure the fast box tag is always read first On some platfoms reading a 64-bit value is non-atomic and it is possible that the two 32-bit values are read in the wrong order. To ensure the tag is always read first this commit reads the tag before reading the full 64-bit value. Signed-off-by: Nathan Hjelm (cherry picked from commit 66a7dc4c72cb25df67e7f872bee7a20b5fa9c763) --- opal/mca/btl/vader/btl_vader_fbox.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/opal/mca/btl/vader/btl_vader_fbox.h b/opal/mca/btl/vader/btl_vader_fbox.h index f85a5f6a22..5af802781e 100644 --- a/opal/mca/btl/vader/btl_vader_fbox.h +++ b/opal/mca/btl/vader/btl_vader_fbox.h @@ -56,6 +56,16 @@ static inline void mca_btl_vader_fbox_set_header (mca_btl_vader_fbox_hdr_t *hdr, hdr->data.tag = tag; } +static inline mca_btl_vader_fbox_hdr_t mca_btl_vader_fbox_read_header (mca_btl_vader_fbox_hdr_t *hdr) +{ + mca_btl_vader_fbox_hdr_t tmp; + uint16_t tag = hdr->data.tag; + opal_atomic_rmb (); + tmp.ival = hdr->ival; + tmp.data.tag = tag; + return tmp; +} + /* attempt to reserve a contiguous segment from the remote ep */ static inline bool mca_btl_vader_fbox_sendi (mca_btl_base_endpoint_t *ep, unsigned char tag, void * restrict header, const size_t header_size, @@ -175,7 +185,7 @@ static inline bool mca_btl_vader_check_fboxes (void) int poll_count; for (poll_count = 0 ; poll_count <= MCA_BTL_VADER_POLL_COUNT ; ++poll_count) { - const mca_btl_vader_fbox_hdr_t hdr = {.ival = MCA_BTL_VADER_FBOX_HDR(ep->fbox_in.buffer + start)->ival}; + const mca_btl_vader_fbox_hdr_t hdr = mca_btl_vader_fbox_read_header (MCA_BTL_VADER_FBOX_HDR(ep->fbox_in.buffer + start)); /* check for a valid tag a sequence number */ if (0 == hdr.data.tag || hdr.data.seq != ep->fbox_in.seq) { From fa768d748fbb6e1ad31249d2ee74dfbd1c537548 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Tue, 2 Oct 2018 14:55:31 -0600 Subject: [PATCH 2/2] btl/vader: work around Oracle compiler bug This commit works around an Oracle C compiler bug in 5.15 (not sure when it was introduced). The bug is triggered when we chain assignments of atomic variables. Ex: _Atomic intptr x, y; intptr_t z = 0; x = y = z; Will produce a compiler error of the form: operand cannot have void type: op "=" assignment type mismatch: long "=" void To work around the issue we are removing the chain assignment and setting the head and tail on different lines. Fixes #5814 Signed-off-by: Nathan Hjelm (cherry picked from commit dfa8d3a81ac64e32d2bfb13a9afb20b83d747e03) --- opal/mca/btl/vader/btl_vader_fifo.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/opal/mca/btl/vader/btl_vader_fifo.h b/opal/mca/btl/vader/btl_vader_fifo.h index 0dc70bc8a1..178a416704 100644 --- a/opal/mca/btl/vader/btl_vader_fifo.h +++ b/opal/mca/btl/vader/btl_vader_fifo.h @@ -12,7 +12,7 @@ * All rights reserved. * Copyright (c) 2006-2007 Voltaire. All rights reserved. * Copyright (c) 2009-2010 Cisco Systems, Inc. All rights reserved. - * Copyright (c) 2010-2017 Los Alamos National Security, LLC. + * Copyright (c) 2010-2018 Los Alamos National Security, LLC. * All rights reserved. * $COPYRIGHT$ * @@ -155,7 +155,11 @@ static inline mca_btl_vader_hdr_t *vader_fifo_read (vader_fifo_t *fifo, struct m static inline void vader_fifo_init (vader_fifo_t *fifo) { - fifo->fifo_head = fifo->fifo_tail = VADER_FIFO_FREE; + /* due to a compiler bug in Oracle C 5.15 the following line was broken into two. Not + * ideal but oh well. See #5814 */ + /* fifo->fifo_head = fifo->fifo_tail = VADER_FIFO_FREE; */ + fifo->fifo_head = VADER_FIFO_FREE; + fifo->fifo_tail = VADER_FIFO_FREE; fifo->fbox_available = mca_btl_vader_component.fbox_max; mca_btl_vader_component.my_fifo = fifo; }