Prepare osc framework for bigcount #12379

jtronge · 2024-02-26T20:06:38Z

This updates the osc framework to use size_t for counts and ptrdiff_t for displacements.

devreal

Thanks @hppritcha! It mostly looks good, just two points:

I'm afraid all of the verbose output statements need to be adjusted, like:
https://github.com/open-mpi/ompi/pull/12379/files#diff-e4bd30eae59fa8aabf9ee87dcb523aa60a292e7c09d1e84e014c1ae7e13e1e3cR1145
I think the disp_units members of the modules need to become ptrdiff_t (currently int) since MPI_Win_create_c takes MPI_Aint for the displacements.

wenduwan · 2024-02-28T01:48:29Z

Running AWS CI

wenduwan · 2024-02-28T14:56:57Z

AWS CI failed due to OMB onesided suite. Will provide more info.

hppritcha · 2024-02-28T15:39:36Z

You mean IMB right?

wenduwan · 2024-02-28T17:00:35Z

Example failure

mpirun --wdir . -n 2  osu-micro-benchmarks/mpi/one-sided/osu_put_bw
# OSU MPI_Put Bandwidth Test v7.0-lrbison3
# Window creation: MPI_Win_allocate
# Synchronization: MPI_Win_flush
# Size      Bandwidth (MB/s)
1                      11.14
2                       8.96
segfault

Backtrace

#0  0x00007f94d52edc1a in __memmove_avx_unaligned_erms () from /lib64/libc.so.6
#1  0x00007f94d4f159bc in opal_datatype_accelerator_memcpy (dest=0x7f94d6140100, src=0x187c050, size=4) at opal_datatype_copy.c:74
#2  0x00007f94d4f1648c in non_overlap_accelerator_copy_content_same_ddt (datatype=0x6176e0 <ompi_mpi_char>, count=4,
    destination_base=0x7f94d6140100 "\206\006A\016\070\203\aG\016@\002L\n\016\070D\016\060A\016(B\016 B\016\030B\016\020B\016\bA\v",
    source_base=0x187c050 'a' <repeats 176 times>) at opal_datatype_copy.h:151
#3  0x00007f94d4f1805b in opal_datatype_copy_content_same_ddt (datatype=0x6176e0 <ompi_mpi_char>, count=4,
    destination_base=0x7f94d6140100 "\206\006A\016\070\203\aG\016@\002L\n\016\070D\016\060A\016(B\016 B\016\030B\016\020B\016\bA\v",
    source_base=0x187c050 'a' <repeats 176 times>) at opal_datatype_copy.c:176
#4  0x00007f94d5803bc8 in ompi_datatype_sndrcv (sbuf=0x187c050, scount=4, sdtype=0x6176e0 <ompi_mpi_char>, rbuf=0x7f94d6140100, rcount=4,
    rdtype=0x6176e0 <ompi_mpi_char>) at ompi_datatype_sndrcv.c:62
#5  0x00007f94c40250fa in ompi_osc_sm_put (origin_addr=0x187c050, origin_count=4, origin_dt=0x6176e0 <ompi_mpi_char>, target=1,
    target_disp=80, target_count=4, target_dt=0x6176e0 <ompi_mpi_char>, win=0x1863050) at osc_sm_comm.c:232
#6  0x00007f94d585f97b in PMPI_Put (origin_addr=0x187c050, origin_count=4, origin_datatype=0x6176e0 <ompi_mpi_char>, target_rank=1,
    target_disp=80, target_count=4, target_datatype=0x6176e0 <ompi_mpi_char>, win=0x1863050) at put.c:82
#7  0x0000000000403150 in run_put_with_flush (rank=0, type=WIN_ALLOCATE) at osu_put_bw.c:268
#8  0x00000000004029c3 in main (argc=<optimized out>, argv=<optimized out>) at osu_put_bw.c:129

hppritcha · 2024-02-28T17:55:29Z

having problems reproducing your findings @wenduwan . what are you config options and compiler used? and also processor type?

wenduwan · 2024-02-28T17:59:10Z

./configure --with-sge --without-verbs --with-libfabric=/opt/amazon/efa --disable-man-pages --enable-ipv6 LDFLAGS=-Wl,--as-needed --enable-prte-prefix-by-default --enable-mca-dso=all --with-libevent=external --with-hwloc=external --enable-debug

Note that I don't imagine libfabric to be an issue since the failure happens in sm

The segfault happens on:

Amazon Linux 2 (GCC7) + x86
Amazon LInux 2 (GCC7) + arm
Ubuntu 20.04 + x86
Ubuntu 20.04 + arm

hppritcha · 2024-02-28T20:07:48Z

@wenduwan could you please rerun AWS CI. @jtronge pushed something to hopefully fix what you're seeing.

wenduwan · 2024-02-28T21:43:14Z

Thanks. The test passed.

hppritcha · 2024-03-12T14:44:16Z

@devreal could you take at this PR again to see if your questions were addressed?

devreal

There are a few places where the format strings need to be adjusted

devreal · 2024-03-12T14:51:09Z

ompi/mca/osc/rdma/osc_rdma_comm.c

