From 11bb8b09a03aecf147598cea06edee12c556aa75 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Wed, 8 Nov 2017 10:23:42 -0700 Subject: [PATCH] opal/class: use new compare-and-swap functions Signed-off-by: Nathan Hjelm --- opal/class/opal_fifo.h | 49 ++++++++++++++++++------------------- opal/class/opal_lifo.h | 45 ++++++++++++++++++++-------------- opal/threads/thread_usage.h | 4 +-- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/opal/class/opal_fifo.h b/opal/class/opal_fifo.h index a9da88f459..ad67c77a6f 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-2016 Los Alamos National Security, LLC. All rights + * Copyright (c) 2014-2017 Los Alamos National Security, LLC. All rights * reseved. * $COPYRIGHT$ * @@ -85,14 +85,12 @@ static inline bool opal_fifo_is_empty( opal_fifo_t* fifo ) static inline opal_list_item_t *opal_fifo_push_atomic (opal_fifo_t *fifo, opal_list_item_t *item) { - opal_counted_pointer_t tail; + opal_counted_pointer_t tail = {.value = fifo->opal_fifo_tail.value}; item->opal_list_next = &fifo->opal_fifo_ghost; do { - tail.value = fifo->opal_fifo_tail.value; - - if (opal_update_counted_pointer (&fifo->opal_fifo_tail, tail, item)) { + if (opal_update_counted_pointer (&fifo->opal_fifo_tail, &tail, item)) { break; } } while (1); @@ -102,7 +100,7 @@ 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 */ opal_counted_pointer_t head = {.value = fifo->opal_fifo_head.value}; - opal_update_counted_pointer (&fifo->opal_fifo_head, head, item); + opal_update_counted_pointer (&fifo->opal_fifo_head, &head, item); } else { /* update previous item */ tail.data.item->opal_list_next = item; @@ -116,29 +114,28 @@ static inline opal_list_item_t *opal_fifo_push_atomic (opal_fifo_t *fifo, */ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) { - opal_list_item_t *item, *next; - opal_counted_pointer_t head, tail; + opal_list_item_t *item, *next, *ghost = &fifo->opal_fifo_ghost; + opal_counted_pointer_t head = {.value = fifo->opal_fifo_head.value}, tail; do { - head.value = fifo->opal_fifo_head.value; tail.value = fifo->opal_fifo_tail.value; opal_atomic_rmb (); item = (opal_list_item_t *) head.data.item; next = (opal_list_item_t *) item->opal_list_next; - if (&fifo->opal_fifo_ghost == tail.data.item && &fifo->opal_fifo_ghost == item) { + if (ghost == tail.data.item && ghost == item) { return NULL; } /* the head or next pointer are in an inconsistent state. keep looping. */ - if (tail.data.item != item && &fifo->opal_fifo_ghost != tail.data.item && - &fifo->opal_fifo_ghost == next) { + if (tail.data.item != item && ghost != tail.data.item && ghost == next) { + head.value = fifo->opal_fifo_head.value; continue; } /* try popping the head */ - if (opal_update_counted_pointer (&fifo->opal_fifo_head, head, next)) { + if (opal_update_counted_pointer (&fifo->opal_fifo_head, &head, next)) { break; } } while (1); @@ -146,14 +143,14 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) opal_atomic_wmb (); /* check for tail and head consistency */ - if (&fifo->opal_fifo_ghost == next) { + if (ghost == next) { /* the head was just set to &fifo->opal_fifo_ghost. try to update the tail as well */ - if (!opal_update_counted_pointer (&fifo->opal_fifo_tail, tail, &fifo->opal_fifo_ghost)) { + if (!opal_update_counted_pointer (&fifo->opal_fifo_tail, &tail, ghost)) { /* tail was changed by a push operation. wait for the item's next pointer to be se then * update the head */ /* wait for next pointer to be updated by push */ - while (&fifo->opal_fifo_ghost == item->opal_list_next) { + while (ghost == item->opal_list_next) { opal_atomic_rmb (); } @@ -166,7 +163,7 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) head.value = fifo->opal_fifo_head.value; next = (opal_list_item_t *) item->opal_list_next; - assert (&fifo->opal_fifo_ghost == head.data.item); + assert (ghost == head.data.item); fifo->opal_fifo_head.data.item = next; opal_atomic_wmb (); @@ -215,14 +212,14 @@ static inline opal_list_item_t *opal_fifo_push_atomic (opal_fifo_t *fifo, */ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) { - opal_list_item_t *item, *next; + opal_list_item_t *item, *next, *ghost = &fifo->opal_fifo_ghost; #if OPAL_HAVE_ATOMIC_LLSC_PTR /* use load-linked store-conditional to avoid ABA issues */ do { item = opal_atomic_ll_ptr (&fifo->opal_fifo_head.data.item); - if (&fifo->opal_fifo_ghost == item) { - if (&fifo->opal_fifo_ghost == fifo->opal_fifo_tail.data.item) { + if (ghost == item) { + if (ghost == fifo->opal_fifo_tail.data.item) { return NULL; } @@ -239,7 +236,7 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) #else /* protect against ABA issues by "locking" the head */ do { - if (opal_atomic_bool_cmpset_32 ((int32_t *) &fifo->opal_fifo_head.data.counter, 0, 1)) { + if (!opal_atomic_swap_32 ((volatile int32_t *) &fifo->opal_fifo_head.data.counter, 1)) { break; } @@ -249,7 +246,7 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) opal_atomic_wmb(); item = opal_fifo_head (fifo); - if (&fifo->opal_fifo_ghost == item) { + if (ghost == item) { fifo->opal_fifo_head.data.counter = 0; return NULL; } @@ -258,9 +255,11 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo) fifo->opal_fifo_head.data.item = next; #endif - if (&fifo->opal_fifo_ghost == next) { - if (!opal_atomic_bool_cmpset_ptr (&fifo->opal_fifo_tail.data.item, item, &fifo->opal_fifo_ghost)) { - while (&fifo->opal_fifo_ghost == item->opal_list_next) { + if (ghost == next) { + void *tmp = item; + + if (!opal_atomic_compare_exchange_strong_ptr (&fifo->opal_fifo_tail.data.item, &tmp, ghost)) { + while (ghost == item->opal_list_next) { opal_atomic_rmb (); } diff --git a/opal/class/opal_lifo.h b/opal/class/opal_lifo.h index 0d8512fe0e..e5a3f9110c 100644 --- a/opal/class/opal_lifo.h +++ b/opal/class/opal_lifo.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-2017 Los Alamos National Security, LLC. All rights * reseved. * Copyright (c) 2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. @@ -65,13 +65,13 @@ typedef union opal_counted_pointer_t opal_counted_pointer_t; * to allow the upper level to detect if this element is the first one in the * list (if the list was empty before this operation). */ -static inline bool opal_update_counted_pointer (volatile opal_counted_pointer_t *addr, opal_counted_pointer_t old, +static inline bool opal_update_counted_pointer (volatile opal_counted_pointer_t *addr, opal_counted_pointer_t *old, opal_list_item_t *item) { opal_counted_pointer_t new_p; new_p.data.item = item; - new_p.data.counter = old.data.counter + 1; - return opal_atomic_bool_cmpset_128 (&addr->value, old.value, new_p.value); + new_p.data.counter = old->data.counter + 1; + return opal_atomic_compare_exchange_strong_128 (&addr->value, &old->value, new_p.value); } #endif @@ -119,14 +119,14 @@ static inline bool opal_lifo_is_empty( opal_lifo_t* lifo ) static inline opal_list_item_t *opal_lifo_push_atomic (opal_lifo_t *lifo, opal_list_item_t *item) { - do { - opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item; + opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item; + do { item->opal_list_next = next; opal_atomic_wmb (); /* to protect against ABA issues it is sufficient to only update the counter in pop */ - if (opal_atomic_bool_cmpset_ptr (&lifo->opal_lifo_head.data.item, next, item)) { + if (opal_atomic_compare_exchange_strong_ptr (&lifo->opal_lifo_head.data.item, &next, item)) { return next; } /* DO some kind of pause to release the bus */ @@ -141,17 +141,17 @@ static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo) opal_counted_pointer_t old_head; opal_list_item_t *item; + old_head.data.counter = lifo->opal_lifo_head.data.counter; + opal_atomic_rmb (); + old_head.data.item = (opal_list_item_t *) lifo->opal_lifo_head.data.item; + do { - - old_head.data.counter = lifo->opal_lifo_head.data.counter; - opal_atomic_rmb (); - old_head.data.item = item = (opal_list_item_t*)lifo->opal_lifo_head.data.item; - + item = (opal_list_item_t *) old_head.data.item; if (item == &lifo->opal_lifo_ghost) { return NULL; } - if (opal_update_counted_pointer (&lifo->opal_lifo_head, old_head, + if (opal_update_counted_pointer (&lifo->opal_lifo_head, &old_head, (opal_list_item_t *) item->opal_list_next)) { opal_atomic_wmb (); item->opal_list_next = NULL; @@ -169,13 +169,15 @@ static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo) static inline opal_list_item_t *opal_lifo_push_atomic (opal_lifo_t *lifo, opal_list_item_t *item) { + opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item; + /* item free acts as a mini lock to avoid ABA problems */ item->item_free = 1; + do { - opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item; item->opal_list_next = next; opal_atomic_wmb(); - if (opal_atomic_bool_cmpset_ptr (&lifo->opal_lifo_head.data.item, next, item)) { + if (opal_atomic_compare_exchange_strong_ptr (&lifo->opal_lifo_head.data.item, &next, item)) { opal_atomic_wmb (); /* now safe to pop this item */ item->item_free = 0; @@ -236,8 +238,11 @@ static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo) */ static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo) { - opal_list_item_t *item; - while ((item = (opal_list_item_t *) lifo->opal_lifo_head.data.item) != &lifo->opal_lifo_ghost) { + opal_list_item_t *item, *head, *ghost = &lifo->opal_lifo_ghost; + + item = (opal_list_item_t *) lifo->opal_lifo_head.data.item; + + while (item != ghost) { /* ensure it is safe to pop the head */ if (opal_atomic_swap_32((volatile int32_t *) &item->item_free, 1)) { continue; @@ -245,14 +250,16 @@ static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo) opal_atomic_wmb (); + head = item; /* try to swap out the head pointer */ - if (opal_atomic_bool_cmpset_ptr (&lifo->opal_lifo_head.data.item, item, - (void *) item->opal_list_next)) { + if (opal_atomic_compare_exchange_strong_ptr (&lifo->opal_lifo_head.data.item, &head, + (void *) item->opal_list_next)) { break; } /* NTH: don't need another atomic here */ item->item_free = 0; + item = head; /* Do some kind of pause to release the bus */ } diff --git a/opal/threads/thread_usage.h b/opal/threads/thread_usage.h index 91260f4c5c..4a41a1dba6 100644 --- a/opal/threads/thread_usage.h +++ b/opal/threads/thread_usage.h @@ -223,8 +223,8 @@ OPAL_THREAD_DEFINE_ATOMIC_SWAP(void *, intptr_t, ptr) #define OPAL_THREAD_BOOL_CMPSET_32 opal_thread_cmpset_bool_32 #define OPAL_ATOMIC_BOOL_CMPSET_32 opal_thread_cmpset_bool_32 -#define OPAL_THREAD_BOOL_COMPARE_EXCHANGE_STRONG_32 opal_thread_compare_exchange_strong_32 -#define OPAL_ATOMIC_BOOL_COMPARE_EXCHANGE_STRONG_32 opal_thread_compare_exchange_strong_32 +#define OPAL_THREAD_COMPARE_EXCHANGE_STRONG_32 opal_thread_compare_exchange_strong_32 +#define OPAL_ATOMIC_COMPARE_EXCHANGE_STRONG_32 opal_thread_compare_exchange_strong_32 #define OPAL_THREAD_BOOL_CMPSET_PTR(x, y, z) opal_thread_cmpset_bool_ptr ((volatile intptr_t *) x, (void *) y, (void *) z) #define OPAL_ATOMIC_BOOL_CMPSET_PTR OPAL_THREAD_BOOL_CMPSET_PTR