Fix additional atomic access issues and add volatile variants

Co-authored-by: devreal <10974502+devreal@users.noreply.github.com>

Author: Copilot

opal/class/opal_lifo.h

@@ -70,7 +70,7 @@ static inline bool opal_update_counted_pointer(volatile opal_counted_pointer_t *
                                                opal_counted_pointer_t *old, opal_list_item_t *item)
 {
     opal_counted_pointer_t new_p;
-    opal_atomic_store_ptr_relaxed(&new_p.data.item, (intptr_t) item);
+    opal_atomic_store_ptr_volatile_relaxed(&new_p.data.item, (intptr_t) item);
     new_p.data.counter = old->data.counter + 1;
     return opal_atomic_compare_exchange_strong_128(&addr->atomic_value, &old->value, new_p.value);
 }
@@ -122,7 +122,7 @@ OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_lifo_t);
  */
 static inline bool opal_lifo_is_empty(opal_lifo_t *lifo)
 {
-    return (opal_list_item_t *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item) == &lifo->opal_lifo_ghost;
+    return (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item) == &lifo->opal_lifo_ghost;
 }
 
 #if OPAL_HAVE_ATOMIC_COMPARE_EXCHANGE_128 && !OPAL_HAVE_ATOMIC_LLSC_PTR
@@ -133,7 +133,7 @@ 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)
 {
-    opal_list_item_t *next = (opal_list_item_t *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item);
+    opal_list_item_t *next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
 
     do {
         item->opal_list_next = next;
@@ -181,7 +181,7 @@ 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 *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item);
+    opal_list_item_t *next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
 
     /* item free acts as a mini lock to avoid ABA problems */
     item->item_free = 1;
@@ -242,7 +242,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
 {
     opal_list_item_t *item, *head, *ghost = &lifo->opal_lifo_ghost;
 
-    while ((item = (opal_list_item_t *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item)) != ghost) {
+    while ((item = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item)) != ghost) {
         /* ensure it is safe to pop the head */
         if (opal_atomic_swap_32((opal_atomic_int32_t *) &item->item_free, 1)) {
             continue;
@@ -282,17 +282,17 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
 /* single-threaded versions of the lifo functions */
 static inline opal_list_item_t *opal_lifo_push_st(opal_lifo_t *lifo, opal_list_item_t *item)
 {
-    item->opal_list_next = (opal_list_item_t *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item);
+    item->opal_list_next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
     item->item_free = 0;
-    opal_atomic_store_ptr_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item);
+    opal_atomic_store_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item);
     return (opal_list_item_t *) item->opal_list_next;
 }
 
 static inline opal_list_item_t *opal_lifo_pop_st(opal_lifo_t *lifo)
 {
     opal_list_item_t *item;
-    item = (opal_list_item_t *) opal_atomic_load_ptr_relaxed(&lifo->opal_lifo_head.data.item);
-    opal_atomic_store_ptr_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item->opal_list_next);
+    item = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
+    opal_atomic_store_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item->opal_list_next);
     if (item == &lifo->opal_lifo_ghost) {
         return NULL;
     }

opal/include/opal_stdatomic.h

@@ -113,6 +113,11 @@ static inline intptr_t opal_atomic_load_ptr(const opal_atomic_intptr_t *addr, me
     return atomic_load_explicit(&addr->value, order);
 }
 
+static inline intptr_t opal_atomic_load_ptr_volatile(const volatile opal_atomic_intptr_t *addr, memory_order order)
+{
+    return atomic_load_explicit(&addr->value, order);
+}
+
 static inline uintptr_t opal_atomic_load_uptr(const opal_atomic_uintptr_t *addr, memory_order order)
 {
     return atomic_load_explicit(&addr->value, order);
@@ -164,6 +169,11 @@ static inline void opal_atomic_store_ptr(opal_atomic_intptr_t *addr, intptr_t va
     atomic_store_explicit(&addr->value, value, order);
 }
 
+static inline void opal_atomic_store_ptr_volatile(volatile opal_atomic_intptr_t *addr, intptr_t value, memory_order order)
+{
+    atomic_store_explicit(&addr->value, value, order);
+}
+
 static inline void opal_atomic_store_uptr(opal_atomic_uintptr_t *addr, uintptr_t value, memory_order order)
 {
     atomic_store_explicit(&addr->value, value, order);
@@ -185,6 +195,11 @@ static inline intptr_t opal_atomic_load_ptr_relaxed(const opal_atomic_intptr_t *
     return opal_atomic_load_ptr(addr, memory_order_relaxed);
 }
 
+static inline intptr_t opal_atomic_load_ptr_volatile_relaxed(const volatile opal_atomic_intptr_t *addr)
+{
+    return opal_atomic_load_ptr_volatile(addr, memory_order_relaxed);
+}
+
 static inline void opal_atomic_store_32_relaxed(opal_atomic_int32_t *addr, int32_t value)
 {
     opal_atomic_store_32(addr, value, memory_order_relaxed);
@@ -200,6 +215,11 @@ static inline void opal_atomic_store_ptr_relaxed(opal_atomic_intptr_t *addr, int
     opal_atomic_store_ptr(addr, value, memory_order_relaxed);
 }
 
+static inline void opal_atomic_store_ptr_volatile_relaxed(volatile opal_atomic_intptr_t *addr, intptr_t value)
+{
+    opal_atomic_store_ptr_volatile(addr, value, memory_order_relaxed);
+}
+
 static inline size_t opal_atomic_load_size_t_relaxed(const opal_atomic_size_t *addr)
 {
     return opal_atomic_load_size_t(addr, memory_order_relaxed);
@@ -210,6 +230,16 @@ static inline void opal_atomic_store_size_t_relaxed(opal_atomic_size_t *addr, si
     opal_atomic_store_size_t(addr, value, memory_order_relaxed);
 }
 
+static inline uint32_t opal_atomic_load_uint32_relaxed(const opal_atomic_uint32_t *addr)
+{
+    return opal_atomic_load_uint32(addr, memory_order_relaxed);
+}
+
+static inline void opal_atomic_store_uint32_relaxed(opal_atomic_uint32_t *addr, uint32_t value)
+{
+    opal_atomic_store_uint32(addr, value, memory_order_relaxed);
+}
+
 #    endif /* OPAL_USE_C11_ATOMICS == 0 */
 
 #    if HAVE_OPAL_INT128_T

opal/mca/btl/sm/btl_sm_fifo.h

@@ -73,18 +73,18 @@ static inline mca_btl_sm_hdr_t *sm_fifo_read(sm_fifo_t *fifo, struct mca_btl_bas
     mca_btl_sm_hdr_t *hdr;
     fifo_value_t value;
 
-    if (SM_FIFO_FREE == fifo->fifo_head) {
+    if (SM_FIFO_FREE == opal_atomic_load_ptr_relaxed(&fifo->fifo_head)) {
         return NULL;
     }
 
     opal_atomic_rmb();
 
-    value = fifo->fifo_head;
+    value = opal_atomic_load_ptr_relaxed(&fifo->fifo_head);
 
     *ep = &mca_btl_sm_component.endpoints[value >> MCA_BTL_SM_OFFSET_BITS];
     hdr = (mca_btl_sm_hdr_t *) relative2virtual(value);
 
-    fifo->fifo_head = SM_FIFO_FREE;
+    opal_atomic_store_ptr_relaxed(&fifo->fifo_head, SM_FIFO_FREE);
 
     assert(hdr->next != value);
 
@@ -96,10 +96,10 @@ static inline mca_btl_sm_hdr_t *sm_fifo_read(sm_fifo_t *fifo, struct mca_btl_bas
                 opal_atomic_rmb();
             }
 
-            fifo->fifo_head = hdr->next;
+            opal_atomic_store_ptr_relaxed(&fifo->fifo_head, hdr->next);
         }
     } else {
-        fifo->fifo_head = hdr->next;
+        opal_atomic_store_ptr_relaxed(&fifo->fifo_head, hdr->next);
     }
 
     opal_atomic_wmb();
@@ -111,7 +111,7 @@ static inline void sm_fifo_init(sm_fifo_t *fifo)
     /* 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 = SM_FIFO_FREE; */
-    fifo->fifo_head = SM_FIFO_FREE;
+    opal_atomic_store_ptr_relaxed(&fifo->fifo_head, SM_FIFO_FREE);
     fifo->fifo_tail = SM_FIFO_FREE;
     fifo->fbox_available = mca_btl_sm_component.fbox_max;
     mca_btl_sm_component.my_fifo = fifo;
@@ -131,7 +131,7 @@ static inline void sm_fifo_write(sm_fifo_t *fifo, fifo_value_t value)
         mca_btl_sm_hdr_t *hdr = (mca_btl_sm_hdr_t *) relative2virtual(prev);
         hdr->next = value;
     } else {
-        fifo->fifo_head = value;
+        opal_atomic_store_ptr_relaxed(&fifo->fifo_head, value);
     }
 
     opal_atomic_wmb();

opal/mca/common/sm/common_sm.c

@@ -110,8 +110,8 @@ static mca_common_sm_module_t *attach_and_init(opal_shmem_ds_t *shmem_bufp, size
         /* initialize some segment information */
         size_t mem_offset = map->module_data_addr - (unsigned char *) map->module_seg;
         opal_atomic_lock_init(&map->module_seg->seg_lock, OPAL_ATOMIC_LOCK_UNLOCKED);
-        map->module_seg->seg_inited = 0;
-        map->module_seg->seg_num_procs_inited = 0;
+        opal_atomic_store_32_relaxed(&map->module_seg->seg_inited, 0);
+        opal_atomic_store_size_t_relaxed(&map->module_seg->seg_num_procs_inited, 0);
         map->module_seg->seg_offset = mem_offset;
         map->module_seg->seg_size = size - mem_offset;
         opal_atomic_wmb();

opal/mca/rcache/base/rcache_base_frame.c

@@ -57,8 +57,8 @@ static void mca_rcache_base_registration_constructor(mca_rcache_base_registratio
     reg->rcache = NULL;
     reg->base = NULL;
     reg->bound = NULL;
-    reg->ref_count = 0;
-    reg->flags = 0;
+    opal_atomic_store_32_relaxed(&reg->ref_count, 0);
+    opal_atomic_store_uint32_relaxed(&reg->flags, 0);
 }
 
 OBJ_CLASS_INSTANCE(mca_rcache_base_registration_t, opal_free_list_item_t,

opal/mca/rcache/base/rcache_base_vma_tree.c

@@ -144,7 +144,7 @@ static int mca_rcache_base_tree_dump_range_helper(uint64_t low, uint64_t high, v
     mca_rcache_base_registration_t *reg = (mca_rcache_base_registration_t *) data;
 
     opal_output(0, "    reg: base=%p, bound=%p, ref_count=%d, flags=0x%x", (void *) reg->base,
-                (void *) reg->bound, reg->ref_count, reg->flags);
+                (void *) reg->bound, opal_atomic_load_32_relaxed(&reg->ref_count), opal_atomic_load_uint32_relaxed(&reg->flags));
 
     return OPAL_SUCCESS;
 }

opal/mca/threads/base/wait_sync.c

@@ -47,7 +47,7 @@ void opal_threads_base_wait_sync_global_wakeup_mt(int status)
     opal_mutex_unlock(&wait_sync_lock);
 }
 
-static opal_atomic_int32_t num_thread_in_progress = 0;
+static opal_atomic_int32_t num_thread_in_progress = { .value = 0 };
 
 #define WAIT_SYNC_PASS_OWNERSHIP(who)                        \
     do {                                                     \
@@ -62,7 +62,7 @@ int ompi_sync_wait_mt(ompi_wait_sync_t *sync)
      * race condition around the release of the synchronization using the
      * signaling field.
      */
-    if (sync->count <= 0) {
+    if (opal_atomic_load_32_relaxed(&sync->count) <= 0) {
         return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
     }
 
@@ -72,7 +72,7 @@ int ompi_sync_wait_mt(ompi_wait_sync_t *sync)
     /* Now that we hold the lock make sure another thread has not already
      * call cond_signal.
      */
-    if (sync->count <= 0) {
+    if (opal_atomic_load_32_relaxed(&sync->count) <= 0) {
         opal_thread_internal_mutex_unlock(&sync->lock);
         return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
     }
@@ -97,7 +97,7 @@ int ompi_sync_wait_mt(ompi_wait_sync_t *sync)
      *  - our sync has been triggered.
      */
 check_status:
-    if (sync != opal_threads_base_wait_sync_list && num_thread_in_progress >= opal_max_thread_in_progress) {
+    if (sync != opal_threads_base_wait_sync_list && opal_atomic_load_32_relaxed(&num_thread_in_progress) >= opal_max_thread_in_progress) {
         opal_thread_internal_cond_wait(&sync->condition, &sync->lock);
 
         /**
@@ -106,7 +106,7 @@ int ompi_sync_wait_mt(ompi_wait_sync_t *sync)
          * promoted as the progress manager.
          */
 
-        if (sync->count <= 0) { /* Completed? */
+        if (opal_atomic_load_32_relaxed(&sync->count) <= 0) { /* Completed? */
             opal_thread_internal_mutex_unlock(&sync->lock);
             goto i_am_done;
         }
@@ -116,7 +116,7 @@ int ompi_sync_wait_mt(ompi_wait_sync_t *sync)
     opal_thread_internal_mutex_unlock(&sync->lock);
 
     OPAL_THREAD_ADD_FETCH32(&num_thread_in_progress, 1);
-    while (sync->count > 0) { /* progress till completion */
+    while (opal_atomic_load_32_relaxed(&sync->count) > 0) { /* progress till completion */
         /* don't progress with the sync lock locked or you'll deadlock */
         opal_progress();
     }

opal/mca/threads/wait_sync.h

@@ -100,7 +100,7 @@ static inline int sync_wait_st(ompi_wait_sync_t *sync)
     assert(NULL == sync->next);
     opal_threads_base_wait_sync_list = sync;
 
-    while (sync->count > 0) {
+    while (opal_atomic_load_32_relaxed(&sync->count) > 0) {
         opal_progress();
     }
     opal_threads_base_wait_sync_list = NULL;