@@ -923,7 +923,7 @@ int ompi_osc_rdma_rget (void *origin_addr, int origin_count, ompi_datatype_t *or
    ompi_osc_rdma_sync_t *sync;
    int ret;

-    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %d, %s, %d, %d, %d, %s, %s", (unsigned long) origin_addr,
+    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,


origin_count is now size_t:

Suggested change

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %zu, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

devreal · 2024-03-12T14:51:31Z

ompi/mca/osc/rdma/osc_rdma_comm.c

                       ompi_datatype_t *source_datatype, ompi_win_t *win)
 {
    ompi_osc_rdma_module_t *module = GET_MODULE(win);
    ompi_osc_rdma_peer_t *peer;
    ompi_osc_rdma_sync_t *sync;

-    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "get: 0x%lx, %d, %s, %d, %d, %d, %s, %s", (unsigned long) origin_addr,
+    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "get: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,


origin_count is now size_t

Suggested change

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "get: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "get: 0x%lx, %zu, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

devreal · 2024-03-12T14:51:50Z

ompi/mca/osc/rdma/osc_rdma_comm.c

@@ -867,7 +867,7 @@ int ompi_osc_rdma_rput (const void *origin_addr, int origin_count, ompi_datatype
    ompi_osc_rdma_sync_t *sync;
    int ret;

-    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rput: 0x%lx, %d, %s, %d, %d, %d, %s, %s", (unsigned long) origin_addr, origin_count,
+    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rput: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr, origin_count,


origin_count is now size_t

Suggested change

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rput: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr, origin_count,

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rput: 0x%lx, %zu, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr, origin_count,

devreal · 2024-03-12T14:52:01Z

ompi/mca/osc/rdma/osc_rdma_comm.c

                       ompi_datatype_t *target_datatype, ompi_win_t *win)
 {
    ompi_osc_rdma_module_t *module = GET_MODULE(win);
    ompi_osc_rdma_peer_t *peer;
    ompi_osc_rdma_sync_t *sync;

-    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "put: 0x%lx, %d, %s, %d, %d, %d, %s, %s", (unsigned long) origin_addr,
+    OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "put: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,


origin_count is now size_t

Suggested change

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "put: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "put: 0x%lx, %zu, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

devreal · 2024-03-12T14:55:18Z

ompi/mca/osc/portals4/osc_portals4_comm.c

@@ -3194,7 +3194,7 @@ ompi_osc_portals4_get_accumulate(const void *origin_addr,
    ptrdiff_t length, origin_lb, target_lb, result_lb, extent;

    OPAL_OUTPUT_VERBOSE((50, ompi_osc_base_framework.framework_output,
-                         "get_accumulate: 0x%lx, %d, %s, 0x%lx, %d, %s, %d, %lu, %d, %s, %s, 0x%lx",
+                         "get_accumulate: 0x%lx, %zu, %s, 0x%lx, %d, %s, %d, %lu, %zu, %s, %s, 0x%lx",


Suggested change

"get_accumulate: 0x%lx, %zu, %s, 0x%lx, %d, %s, %d, %lu, %zu, %s, %s, 0x%lx",

"get_accumulate: 0x%lx, %zu, %s, 0x%lx, %zu, %s, %d, %lu, %zu, %s, %s, 0x%lx",

Update the osc framework to use size_t for counts and ptrdiff_t for displacements. Signed-off-by: Jake Tronge <[email protected]>

jtronge · 2024-03-12T16:26:08Z

Thanks @devreal. I think all the format strings should be fixed.

github-actions bot added the Target: main label Feb 26, 2024

hppritcha requested a review from devreal February 26, 2024 21:35

devreal requested changes Feb 26, 2024

View reviewed changes

jtronge force-pushed the osc-framework-bigcount branch from 0a66a0b to 449e59a Compare February 27, 2024 00:01

hppritcha requested a review from devreal March 12, 2024 14:44

devreal requested changes Mar 12, 2024

View reviewed changes

jtronge force-pushed the osc-framework-bigcount branch from fe3e9ed to 4846192 Compare March 12, 2024 15:58

Prepare osc framework for bigcount

8bcbe17

Update the osc framework to use size_t for counts and ptrdiff_t for displacements. Signed-off-by: Jake Tronge <[email protected]>

jtronge force-pushed the osc-framework-bigcount branch from 4846192 to 8bcbe17 Compare March 12, 2024 16:10

devreal approved these changes Mar 12, 2024

View reviewed changes

hppritcha merged commit 44880f6 into open-mpi:main Mar 12, 2024
11 checks passed

devreal mentioned this pull request Apr 2, 2024

Fix incorrect disp_unit type passed to osc_win_shared_query #12451

Merged

devreal mentioned this pull request Apr 26, 2024

Fix format specifier mismatch in osc/rdma #12499

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare osc framework for bigcount #12379

Prepare osc framework for bigcount #12379

jtronge commented Feb 26, 2024

devreal left a comment

wenduwan commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Mar 12, 2024

devreal left a comment

devreal Mar 12, 2024

devreal Mar 12, 2024

devreal Mar 12, 2024

devreal Mar 12, 2024

devreal Mar 12, 2024

jtronge commented Mar 12, 2024

	OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %d, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,
	OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "rget: 0x%lx, %zu, %s, %d, %d, %zu, %s, %s", (unsigned long) origin_addr,

	"get_accumulate: 0x%lx, %zu, %s, 0x%lx, %d, %s, %d, %lu, %zu, %s, %s, 0x%lx",
	"get_accumulate: 0x%lx, %zu, %s, 0x%lx, %zu, %s, %d, %lu, %zu, %s, %s, 0x%lx",

Prepare osc framework for bigcount #12379

Prepare osc framework for bigcount #12379

Conversation

jtronge commented Feb 26, 2024

devreal left a comment

Choose a reason for hiding this comment

wenduwan commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Feb 28, 2024

wenduwan commented Feb 28, 2024

hppritcha commented Mar 12, 2024

devreal left a comment

Choose a reason for hiding this comment

devreal Mar 12, 2024

Choose a reason for hiding this comment

devreal Mar 12, 2024

Choose a reason for hiding this comment

devreal Mar 12, 2024

Choose a reason for hiding this comment

devreal Mar 12, 2024

Choose a reason for hiding this comment

devreal Mar 12, 2024

Choose a reason for hiding this comment

jtronge commented Mar 12, 2024