fine grained locking causes strange segfaults

Originally by robl on 2015-01-14 13:36:12 -0600

Will Scullin and Paul Coffman have found that with recent MPICH and --enable-thread-cs=per-object even the most simple of tests will segfault in strange places (such as MPI_COMM_FREE)

Originally by robl on 2015-01-14 13:37:23 -0600

${HOME}/src/mpich/configure \ --host=powerpc64-bgq-linux \ --with-device=pamid \ --with-file-system=gpfs:BGQ \ --disable-wrapper-rpath \ --prefix=${HOME}/soft/mpich-master-alt\ --enable-thread-cs=per-object \ --enable-g=dbg\ --with-atomic-primitives \ --enable-handle-allocation=tls \ --enable-refcount=lock-free \ --disable-fortran \ --enable-fast=O3 \ --disable-predefined-refcount $@

Originally by robl on 2015-01-14 13:41:04 -0600

yeilds odd backtraces like this with the ROMIO 'simple' test:

bucket 2 : instances # 5, rank35 0x014092e8: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/stdlib/abort.c:77 0x01403448: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/assert/assert.c:81 0x010b32d0: /home/robl/src/mpich/src/mpid/pamid/src/mpix/mpix.c:539 0x0111eb24: /home/robl/src/mpich/src/mpi/romio/adio/ad_gpfs/bg/ad_bg_pset.c:165 0x0111d378: /home/robl/src/mpich/src/mpi/romio/adio/ad_gpfs/bg/ad_bg_aggrs.c:98 0x010c8edc: /home/robl/src/mpich/src/mpi/romio/adio/ad_gpfs/ad_gpfs_hints.c:242 0x010d82a8: /home/robl/src/mpich/src/mpi/romio/adio/common/ad_open.c:116 0x0105d1d8: /home/robl/src/mpich/src/mpi/romio/mpi-io/open.c:154 0x01000d84: /home/robl/src/mpich/src/mpi/romio/test/simple.c:71 0x014008e8: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/csu/../csu/libc-start.c:226 0x01400be4: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/csu/../sysdeps/unix/sysv/linux/powerpc/libc-start.c:194

bucket 3 : instances # 6, rank38 0x0108b8f8: /home/robl/src/mpich/src/mpi/comm/comm_free.c:127 0x0105d080: /home/robl/src/mpich/src/mpi/romio/mpi-io/open.c:203 0x01000d84: /home/robl/src/mpich/src/mpi/romio/test/simple.c:71 0x014008e8: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/csu/../csu/libc-start.c:226 0x01400be4: /bgsys/drivers/V1R2M2/ppc64/toolchain/gnu/glibc-2.12.2/csu/../sysdeps/unix/sysv/linux/powerpc/libc-start.c:194

Originally by wscullin on 2015-01-14 16:22:02 -0600

I rebuilt revision f544cbf9921e1536490129e37294253c2f47a989 with all the debugging I could think of on and with the "hello world" came back to the same core dump on exit:

(gdb) bt
#0  OPA_load_int (comm_ptr=0x1662c68, isDisconnect=0)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
#1  MPIR_Comm_release_always (comm_ptr=0x1662c68, isDisconnect=0) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/commutil.c:2111
#2  0x000000000101e994 in MPID_Finalize () at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/src/mpid_finalize.c:107
#3  0x0000000001001a64 in PMPI_Finalize () at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/init/finalize.c:237
#4  0x00000000010009ac in main (argc=1, argv=0x1dbfffbec8) at ../hello.c:16
(gdb) fr 4
#4  0x00000000010009ac in main (argc=1, argv=0x1dbfffbec8) at ../hello.c:16
16                MPI_Finalize();
(gdb) list 16
11
12                MPI_Init (&argc, &argv);      /* starts MPI */
13                MPI_Comm_rank (MPI_COMM_WORLD, &rank);        /* get current process id */
14                MPI_Comm_size (MPI_COMM_WORLD, &size);        /* get number of processes */
15                printf( "Hello world from process %d of %d\n", rank, size );
16                MPI_Finalize();
17                return 0;
18      }
19

