opal/asm changes causes MT applications hang.

[thananon]

I did a rebase to master head from a version from October and I found that some of the multithreaded applications I use get deadlock when the program is initialized with THREAD_MULTIPLE with more than 1 thread doing communication. (1 is fine)

I did a bisection and it seems to start from the commit 84f63d0 from @hjelmn . I dug a little bit deeper and found that it might be the problem from opal_free_lifo_pop_atomic(). This is the stack from a typical MT ping-ping injection rate. It looks like the item_free field is always 1 although it is the only thread trying to do the pop.

#0  opal_lifo_pop_atomic (lifo=0x7ffff7dda780 <mca_pml_base_recv_requests>) at ../../../../opal/class/opal_lifo.h:247
247             if (opal_atomic_swap_32((volatile int32_t *) &item->item_free, 1)) {
(gdb) bt
#0  opal_lifo_pop_atomic (lifo=0x7ffff7dda780 <mca_pml_base_recv_requests>) at ../../../../opal/class/opal_lifo.h:247
#1  0x00007fffe2f37002 in opal_free_list_get_mt (flist=0x7ffff7dda780 <mca_pml_base_recv_requests>) at ../../../../opal/class/opal_free_list.h:193
#2  0x00007fffe2f370ec in opal_free_list_get (flist=0x7ffff7dda780 <mca_pml_base_recv_requests>) at ../../../../opal/class/opal_free_list.h:222
#3  0x00007fffe2f38537 in mca_pml_ob1_recv (addr=0x7fffd00008c0, count=1, datatype=0x603360 <ompi_mpi_byte>, src=1, tag=2, comm=0x603160 <ompi_mpi_comm_world>, status=0x7fffdfa04dc0) at pml_ob1_irecv.c:121
#4  0x00007ffff7ae5e84 in PMPI_Recv (buf=0x7fffd00008c0, count=1, type=0x603360 <ompi_mpi_byte>, source=1, tag=2, comm=0x603160 <ompi_mpi_comm_world>, status=0x7fffdfa04dc0) at precv.c:79
#5  0x000000000040150a in thread_work (info=0x902fd0) at pairwise.c:193
#6  0x00007ffff7815e25 in start_thread () from /usr/lib64/libpthread.so.0
#7  0x00007ffff754334d in clone () from /usr/lib64/libc.so.6

Another application that has the same problem is GRID. The threaded-stencil benchmark deadlocks but thread-test from ompi-tests seems to run without problems.

OS: Red Hat Scientific Linux release 7.3

Any suggestion?

[ggouaillardet]

which program did you use to generate this stack trace ? if this is GRID, can you kindly provide basic instructions on how to build and run the app ?

[hjelmn]

@thanon Is this with it without built-in atomics?

[hjelmn]

Also, what architecture and compiler. I am assuming x86-64 with a gcc 6.x or newer.

[thananon]

@hjelmn OMPI is built without flag --disable-builtin-atomic. This is X86-64 with GCC 7.1.0.

@ggouaillardet This is the stack trace from my ping-ping app here.

For GRID, to compile:
./configure --enable-mkl=$MKLROOT --enable-lapack=$MKLROOT CC=mpicc CXX=mpicxx --enable-comms=mpi --enable-simd=AVX

and try to run Benchmark_ITT with args ./Benchmark_ITT --mpi 2.1.1.1 --threads 20 --comms=mpi

[ggouaillardet]

@thananon the link is broken, can you please fix/update it ?

[thananon]

@ggouaillardet My bad. It is fixed now.

[ggouaillardet]

@thananon thanks, and what is your command line for pairwise ? how many nodes, which interconnect ? do use use UCX ?

[ggouaillardet]

OK, I think I understand the race condition.

First, let's note that OPAL_HAVE_ATOMIC_COMPARE_EXCHANGE_128=0 with gcc 7.1.
FWIW, RHEL7 default gcc (4.8.5) ends up with OPAL_HAVE_ATOMIC_COMPARE_EXCHANGE_128=1.

Let's consider an opal_lifo_t with one item, and two threads invoke opal_lifo_pop_atomic() at the same time, and then wait for each other after that.

239 static inline opal_list_item_t *opal_lifo_pop_atomic (opal_lifo_t* lifo)
240 {
241     opal_list_item_t *item, *head, *ghost = &lifo->opal_lifo_ghost;
242 
243     item = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
244 
245     while (item != ghost) {
246         /* ensure it is safe to pop the head */
247         if (opal_atomic_swap_32((volatile int32_t *) &item->item_free, 1)) {
248             continue;
249         }
250 
251         opal_atomic_wmb ();
252 
253         head = item;
254         /* try to swap out the head pointer */
255         if (opal_atomic_compare_exchange_strong_ptr (&lifo->opal_lifo_head.data.item, &head,
256                                                      (void *) item->opal_list_next)) {
257             break;
258         }

Let's now consider the following scenario.

• both threads run line 243 item = (opal_list_item_t *) lifo->opal_lifo_head.data.item; at the same time, and hence end up both with the same item
• the first thread goes fast, opal_atomic_swap32() marks the item as free, and then the exchange of the head is successful, so the function returns and thread 1 now wait for thread 2.
• the second thread now executes opal_atomic_swap_32((volatile int32_t *) &item->item_free, 1), and enters an endless loop since the item has already been marked as free by the first thread.

The inline patch below fixes that

@thananon can you please give it a try ?
@hjelmn can you please review it ?

diff --git a/opal/class/opal_lifo.h b/opal/class/opal_lifo.h
index e5a3f91..aea21fc 100644
--- a/opal/class/opal_lifo.h
+++ b/opal/class/opal_lifo.h
@@ -14,8 +14,8 @@
  * Copyright (c) 2010      IBM Corporation.  All rights reserved.
  * 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.
+ * Copyright (c) 2016-2018 Research Organization for Information Science
+ *                         and Technology (RIST).  All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -240,9 +240,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;
 
-    item = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
-
-    while (item != ghost) {
+    while ((item=(opal_list_item_t *)lifo->opal_lifo_head.data.item) != ghost) {
         /* ensure it is safe to pop the head */
         if (opal_atomic_swap_32((volatile int32_t *) &item->item_free, 1)) {
             continue;

[bosilca]

Nice catch, that's an ugly version of the ABA problem. Once a thread fails to get a free head it will be stuck forever until the same item makes it back in the list. If the item is the head, it will work with a drastic performance impact. Otherwise we destroy the lifo, by dropping everything between the real head and the element we thought was the head.

I wonder how do we conclude that OPAL_HAVE_ATOMIC_COMPARE_EXCHANGE_128 is 0 with gcc7.1. Are we failing to check in libatomic ?

[ggouaillardet]

I am afk and will be on the road the next two days.
Iirc, support of __atomic builtin atomic compare-and-swap on 128-bit values compiles and link with older gcc, but compiles and fails to link with gcc 7.1

[thananon]

@ggouaillardet The patch works. Thank you for looking into this.

If you are still interested in pairwise, it is designed to be run on two nodes only and I used openib BTL for the test.

./pairwise -s (msg size) -i (iteration) -w (window size) -t (num threads)