Skip to content

Commit dc00021

Browse files
committed
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 #1460. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent f0e3740 commit dc00021

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

opal/class/opal_fifo.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* All rights reserved.
1313
* Copyright (c) 2007 Voltaire All rights reserved.
1414
* Copyright (c) 2010 IBM Corporation. All rights reserved.
15-
* Copyright (c) 2014-2015 Los Alamos National Security, LLC. All rights
15+
* Copyright (c) 2014-2016 Los Alamos National Security, LLC. All rights
1616
* reseved.
1717
* $COPYRIGHT$
1818
*
@@ -101,7 +101,8 @@ static inline opal_list_item_t *opal_fifo_push_atomic (opal_fifo_t *fifo,
101101

102102
if (&fifo->opal_fifo_ghost == tail.data.item) {
103103
/* update the head */
104-
fifo->opal_fifo_head.data.item = item;
104+
opal_counted_pointer_t head = {.value = fifo->opal_fifo_head.value};
105+
opal_update_counted_pointer (&fifo->opal_fifo_head, head, item);
105106
} else {
106107
/* update previous item */
107108
tail.data.item->opal_list_next = item;

0 commit comments

Comments
 (0)