From b08a5e3f24432cd16aa8ca1f3d0bb5984e089fc9 Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Thu, 4 Jun 2026 12:45:48 -0700 Subject: [PATCH 1/7] symmetric heap: use anonymous MAP_HUGETLB for huge page allocation When SYMMETRIC_HEAP_USE_HUGE_PAGES is enabled but no hugetlbfs mount is configured, use anonymous MAP_HUGETLB with explicit 2MB page size instead of falling back to transparent huge pages (THP). This allows the kernel's nr_overcommit_hugepages mechanism to dynamically allocate surplus huge pages on demand without requiring pre-reserved HugePages_Total. Anonymous MAP_HUGETLB with (21 << MAP_HUGE_SHIFT) explicitly requests 2MB pages, matching the CXI provider's behavior. When used with scalable MR mode (--enable-ofi-mr=scalable), this enables the CXI NIC's ATU to use 2MB page translations (derivative1) instead of 4KB base pages, significantly improving ATU cache hit rates. Fallback to THP via madvise(MADV_HUGEPAGE) still occurs if MAP_HUGETLB fails, ensuring compatibility across different kernel configurations. Co-Authored-By: Claude Sonnet 4.5 --- src/symmetric_heap_c.c | 54 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/symmetric_heap_c.c b/src/symmetric_heap_c.c index 30b319ea9..e95b92635 100644 --- a/src/symmetric_heap_c.c +++ b/src/symmetric_heap_c.c @@ -201,14 +201,66 @@ static void *mmap_alloc(size_t bytes) } } } -#endif /* __linux__ */ + if (fd) { + /* Map the hugetlbfs file directly; MAP_ANON must not be used here + * because MAP_ANONYMOUS causes the kernel to ignore the fd, which + * would silently fall back to regular pages. */ + if (ftruncate(fd, bytes) == -1) { + RAISE_WARN_MSG("ftruncate on hugetlbfs file failed (%s), " + "falling back to transparent huge pages via madvise\n", + strerror(errno)); + unlink(file_name); + close(fd); + free(directory); + free(file_name); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, + MAP_ANON | MAP_PRIVATE, -1, 0); + if (ret != MAP_FAILED) { + if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { + DEBUG_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages", + strerror(errno)); + } + } + } else { + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_HUGETLB, fd, 0); + unlink(file_name); + close(fd); + free(directory); + free(file_name); + } + } else if (shmem_internal_params.SYMMETRIC_HEAP_USE_HUGE_PAGES) { + /* Try anonymous MAP_HUGETLB first (works with nr_overcommit_hugepages). + * Explicitly request 2MB pages via MAP_HUGE_SHIFT (21 << MAP_HUGE_SHIFT = 2^21 = 2MB). */ + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, + MAP_ANON | MAP_PRIVATE | MAP_HUGETLB | (21 << MAP_HUGE_SHIFT), -1, 0); + if (ret == MAP_FAILED) { + DEBUG_MSG("mmap(MAP_HUGETLB) failed (%s), falling back to THP via madvise", + strerror(errno)); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, + MAP_ANON | MAP_PRIVATE, -1, 0); + if (ret != MAP_FAILED) { + if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { + RAISE_WARN_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages\n", + strerror(errno)); + } + } + } else { + DEBUG_MSG("Allocated symmetric heap with explicit huge pages (MAP_HUGETLB), %zu bytes", + bytes); + } + } else { + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + } +#else ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, fd, 0); +#endif /* __linux__ */ if (ret == MAP_FAILED) { RAISE_WARN_MSG("Unable to allocate sym. heap, size %zuB: %s\n" RAISE_PE_PREFIX From c3770a505bf1b7fffc8f62500739a46cf52a546d Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Fri, 5 Jun 2026 11:51:53 -0700 Subject: [PATCH 2/7] Formatting --- src/symmetric_heap_c.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/symmetric_heap_c.c b/src/symmetric_heap_c.c index e95b92635..287264dfe 100644 --- a/src/symmetric_heap_c.c +++ b/src/symmetric_heap_c.c @@ -167,9 +167,7 @@ static void *mmap_alloc(size_t bytes) char *file_name = NULL; int fd = 0; char *directory = NULL; - void *requested_base = - (void*) (((unsigned long) shmem_internal_data_base + - shmem_internal_data_length + 2 * ONEGIG) & ~(ONEGIG - 1)); + void *requested_base = (void*) (((unsigned long) shmem_internal_data_base + shmem_internal_data_length + 2 * ONEGIG) & ~(ONEGIG - 1)); void *ret; #ifdef __linux__ @@ -207,24 +205,19 @@ static void *mmap_alloc(size_t bytes) * because MAP_ANONYMOUS causes the kernel to ignore the fd, which * would silently fall back to regular pages. */ if (ftruncate(fd, bytes) == -1) { - RAISE_WARN_MSG("ftruncate on hugetlbfs file failed (%s), " - "falling back to transparent huge pages via madvise\n", - strerror(errno)); + RAISE_WARN_MSG("ftruncate on hugetlbfs file failed (%s), falling back to transparent huge pages via madvise\n", strerror(errno)); unlink(file_name); close(fd); free(directory); free(file_name); - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, - MAP_ANON | MAP_PRIVATE, -1, 0); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { - DEBUG_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages", - strerror(errno)); + DEBUG_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages", strerror(errno)); } } } else { - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, - MAP_SHARED | MAP_HUGETLB, fd, 0); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_HUGETLB, fd, 0); unlink(file_name); close(fd); free(directory); @@ -233,22 +226,17 @@ static void *mmap_alloc(size_t bytes) } else if (shmem_internal_params.SYMMETRIC_HEAP_USE_HUGE_PAGES) { /* Try anonymous MAP_HUGETLB first (works with nr_overcommit_hugepages). * Explicitly request 2MB pages via MAP_HUGE_SHIFT (21 << MAP_HUGE_SHIFT = 2^21 = 2MB). */ - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, - MAP_ANON | MAP_PRIVATE | MAP_HUGETLB | (21 << MAP_HUGE_SHIFT), -1, 0); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE | MAP_HUGETLB | (21 << MAP_HUGE_SHIFT), -1, 0); if (ret == MAP_FAILED) { - DEBUG_MSG("mmap(MAP_HUGETLB) failed (%s), falling back to THP via madvise", - strerror(errno)); - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, - MAP_ANON | MAP_PRIVATE, -1, 0); + DEBUG_MSG("mmap(MAP_HUGETLB) failed (%s), falling back to THP via madvise", strerror(errno)); + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { - RAISE_WARN_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages\n", - strerror(errno)); + RAISE_WARN_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages\n", strerror(errno)); } } } else { - DEBUG_MSG("Allocated symmetric heap with explicit huge pages (MAP_HUGETLB), %zu bytes", - bytes); + DEBUG_MSG("Allocated symmetric heap with explicit huge pages (MAP_HUGETLB), %zu bytes", bytes); } } else { ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); From 8f0d93d4b8c4d3f758b0325cd8b6730dd115c52a Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Fri, 5 Jun 2026 11:53:41 -0700 Subject: [PATCH 3/7] symmetric heap: change hugetlbfs file warning to debug message Change the hugetlbfs file open failure from RAISE_WARN_STR to DEBUG_MSG since the fallback to anonymous MAP_HUGETLB works correctly. The warning was misleading because huge pages were still being allocated successfully via the anonymous MAP_HUGETLB path. Co-Authored-By: Claude Sonnet 4.5 --- src/symmetric_heap_c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/symmetric_heap_c.c b/src/symmetric_heap_c.c index 287264dfe..70f852565 100644 --- a/src/symmetric_heap_c.c +++ b/src/symmetric_heap_c.c @@ -189,7 +189,7 @@ static void *mmap_alloc(size_t bytes) sprintf(file_name, "%s/%s.%d", directory, basename, getpid()); fd = open(file_name, O_CREAT | O_RDWR, 0755); if (fd < 0) { - RAISE_WARN_STR("file open failed, cannot use huge pages"); + DEBUG_MSG("file open failed, will fall back to anonymous MAP_HUGETLB"); fd = 0; } else { /* have to round up by the pagesize being used */ From 9fc4bb4ea87c8970e7d5a559bc797b0921aa3667 Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Tue, 9 Jun 2026 07:13:49 -0700 Subject: [PATCH 4/7] symmetric heap: fix huge page allocation bugs and improve fallback handling - Fix double free: set directory/file_name to NULL after freeing - Fix size bug: preserve original bytes, only use hugetlbfs_bytes for file path - Fix fallback: use NULL address hint after MAP_HUGETLB failure - Add debug visibility: log which allocation path succeeded - Change hugetlbfs warnings to debug messages (fallback works correctly) Co-Authored-By: Claude Sonnet 4.5 --- src/symmetric_heap_c.c | 44 +++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/symmetric_heap_c.c b/src/symmetric_heap_c.c index 70f852565..c1c90969c 100644 --- a/src/symmetric_heap_c.c +++ b/src/symmetric_heap_c.c @@ -169,6 +169,7 @@ static void *mmap_alloc(size_t bytes) char *directory = NULL; void *requested_base = (void*) (((unsigned long) shmem_internal_data_base + shmem_internal_data_length + 2 * ONEGIG) & ~(ONEGIG - 1)); void *ret; + size_t hugetlbfs_bytes = 0; /* Rounded size for hugetlbfs, 0 if not used */ #ifdef __linux__ /* huge page support only on Linux for now, default is to use 2MB large pages */ @@ -190,38 +191,55 @@ static void *mmap_alloc(size_t bytes) fd = open(file_name, O_CREAT | O_RDWR, 0755); if (fd < 0) { DEBUG_MSG("file open failed, will fall back to anonymous MAP_HUGETLB"); + free(directory); + free(file_name); + directory = NULL; + file_name = NULL; fd = 0; } else { - /* have to round up by the pagesize being used */ - bytes = CEILING(bytes, shmem_internal_params.SYMMETRIC_HEAP_PAGE_SIZE); + /* Round up by the pagesize for hugetlbfs file */ + hugetlbfs_bytes = CEILING(bytes, shmem_internal_params.SYMMETRIC_HEAP_PAGE_SIZE); } } } } } + DEBUG_MSG("mmap_alloc: bytes=%zu, hugetlbfs_bytes=%zu, fd=%d", + bytes, hugetlbfs_bytes, fd); + if (fd) { /* Map the hugetlbfs file directly; MAP_ANON must not be used here * because MAP_ANONYMOUS causes the kernel to ignore the fd, which * would silently fall back to regular pages. */ - if (ftruncate(fd, bytes) == -1) { - RAISE_WARN_MSG("ftruncate on hugetlbfs file failed (%s), falling back to transparent huge pages via madvise\n", strerror(errno)); + if (ftruncate(fd, hugetlbfs_bytes) == -1) { + DEBUG_MSG("ftruncate on hugetlbfs file failed (%s), falling back to THP", strerror(errno)); unlink(file_name); close(fd); free(directory); free(file_name); - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + directory = NULL; + file_name = NULL; + fd = 0; + /* Use NULL to let kernel choose address for fallback */ + ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { DEBUG_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages", strerror(errno)); } } } else { - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_HUGETLB, fd, 0); + ret = mmap(requested_base, hugetlbfs_bytes, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_HUGETLB, fd, 0); + if (ret != MAP_FAILED) { + DEBUG_MSG("Allocated symmetric heap via hugetlbfs file: %zu bytes", hugetlbfs_bytes); + } unlink(file_name); close(fd); free(directory); free(file_name); + directory = NULL; + file_name = NULL; + fd = 0; } } else if (shmem_internal_params.SYMMETRIC_HEAP_USE_HUGE_PAGES) { /* Try anonymous MAP_HUGETLB first (works with nr_overcommit_hugepages). @@ -229,14 +247,15 @@ static void *mmap_alloc(size_t bytes) ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE | MAP_HUGETLB | (21 << MAP_HUGE_SHIFT), -1, 0); if (ret == MAP_FAILED) { DEBUG_MSG("mmap(MAP_HUGETLB) failed (%s), falling back to THP via madvise", strerror(errno)); - ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + /* Use NULL to let kernel choose address - requested_base may not work after MAP_HUGETLB failure */ + ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { RAISE_WARN_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages\n", strerror(errno)); } } } else { - DEBUG_MSG("Allocated symmetric heap with explicit huge pages (MAP_HUGETLB), %zu bytes", bytes); + DEBUG_MSG("Allocated symmetric heap via anonymous MAP_HUGETLB (2MB pages): %zu bytes", bytes); } } else { ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); @@ -256,17 +275,16 @@ static void *mmap_alloc(size_t bytes) bytes, strerror(errno), shmem_internal_my_pe); ret = NULL; } - if (fd) { - if (file_name) - unlink(file_name); - close(fd); - } + /* Cleanup any remaining allocations (will be NULL if already freed above) */ if (directory) { free(directory); } if (file_name) { free(file_name); } + if (fd > 0) { + close(fd); + } return ret; } From da8071b8a7c5c34d7a3bf3e72e1ff8f05eb6f8fc Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Tue, 9 Jun 2026 08:07:07 -0700 Subject: [PATCH 5/7] symmetric heap: fix munmap size mismatch and preserve requested_base in fallbacks - Fix Issue 1 (munmap size mismatch): add size_t *mapped_bytes out-parameter to mmap_alloc(). On the hugetlbfs success path the mapping is rounded up to a huge-page boundary (hugetlbfs_bytes > bytes); the previous code passed the original unrounded size to munmap and transport registration (OFI, Portals4, UCX, XPMEM), leaking the tail pages from the huge-page pool. mmap_alloc now reports the actual mapped size, and shmem_internal_symmetric_init updates shmem_internal_heap_length accordingly so munmap, registration, and bounds checks all use the correct extent. - Fix Issue 2 (requested_base dropped on fallback): both THP fallback paths (ftruncate failure and MAP_HUGETLB failure) previously used mmap(NULL, ...) unconditionally. This discards the requested_base hint (data segment + 2 GB, 1 GB-aligned) that is required for --enable-remote-virtual-addressing to maintain symmetric virtual addresses across PEs. The fallbacks now first attempt mmap(requested_base, ...) and only resort to mmap(NULL, ...) if that also fails. Remove the incorrect comment claiming requested_base will not work after MAP_HUGETLB failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/symmetric_heap_c.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/symmetric_heap_c.c b/src/symmetric_heap_c.c index c1c90969c..6d67d7064 100644 --- a/src/symmetric_heap_c.c +++ b/src/symmetric_heap_c.c @@ -162,7 +162,7 @@ shmem_internal_get_next(intptr_t incr) /* alloc VM space starting @ '_end' + 1GB */ #define ONEGIG (1024UL*1024UL*1024UL) -static void *mmap_alloc(size_t bytes) +static void *mmap_alloc(size_t bytes, size_t *mapped_bytes) { char *file_name = NULL; int fd = 0; @@ -171,6 +171,8 @@ static void *mmap_alloc(size_t bytes) void *ret; size_t hugetlbfs_bytes = 0; /* Rounded size for hugetlbfs, 0 if not used */ + *mapped_bytes = bytes; /* default: actual mapped size equals requested size */ + #ifdef __linux__ /* huge page support only on Linux for now, default is to use 2MB large pages */ if (shmem_internal_params.SYMMETRIC_HEAP_USE_HUGE_PAGES) { @@ -221,8 +223,11 @@ static void *mmap_alloc(size_t bytes) directory = NULL; file_name = NULL; fd = 0; - /* Use NULL to let kernel choose address for fallback */ - ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + /* Prefer requested_base to preserve virtual address symmetry (required for RVA); + * only use NULL as a last resort if requested_base is unavailable. */ + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + if (ret == MAP_FAILED) + ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { DEBUG_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages", strerror(errno)); @@ -232,6 +237,7 @@ static void *mmap_alloc(size_t bytes) ret = mmap(requested_base, hugetlbfs_bytes, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_HUGETLB, fd, 0); if (ret != MAP_FAILED) { DEBUG_MSG("Allocated symmetric heap via hugetlbfs file: %zu bytes", hugetlbfs_bytes); + *mapped_bytes = hugetlbfs_bytes; } unlink(file_name); close(fd); @@ -247,8 +253,12 @@ static void *mmap_alloc(size_t bytes) ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE | MAP_HUGETLB | (21 << MAP_HUGE_SHIFT), -1, 0); if (ret == MAP_FAILED) { DEBUG_MSG("mmap(MAP_HUGETLB) failed (%s), falling back to THP via madvise", strerror(errno)); - /* Use NULL to let kernel choose address - requested_base may not work after MAP_HUGETLB failure */ - ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + /* Prefer requested_base to preserve virtual address symmetry (required for RVA); + * only use NULL as a last resort. MAP_HUGETLB failure means huge pages are + * unavailable, not that the virtual address range is blocked. */ + ret = mmap(requested_base, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); + if (ret == MAP_FAILED) + ret = mmap(NULL, bytes, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (ret != MAP_FAILED) { if (madvise(ret, bytes, MADV_HUGEPAGE) != 0) { RAISE_WARN_MSG("madvise(MADV_HUGEPAGE) failed (%s), using regular pages\n", strerror(errno)); @@ -297,9 +307,13 @@ shmem_internal_symmetric_init(void) SHMEM_INTERNAL_HEAP_OVERHEAD; if (!shmem_internal_params.SYMMETRIC_HEAP_USE_MALLOC) { + size_t mapped_length = shmem_internal_heap_length; shmem_internal_heap_base = shmem_internal_heap_curr = - mmap_alloc(shmem_internal_heap_length); + mmap_alloc(shmem_internal_heap_length, &mapped_length); + /* Use the actual mapped size for munmap and transport registration. + * On the hugetlbfs path this may be rounded up to a huge-page boundary. */ + shmem_internal_heap_length = mapped_length; } else { shmem_internal_heap_base = shmem_internal_heap_curr = From 928d9f4ec4f15227a08e0743365407528afb42a3 Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Thu, 11 Jun 2026 04:44:35 -0700 Subject: [PATCH 6/7] Fix put completion handling in copy_self Use put_quiet instead of unreliable completion watermark. The watermark approach fails because put_nb uses different code paths (inject, bounce buffer, put_large) that don't all update the completion counter reliably. put_quiet guarantees all in-flight puts complete regardless of path taken. Co-authored-by: GitHub Copilot --- src/shmem_comm.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/shmem_comm.h b/src/shmem_comm.h index 3a3381834..1d9f2d140 100644 --- a/src/shmem_comm.h +++ b/src/shmem_comm.h @@ -425,13 +425,15 @@ static inline void shmem_internal_copy_self(void *dest, const void *source, size_t nelems) { #ifdef USE_FI_HMEM - // "completion" set to 1 to wait for completion of put operation initiated - // by shmem_internal_put_nb, even if "completion" not incremented in call - // to shmem_internal_put_nb. - long completion = 1; + /* put_nb may use inject (no counter event), bounce buffer (counter not + * recorded in *completion), or put_large (counter watermark set). A + * watermark of 1 is not reliable across all paths, so drain all + * in-flight puts via put_quiet to guarantee dest is visible before + * returning. */ + long completion = 0; shmem_internal_put_nb(SHMEM_CTX_DEFAULT, dest, source, nelems, shmem_internal_my_pe, &completion); - shmem_internal_put_wait(SHMEM_CTX_DEFAULT, &completion); + shmem_transport_put_quiet((shmem_transport_ctx_t *)SHMEM_CTX_DEFAULT); #else memcpy(dest, source, nelems); #endif From 222d466ffa5f096ea665e2fddeca54f52b6edfad Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Thu, 11 Jun 2026 05:20:27 -0700 Subject: [PATCH 7/7] Refine copy_self comment - explain inject path race condition Update comment to better explain why put_quiet is necessary. The inject path has no counter event, so put_wait with completion=0 would return immediately and leave GPU writes unordered, causing a data race. put_quiet provides the NIC-level ordering fence needed for correctness. Co-authored-by: GitHub Copilot --- src/shmem_comm.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/shmem_comm.h b/src/shmem_comm.h index 1d9f2d140..c36499e53 100644 --- a/src/shmem_comm.h +++ b/src/shmem_comm.h @@ -425,11 +425,14 @@ static inline void shmem_internal_copy_self(void *dest, const void *source, size_t nelems) { #ifdef USE_FI_HMEM - /* put_nb may use inject (no counter event), bounce buffer (counter not - * recorded in *completion), or put_large (counter watermark set). A - * watermark of 1 is not reliable across all paths, so drain all - * in-flight puts via put_quiet to guarantee dest is visible before - * returning. */ + /* put_nb routes through inject, bounce-buffer, or put_large depending on + * size. The inject path has no counter event, so put_wait (watermark- + * based) is not sufficient — it would return immediately with completion=0 + * and leave the GPU write unordered. put_quiet drains all pending puts + * and provides the NIC-level ordering fence needed to guarantee dest is + * visible at the target GPU before returning. + * bounce-buffer and put_large also set *completion, but put_quiet subsumes + * that wait, so no separate put_wait call is needed. */ long completion = 0; shmem_internal_put_nb(SHMEM_CTX_DEFAULT, dest, source, nelems, shmem_internal_my_pe, &completion);