This made me think about the destruction of the communicator and made me create a variant "hello world" that does a dup on world and then frees the communicator:

(gdb) bt
#0  OPA_load_int (comm_ptr=0x1666768) at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
#1  MPIR_Comm_release (comm_ptr=0x1666768) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/include/mpiimpl.h:1304
#2  MPIR_Comm_free_impl (comm_ptr=0x1666768) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_free.c:35
#3  0x0000000001001118 in PMPI_Comm_free (comm=0x1dbfffba5c) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_free.c:126
#4  0x0000000001000988 in main (argc=1, argv=0x1dbfffbec8) at ../hello-comm.c:19
(gdb) fr 4
#4  0x0000000001000988 in main (argc=1, argv=0x1dbfffbec8) at ../hello-comm.c:19
19                MPI_Comm_free(&comm_world_two);
(gdb) list
14                MPI_Init (&argc, &argv);      /* starts MPI */
15
16                comm_world = MPI_COMM_WORLD;
17
18                MPI_Comm_dup(comm_world,&comm_world_two);
19                MPI_Comm_free(&comm_world_two);
20
21
22                MPI_Comm_rank (MPI_COMM_WORLD, &rank);        /* get current process id */
23                MPI_Comm_size (MPI_COMM_WORLD, &size);        /* get number of processes */

To eliminate the compiler and environment as the source of issues, I did a xl build:

 export MPICHLIB_CXXFLAGS="-g -O0 -qmaxmem=-1 -qinline=800 -qflag=i:i -qsaveopt -qsuppress=1506-236"
 export MPICHLIB_CFLAGS="${MPICHLIB_CXXFLAGS}"
 export MPICHLIB_FFLAGS="${MPICHLIB_CXXFLAGS}"
 export MPICHLIB_F90FLAGS="${MPICHLIB_CXXFLAGS}"
 CC=`which bgxlc_r`                                    \
 CXX=`which bgxlC_r`                                     \
 F77=`which bgxlf_r`                                     \
 FC=`which bgxlf90_r`                                   \
 AR=`which powerpc64-bgq-linux-ar`         \
 LD=`which powerpc64-bgq-linux-ld`         \
 RANLIB=`which powerpc64-bgq-linux-ranlib` \
 ../mpich-${TARGET_VERSION}/configure \
 --host=powerpc64-bgq-linux \
 --with-device=pamid \
 --with-file-system=gpfs:BGQ \
 --enable-thread-cs=per-object \
 --with-atomic-primitives \
 --enable-handle-allocation=tls \
 --enable-refcount=lock-free \
 --disable-predefined-refcount \
 --prefix=${INSTALL_DIR}/${compiler}
 make | tee m.txt
 make install | tee i.txt

and this resulted in the same failure:

(gdb) bt
#0  PMPI_Comm_free (comm=0x145a8b0) at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
#1  0x0000000001000618 in main (argc=1, argv=0x1dbfffbec8) at ../hello-comm.c:19

repeating the builds with --enable-refcount=lock created binaries that could produce a successful hello world with either example code.

Originally by wscullin on 2015-01-14 16:33:21 -0600

It's worth tossing in an earlier backtrace from a hello world and build done without debug symbols:

[wscullin@vestalac2 mpich-testing]$ powerpc64-bgq-linux-gdb a.out core.1

...

Reading symbols from /gpfs/vesta-fs0/projects/Operations/wscullin/mpich-testing/a.out...done.

