From dc000213eac3e0c06ab56360993e0de6e1b92d0c Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Thu, 17 Mar 2016 13:12:29 -0600 Subject: [PATCH] opal/fifo: use atomics to set fifo head in opal_fifo_push This commit changes the opal_fifo_push code to use opal_update_counted_pointer to set the head. This fixes a data race that occurs because the read of the fifo head in opal_fifo_pop requires two instructions. This combined with the non-atomic update in opal_fifo_push can lead to an ABA issue that puts the fifo in an inconsistant state. There are other ways this problem could be fixed. One way would be to introduce an opal_atomic_read_128 implementation. On x86_64 this would have to use the cmpxchg16b instruction. Since this instruction would have to be in the pop path (and always executed) it would be slower than the fix in this commit. Closes open-mpi/ompi#1460. Signed-off-by: Nathan Hjelm --- opal/class/opal_fifo.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opal/class/opal_fifo.h b/opal/class/opal_fifo.h index 604601dc35..ad9cbdbcbb 100644 --- a/opal/class/opal_fifo.h +++ b/opal/class/opal_fifo.h @@ -12,7 +12,7 @@ * All rights reserved. * Copyright (c) 2007 Voltaire All rights reserved. * Copyright (c) 2010 IBM Corporation. All rights reserved. - * Copyright (c) 2014-2015 Los Alamos National Security, LLC. All rights + * Copyright (c) 2014-2016 Los Alamos National Security, LLC. All rights * reseved. * $COPYRIGHT$ * @@ -101,7 +101,8 @@ static inline opal_list_item_t *opal_fifo_push_atomic (opal_fifo_t *fifo, if (&fifo->opal_fifo_ghost == tail.data.item) { /* update the head */ - fifo->opal_fifo_head.data.item = item; + opal_counted_pointer_t head = {.value = fifo->opal_fifo_head.value}; + opal_update_counted_pointer (&fifo->opal_fifo_head, head, item); } else { /* update previous item */ tail.data.item->opal_list_next = item;