warning: core file may not match specified executable file.
[New Thread 1]
Core was generated by `/gpfs/vesta-fs0/projects/Operations/wscullin/mpich-testing/a.out'.
Program terminated with signal 11, Segmentation fault.
#0  0x0000000001008180 in .MPIR_Comm_release_always ()
(gdb) bt
#0  0x0000000001008180 in .MPIR_Comm_release_always ()
#1  0x000000000101e994 in .MPID_Finalize ()
#2  0x0000000001001a64 in .PMPI_Finalize ()
#3  0x00000000010009ac in main (argc=1, argv=0x1dbfffbec8) at hello.c:16
(gdb) list 16
11
12                MPI_Init (&argc, &argv);      /* starts MPI */
13                MPI_Comm_rank (MPI_COMM_WORLD, &rank);        /* get current process id */
14                MPI_Comm_size (MPI_COMM_WORLD, &size);        /* get number of processes */
15                printf( "Hello world from process %d of %d\n", rank, size );
16                MPI_Finalize();
17                return 0;
18      }
19

Originally by robl on 2015-01-14 16:59:28 -0600

Oh man this is going to be a headache. git bisect points to this guy as the culprit:

7a36603163cd917fe89a39cad668e5a7b6f917cb is the first bad commit
commit 7a36603163cd917fe89a39cad668e5a7b6f917cb
Author: Joe Ratterman <jratt@us.ibm.com>
Date:   Mon May 2 13:00:29 2011 -0500

    When decrementing a ref-count to 0, do not bother with atomics.

      If the ref-count is 1, we *should* be the only thread holding a
      reference.  It is therefore legal to skip atomics.  If that is
      insufficient justificaion, consider that there are only three things
      a concurrent thread should be doing:

        1) Reading: Not a problem, since the store is atomic.
        2) Decrementing: Illegal; it would go negative using pure atomics.
        3) Incrementing: Illegal; we are about to destroy the object.

    Add #ifdef guard for use of 'aggressive counter optimizations'

    (ibm) 20a558c3802fb3e138eb6b3c786a434538efffdd

    Signed-off-by: Joe Ratterman <jratt@us.ibm.com>
    Signed-off-by: Michael Blocksome <blocksom@us.ibm.com>

    Updated by Pavan Balaji to use an OPA atomic load instead of a simple
    load operation.

    Fixes #1835

    Signed-off-by: Pavan Balaji <balaji@anl.gov>
    Signed-off-by: William Gropp <wgropp@illinois.edu>

:040000 040000 f0c1c49ebbcb6feefb6917d085b6a1a7551d0b8f 0219e59a3d3108f581345fc09066ddf00361ee39 M        src

Now I need to go see what #1835 is all about

Originally by blocksom on 2015-01-15 14:07:33 -0600

Yeah, if 7a36603163cd917fe89a39cad668e5a7b6f917cb exposes the problem then there is some threading problem elsewhere in the code. Some part of the code is expecting this counter to be atomic when it isn't. This worked in mpich2 1.5+ .. but, admittedly, a lot of code has changed since then.

Debugging the per-object locking code is not easy, especially if you configure/compile as is done for the "non-legacy" bgq mpich installs. Those installs turn on a lot of different options - not just fine-grain locks.

There are environment variables you can set to help narrow down the problem. For example, you could:

turn off async progress
turn off 'multi-context'
when using async progress, limit the number of commthreads (set to 1)
... and others I can't think of right now

I hope this helps! If you get stuck you should bother someone who works at IBM. :)

Originally by wscullin on 2015-01-21 12:49:46 -0600

I ran through all combinations and permutations of the PAMI environment variables without any change of behavior with the fine grained locking crashes. The simple destruction of a communicator kept identifying src/openpa/src/primitives/opa_gcc_ppc.h:20

Setting PAMID_COLLECTIVES=0

#0  OPA_load_int (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
20          return ptr->v;
(gdb) bt
#0  OPA_load_int (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
#1  MPID_Request_release_inline (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/include/../src/mpid_request.h:275
#2  MPIDI_Request_complete_inline (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/include/../src/mpid_request.h:329
#3  MPIDI_SendDoneCB_inline (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/include/../src/pt2pt/mpidi_send.h:40
#4  MPIDI_SendDoneCB (context=0x19c53763e0, clientdata=0x16e0a70, result=PAMI_SUCCESS) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/src/pt2pt/mpidi_done.c:37
#5  0x000000000118ab3c in PAMI::Protocol::Send::EagerSimple<PAMI::Device::MU::PacketModel, (PAMI::Protocol::Send::configuration_t)1>::send_complete(pami_context_t, void *, <anonymous enum>) (context=<value optimized out>, cookie=0x19c5838340, 
    result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/p2p/protocols/send/eager/EagerSimple.h:1213
#6  0x00000000010e6e04 in invoke (this=0x19c569b260) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/msg/CompletionEvent.h:94
#7  advanceCompletion (this=0x19c569b260) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/InjChannel.h:249
#8  PAMI::Device::MU::InjGroup::advanceCompletionQueues (this=0x19c569b260) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/InjGroup.h:238
#9  0x00000000012177b8 in advance (this=0x19c568b180) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/InjGroup.h:201
#10 advance_impl (this=0x19c568b180) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/Context.h:525
#11 PAMI::Device::Interface::BaseDevice<PAMI::Device::MU::Context>::advance (this=0x19c568b180) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/BaseDevice.h:168
#12 0x0000000001217d50 in advance_impl (this=0x19c53763e0, maximum=1, result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/bgq/mu2/Factory.h:269
#13 advance (this=0x19c53763e0, maximum=1, result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/components/devices/FactoryInterface.h:110
#14 advance (this=0x19c53763e0, maximum=1, result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/common/bgq/Context.h:789
#15 advance_impl (this=0x19c53763e0, maximum=1, result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/common/bgq/Context.h:1512
#16 PAMI::Interface::Context<PAMI::Context>::advance(size_t, <anonymous enum> &) (this=0x19c53763e0, maximum=1, result=<value optimized out>) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/common/ContextInterface.h:160
#17 0x00000000010e8c28 in PAMI_Context_trylock_advancev (context=0x16d85b0, count=1, maximum=1) at /bgsys/source/srcV1R2M2.1830/comm/sys/buildtools/pami/api/c/pami.cc:554
#18 0x000000000107f520 in MPID_Progress_wait_inline (request_ptr=0x16e0a70)
    at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpid/pamid/include/../src/mpid_progress.h:259
#19 MPIC_Wait (request_ptr=0x16e0a70) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/coll/helper_fns.c:216
#20 0x0000000001081e6c in MPIC_Sendrecv (sendbuf=0x1dbfffb500, sendcount=<value optimized out>, sendtype=1275069445, dest=<value optimized out>, sendtag=<value optimized out>, recvbuf=<value optimized out>, recvcount=<value optimized out>, 
    recvtype=<value optimized out>, source=26, recvtag=14, comm=<value optimized out>, status=0x1dbfffb140, errflag=0x1dbfffb4f0) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/coll/helper_fns.c:468
#21 0x000000000103bbe8 in MPIR_Allreduce_intra (sendbuf=<value optimized out>, recvbuf=0x1dbfffb500, count=65, datatype=1275069445, op=1476395014, comm_ptr=<value optimized out>, errflag=0x1dbfffb4f0)
    at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/coll/allreduce.c:411
#22 0x000000000103c99c in MPIR_Allreduce_impl (sendbuf=0xffffffffffffffff, recvbuf=0x1dbfffb500, count=65, datatype=1275069445, op=1476395014, comm_ptr=<value optimized out>, errflag=<value optimized out>)
    at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/coll/allreduce.c:768
#23 0x0000000001006408 in MPIR_Get_contextid_sparse_group (comm_ptr=0x1660ee8, group_ptr=0x0, tag=268435455, context_id=0x1dbfffb7d0, ignore_id=0) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/commutil.c:1097
#24 0x0000000001006720 in MPIR_Get_contextid (comm_ptr=<value optimized out>, context_id=<value optimized out>) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/commutil.c:828
#25 0x0000000001007b38 in MPIR_Comm_copy (comm_ptr=<value optimized out>, size=32, outcomm_ptr=0x1dbfffb990) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/commutil.c:1771
#26 0x0000000001000a84 in MPIR_Comm_dup_impl (comm_ptr=0x1660ee8, newcomm_ptr=0x1dbfffb990) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_dup.c:56
#27 0x0000000001000d24 in PMPI_Comm_dup (comm=1140850688, newcomm=0x1dbfffba5c) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_dup.c:161
#28 0x0000000001000978 in main (argc=1, argv=0x1dbfffbec8) at ../hello-comm.c:18

versus

Program terminated with signal 11, Segmentation fault.
#0  OPA_load_int (comm_ptr=0x1666768) at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
20          return ptr->v;
(gdb) bt
#0  OPA_load_int (comm_ptr=0x1666768) at /dev/shm/wscullin/mpich/git.f544cbf9921e1536490129e37294253c2f47a989/20150114_2000/mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/openpa/src/primitives/opa_gcc_ppc.h:20
#1  MPIR_Comm_release (comm_ptr=0x1666768) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/include/mpiimpl.h:1304
#2  MPIR_Comm_free_impl (comm_ptr=0x1666768) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_free.c:35
#3  0x0000000001001118 in PMPI_Comm_free (comm=0x1dbfffba5c) at ../mpich-git.f544cbf9921e1536490129e37294253c2f47a989/src/mpi/comm/comm_free.c:126
#4  0x0000000001000988 in main (argc=1, argv=0x1dbfffbec8) at ../hello-comm.c:19

with PAMID_COLLECTIVES=1

The core files in builds with optimization off and debug on back to the locations mentioned in the compiler warnings a little digging turned up #2186 which may have gone from a minor annoyance to a bug.

../mpich-git/src/include/mpiimpl.h: In function 'MPIR_Comm_release':
../mpich-git/src/include/mpiimpl.h:1304: warning: passing argument 1 of 'OPA_load_int' makes pointer from integer without a cast
/d/s/w/mpich/git/20150121_1757/mpich-git/src/openpa/src/primitives/opa_gcc_ppc.h:18: note: expected 'const struct OPA_int_t *' but argument is of type 'int'
--
../mpich-git/src/mpi/comm/commutil.c: In function 'MPIR_Comm_release_always':
../mpich-git/src/mpi/comm/commutil.c:2111: warning: passing argument 1 of 'OPA_load_int' makes pointer from integer without a cast
/d/s/w/mpich/git/20150121_1757/mpich-git/src/openpa/src/primitives/opa_gcc_ppc.h:18: note: expected 'const struct OPA_int_t *' but argument is of type 'int'
--
In file included from /d/s/w/mpich/git/20150121_1757/mpich-git/src/mpid/pamid/include/mpidpost.h:34,
                 from ../mpich-git/src/include/mpiimpl.h:3868,
                 from ../mpich-git/src/mpi/attr/attr_delete.c:8:
/d/s/w/mpich/git/20150121_1757/mpich-git/src/mpid/pamid/include/../src/mpid_request.h: In function 'MPID_Request_release_inline':
/d/s/w/mpich/git/20150121_1757/mpich-git/src/mpid/pamid/include/../src/mpid_request.h:275: warning: passing argument 1 of 'OPA_load_int' makes pointer from intege
r without a cast
/d/s/w/mpich/git/20150121_1757/mpich-git/src/openpa/src/primitives/opa_gcc_ppc.h:18: note: expected 'const struct OPA_int_t *' but argument is of type 'int'
/d/s/w/mpich/git/20150121_1757/mpich-git/src/mpid/pamid/include/../src/mpid_request.h:285: warning: passing argument 1 of 'OPA_load_int' makes pointer from intege
r without a cast
/d/s/w/mpich/git/20150121_1757/mpich-git/src/openpa/src/primitives/opa_gcc_ppc.h:18: note: expected 'const struct OPA_int_t *' but argument is of type 'int'

Originally by wscullin on 2015-01-21 16:16:39 -0600

I revisited #1835 and Mike had proposed the patch (ibm) 20a558c3802fb3e138eb6b3c786a434538efffdd adding an " #ifdef guard for use of 'aggressive counter optimizations'" that was not merged into 7a36603163cd917fe89a39cad668e5a7b6f917cb as outlined in http://git.mpich.org/mpich-ibm.git/commitdiff/de49d246a65e4da02de94e84f7aa8fa0d391f1f3

Merging Mike's patch with master ( currently 7d59d14dd74072037b0daaa67612836974c0ff2b ) seems to resolve this ticket and partly resolve #2186. I'm not entirely sure what the impact is performance-wise, but it puts me back in a place to run regression tests since basic functionality is restored.

Originally by robl on 2015-01-22 13:32:00 -0600

so will, you're saying the change you made is basically this:

diff --git a/src/include/mpihandlemem.h b/src/include/mpihandlemem.h
index 1ab6752e..71f72c3b 100644
--- a/src/include/mpihandlemem.h
+++ b/src/include/mpihandlemem.h
@@ -301,6 +301,7 @@ typedef OPA_int_t MPIU_Handle_ref_count;
         MPIU_HANDLE_CHECK_REFCOUNT(objptr_,"incr");       \
     } while (0)

+#ifdef MPIU_COUNTER_OPTIMIZATION_AGGRESSIVE
 /* Special optimization: when the reference count touches one, we are
  - about to free the object.  In this case, it is illegal for another
  - thread to either increment the reference (i.e., create a new object
@@ -322,6 +323,15 @@ typedef OPA_int_t MPIU_Handle_ref_count;
         MPIU_HANDLE_LOG_REFCOUNT_CHANGE(objptr_, "decr");               \
         MPIU_HANDLE_CHECK_REFCOUNT(objptr_,"decr");                     \
     } while (0)
+# else
+#  define MPIU_Object_release_ref_always(objptr_,inuse_ptr)               \
+    do {                                                                \
+       int got_zero_ = OPA_decr_and_test_int(&((objptr_)->ref_count)); \
+       *(inuse_ptr) = got_zero_ ? 0 : 1;                               \
+       MPIU_HANDLE_LOG_REFCOUNT_CHANGE(objptr_, "decr");               \
+       MPIU_HANDLE_CHECK_REFCOUNT(objptr_,"decr");                     \
+    } while (0
+# endif
 #else
 #error invalid value for MPIU_THREAD_REFCOUNT
 #endif

if so, send me your change. I'll push it to jenkins (where it will likely pass as nothing defines OPTIMIZATION_AGGRESSIVE, sign off on it if it passes, and push to master.

Originally by balaji on 2015-02-14 18:23:06 -0600

Merging the ifdef part is essentially equivalent to reverting this patch. The "ifdef" part is never defined anywhere.

Originally by blocksom on 2015-02-17 08:41:52 -0600

I think the intent was that IBM would define MPIU_COUNTER_OPTIMIZATION_AGGRESSIVE in its build system for BG/Q once the patch went in.

Originally by balaji on 2015-02-17 16:27:16 -0600

My point is that if it's breaking things then the macro doesn't matter. There's a bug in the rest of the BG/Q code (the patch itself looks OK to me). So the only three reasonable options are:

Fix the bug in the rest of the BG/Q code.
Revert this patch.
Ignore BG/Q. Current BG/Q users can drop that patch. But that assumes that pamid itself works correctly on other platforms.

Protecting it with a macro doesn't make sense to me.

Originally by wscullin on 2015-03-05 16:40:35 -0600

Replying to balaji:

My point is that if it's breaking things then the macro doesn't matter. There's a bug in the rest of the BG/Q code (the patch itself looks OK to me). So the only three reasonable options are:

Fix the bug in the rest of the BG/Q code.

Revert this patch.

Ignore BG/Q. Current BG/Q users can drop that patch. But that assumes that pamid itself works correctly on other platforms.

Protecting it with a macro doesn't make sense to me.

Referring to [comment:37:#1835] Bill Gropp encounters a SEGVs in finalize running cpi which is seemingly unaddressed. Using his options to enable fine-grained locking on Linux (Linux ftlogin2 2.6.32-504.3.3.el6.x86_64 !#1 SMP Fri Dec 12 16:05:43 EST 2014 x86_64 x86_64 x86_64 GNU/Linux) and BG/Q reproduces the SEGV making it a general issue suggesting either ifdefing the patch or reverting it (option 2) is the correct path forward.

Building with Bill's options on x86_64 Linux and running cpi, the core dump shows:

#0  OPA_load_int () at /home/wscullin/src/mpich/src/openpa/src/primitives/opa_gcc_intel_32_64_ops.h:34
#1  MPIDI_PG_Close_VCs () at src/mpid/ch3/src/mpidi_pg.c:1190
#2  0x00000000004233ea in MPID_Finalize () at src/mpid/ch3/src/mpid_finalize.c:101
#3  0x000000000040c5bf in PMPI_Finalize () at src/mpi/init/finalize.c:237
#4  0x000000000040235c in main (argc=1, argv=0x7fff8e841d38) at cpi.c:62
(gdb) list
29      #define OPA_PTR_T_INITIALIZER(val_) { (val_) }
30
31      /* Aligned loads and stores are atomic on x86(-64). */
32      static _opa_inline int OPA_load_int(_opa_const OPA_int_t *ptr)
33      {
34          return ptr->v;
35      }
36
37      /* Aligned loads and stores are atomic on x86(-64). */
38      static _opa_inline void OPA_store_int(OPA_int_t *ptr, int val)

reverting the patch:

diff --git a/src/include/mpihandlemem.h b/src/include/mpihandlemem.h
index 1ab6752..8e6b5a5 100644
--- a/src/include/mpihandlemem.h
+++ b/src/include/mpihandlemem.h
@@ -311,14 +311,8 @@ typedef OPA_int_t MPIU_Handle_ref_count;
  - with load atomicity and no need for load memory barriers. */
 #define MPIU_Object_release_ref_always(objptr_,inuse_ptr)               \
     do {                                                                \
-        if (likely(OPA_load_int((objptr_)->ref_count.v) == 1)) {        \
-            (objptr_)->ref_count.v = 0;                                 \
-            *(inuse_ptr) = 0;                                           \
-        }                                                               \
-        else {                                                          \
             int got_zero_ = OPA_decr_and_test_int(&((objptr_)->ref_count)); \
             -(inuse_ptr) = got_zero_ ? 0 : 1;                           \
-        }                                                               \
         MPIU_HANDLE_LOG_REFCOUNT_CHANGE(objptr_, "decr");               \
         MPIU_HANDLE_CHECK_REFCOUNT(objptr_,"decr");                     \
     } while (0)

leads to successful execution on the same system.

Originally by Rob Latham robl@mcs.anl.gov on 2015-04-02 11:08:20 -0500

In a5686ec3c42f0357119cab7f21df46389c7acec8: Revert "When decrementing a ref-count to 0, do not bother with atomics."

This reverts commit 7a36603163cd917fe89a39cad668e5a7b6f917cb.

At one point (circa MPICH2-1.5) this clever optimization behaved as expected. Once ported forward to more recent MPICH, segfaults on trivial MPI applications.

Closes: #2217 Reopens: #1835

Signed-off-by: Ken Raffenetti raffenet@mcs.anl.gov

pmodels / mpich

fine grained locking causes strange segfaults #2217