From f65679909302debb275536d6130b14d3b3557bfd Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Thu, 1 Oct 2026 22:01:28 +0000 Subject: [PATCH 1/4] pml/ob1: defer accelerator stream and event creation to first use Previously, mca_pml_ob1_accelerator_init() unconditionally created two accelerator streams and 2*N events (default N=400) at MPI_Init time whenever a non-null accelerator component was selected, even if no device buffers were ever passed to MPI. Split the initialization into two phases. The existing mca_pml_ob1_accelerator_init() now only allocates the lightweight ring-buffer bookkeeping structures (pointer arrays and counters). A new static mca_pml_ob1_accelerator_lazy_init() creates the streams and events on first actual use, called through the public mca_pml_ob1_accelerator_ensure_init() wrapper. This also fixes a correctness issue: streams and events created at MPI_Init time capture whatever accelerator device happened to be selected at that point. Applications that set their device after MPI_Init would get streams and events bound to the wrong device. Deferring creation to the first MPI call that passes a device buffer ensures the resources are created in the correct device context. Lazy init is triggered from: - mca_pml_ob1_record_htod_event() before acquiring the htod lock - mca_pml_ob1_get_dtoh_stream() and mca_pml_ob1_get_htod_stream() (covering the COPY_ASYNC BTL path, e.g. btl/smcuda) - mca_pml_ob1_recv_request_progress_rget() when the receive buffer is a device buffer (covering the btl/sm + smsc/accelerator RGET path) - mca_pml_ob1_send_request_start_accelerator() for large device buffer sends A new MCA parameter pml_ob1_accelerator_lazy_init (default 1) allows the eager behavior to be restored for environments where streams and events should be pre-created before any GPU operation. Signed-off-by: Edgar Gabriel Signed-off-by: Edgar Gabriel --- ompi/mca/pml/ob1/pml_ob1.h | 2 + ompi/mca/pml/ob1/pml_ob1_accelerator.c | 149 ++++++++++++++++++------- ompi/mca/pml/ob1/pml_ob1_accelerator.h | 2 + ompi/mca/pml/ob1/pml_ob1_component.c | 7 ++ ompi/mca/pml/ob1/pml_ob1_recvreq.c | 4 + 5 files changed, 121 insertions(+), 43 deletions(-) diff --git a/ompi/mca/pml/ob1/pml_ob1.h b/ompi/mca/pml/ob1/pml_ob1.h index 130c717afea..62712b02104 100644 --- a/ompi/mca/pml/ob1/pml_ob1.h +++ b/ompi/mca/pml/ob1/pml_ob1.h @@ -92,6 +92,8 @@ struct mca_pml_ob1_t { unsigned int unexpected_limit; /* Accelerator support initialized */ bool accelerator_enabled; + /* Defer stream/event creation to first device buffer use */ + bool accelerator_lazy_init; }; typedef struct mca_pml_ob1_t mca_pml_ob1_t; diff --git a/ompi/mca/pml/ob1/pml_ob1_accelerator.c b/ompi/mca/pml/ob1/pml_ob1_accelerator.c index 4bc7a5aab7b..682e25c1889 100644 --- a/ompi/mca/pml/ob1/pml_ob1_accelerator.c +++ b/ompi/mca/pml/ob1/pml_ob1_accelerator.c @@ -53,6 +53,9 @@ static opal_accelerator_stream_t *htod_stream = NULL; static opal_mutex_t pml_ob1_accelerator_htod_lock; static opal_mutex_t pml_ob1_accelerator_dtoh_lock; +/* Set to true once streams and events have been created. */ +static bool pml_ob1_accelerator_streams_initialized = false; + /* Array of accelerator events to be queried for sending side and * receiving side. */ static opal_accelerator_event_t **accelerator_event_dtoh_array = NULL; @@ -75,6 +78,73 @@ static volatile int accelerator_event_dtoh_num_used, accelerator_event_htod_num_ /* Size of array holding events */ static int accelerator_event_htod_most = 0; +/* Create streams and events on first use of a device buffer. + * At runtime, callers must hold pml_ob1_accelerator_htod_lock. + * The exception is the eager path in mca_pml_ob1_accelerator_init(), + * which runs single-threaded before MPI_Init completes. */ +static int mca_pml_ob1_accelerator_lazy_init(void) +{ + int result; + int i; + + if (pml_ob1_accelerator_streams_initialized) { + return OPAL_SUCCESS; + } + + result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &dtoh_stream); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator dtoh_stream stream."); + return result; + } + + result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &htod_stream); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator htod_stream stream."); + OBJ_RELEASE(dtoh_stream); + dtoh_stream = NULL; + return result; + } + + for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { + result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_dtoh_array[i], + false); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create dtoh event failed."); + goto cleanup; + } + } + + for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { + result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_htod_array[i], + false); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create htod event failed."); + goto cleanup; + } + } + + pml_ob1_accelerator_streams_initialized = true; + return OPAL_SUCCESS; + +cleanup: + /* Release any events already created, then release the streams. */ + for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { + if (NULL != accelerator_event_htod_array[i]) { + OBJ_RELEASE(accelerator_event_htod_array[i]); + accelerator_event_htod_array[i] = NULL; + } + if (NULL != accelerator_event_dtoh_array[i]) { + OBJ_RELEASE(accelerator_event_dtoh_array[i]); + accelerator_event_dtoh_array[i] = NULL; + } + } + OBJ_RELEASE(htod_stream); + htod_stream = NULL; + OBJ_RELEASE(dtoh_stream); + dtoh_stream = NULL; + return result; +} + int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *frag) { int result; @@ -86,6 +156,11 @@ int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *f /* First make sure there is room to store the event. If not, then * return an error. The error message will tell the user to try and * run again, but with a larger array for storing events. */ + result = mca_pml_ob1_accelerator_ensure_init(); + if (OPAL_SUCCESS != result) { + return result; + } + OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); if (accelerator_event_htod_num_used == mca_pml_ob1_accelerator_events_max) { opal_output_verbose(1, mca_pml_ob1_output, "Out of event handles. Max: %d. Suggested to rerun with new max with --mca pml_ob1_accelerator_events_max %d.", @@ -124,10 +199,13 @@ int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *f opal_accelerator_stream_t *mca_pml_ob1_get_dtoh_stream(void) { + (void) mca_pml_ob1_accelerator_ensure_init(); return dtoh_stream; } + opal_accelerator_stream_t *mca_pml_ob1_get_htod_stream(void) { + (void) mca_pml_ob1_accelerator_ensure_init(); return htod_stream; } @@ -183,8 +261,6 @@ int mca_pml_ob1_progress_one_htod_event(struct mca_btl_base_descriptor_t **frag) int mca_pml_ob1_accelerator_init(void) { int rc = OPAL_SUCCESS; - int result = OPAL_SUCCESS; - int i; if (0 == strcmp(opal_accelerator_base_selected_component.base_version.mca_component_name, "null")) { return 0; @@ -193,21 +269,6 @@ int mca_pml_ob1_accelerator_init(void) OBJ_CONSTRUCT(&pml_ob1_accelerator_htod_lock, opal_mutex_t); OBJ_CONSTRUCT(&pml_ob1_accelerator_dtoh_lock, opal_mutex_t); - /* Create Streams */ - result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &dtoh_stream); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator dtoh_stream stream."); - rc = result; - goto cleanup_and_error; - } - - result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &htod_stream); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator htod_stream stream."); - rc = result; - goto cleanup_and_error; - } - /* Set up an array of pointers to store outstanding async dtoh events. * Used on the sending side for asynchronous copies. */ accelerator_event_dtoh_num_used = 0; @@ -221,18 +282,7 @@ int mca_pml_ob1_accelerator_init(void) goto cleanup_and_error; } - /* Create the events since they can be reused. */ - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_dtoh_array[i], false); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create event failed."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - } - - /* The first available status index is 0. Make an empty frag - array. */ + /* The first available status index is 0. Make an empty frag array. */ accelerator_event_dtoh_frag_array = (struct mca_btl_base_descriptor_t **) malloc( sizeof(struct mca_btl_base_descriptor_t *) * mca_pml_ob1_accelerator_events_max); if (NULL == accelerator_event_dtoh_frag_array) { @@ -254,18 +304,7 @@ int mca_pml_ob1_accelerator_init(void) goto cleanup_and_error; } - /* Create the events since they can be reused. */ - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_htod_array[i], false); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create event failed."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - } - - /* The first available status index is 0. Make an empty frag - array. */ + /* The first available status index is 0. Make an empty frag array. */ accelerator_event_htod_frag_array = (struct mca_btl_base_descriptor_t **) malloc( sizeof(struct mca_btl_base_descriptor_t *) * mca_pml_ob1_accelerator_events_max); if (NULL == accelerator_event_htod_frag_array) { @@ -274,19 +313,28 @@ int mca_pml_ob1_accelerator_init(void) goto cleanup_and_error; } + /* When lazy init is disabled, create streams and events immediately. */ + if (!mca_pml_ob1.accelerator_lazy_init) { + rc = mca_pml_ob1_accelerator_lazy_init(); + } + cleanup_and_error: if (OPAL_SUCCESS != rc) { if (NULL != accelerator_event_dtoh_array) { free(accelerator_event_dtoh_array); + accelerator_event_dtoh_array = NULL; } if (NULL != accelerator_event_dtoh_frag_array) { free(accelerator_event_dtoh_frag_array); + accelerator_event_dtoh_frag_array = NULL; } if (NULL != accelerator_event_htod_array) { free(accelerator_event_htod_array); + accelerator_event_htod_array = NULL; } if (NULL != accelerator_event_htod_frag_array) { free(accelerator_event_htod_frag_array); + accelerator_event_htod_frag_array = NULL; } OBJ_DESTRUCT(&pml_ob1_accelerator_htod_lock); OBJ_DESTRUCT(&pml_ob1_accelerator_dtoh_lock); @@ -327,8 +375,12 @@ void mca_pml_ob1_accelerator_fini(void) free(accelerator_event_htod_frag_array); } - OBJ_RELEASE(htod_stream); - OBJ_RELEASE(dtoh_stream); + if (NULL != htod_stream) { + OBJ_RELEASE(htod_stream); + } + if (NULL != dtoh_stream) { + OBJ_RELEASE(dtoh_stream); + } OBJ_DESTRUCT(&pml_ob1_accelerator_htod_lock); OBJ_DESTRUCT(&pml_ob1_accelerator_dtoh_lock); @@ -346,6 +398,17 @@ int mca_pml_ob1_accelerator_need_buffers(void * rreq, void mca_pml_ob1_accelerator_add_ipc_support(struct mca_btl_base_module_t* btl, int32_t flags, ompi_proc_t* errproc, char* btlinfo); +int mca_pml_ob1_accelerator_ensure_init(void) +{ + int rc = OPAL_SUCCESS; + if (!pml_ob1_accelerator_streams_initialized) { + OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); + rc = mca_pml_ob1_accelerator_lazy_init(); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + } + return rc; +} + /** * Handle the accelerator buffer. */ diff --git a/ompi/mca/pml/ob1/pml_ob1_accelerator.h b/ompi/mca/pml/ob1/pml_ob1_accelerator.h index e24bbe698d3..f5ab3d18647 100644 --- a/ompi/mca/pml/ob1/pml_ob1_accelerator.h +++ b/ompi/mca/pml/ob1/pml_ob1_accelerator.h @@ -36,5 +36,7 @@ OPAL_DECLSPEC opal_accelerator_stream_t *mca_pml_ob1_get_htod_stream(void); OPAL_DECLSPEC int mca_pml_ob1_progress_one_htod_event(struct mca_btl_base_descriptor_t **); OPAL_DECLSPEC int mca_pml_ob1_accelerator_init(void); OPAL_DECLSPEC void mca_pml_ob1_accelerator_fini(void); +/* Trigger lazy stream/event creation on first device buffer use. */ +OPAL_DECLSPEC int mca_pml_ob1_accelerator_ensure_init(void); #endif /* OMPI_PML_OB1_ACCELERATOR_H */ diff --git a/ompi/mca/pml/ob1/pml_ob1_component.c b/ompi/mca/pml/ob1/pml_ob1_component.c index cd5ce5c3d17..fc1ae5668db 100644 --- a/ompi/mca/pml/ob1/pml_ob1_component.c +++ b/ompi/mca/pml/ob1/pml_ob1_component.c @@ -252,6 +252,13 @@ static int mca_pml_ob1_component_register(void) MCA_BASE_VAR_TYPE_INT, NULL, 0, 0, OPAL_INFO_LVL_5, MCA_BASE_VAR_SCOPE_READONLY, &mca_pml_ob1_accelerator_events_max); + mca_pml_ob1.accelerator_lazy_init = true; + (void) mca_base_component_var_register(&mca_pml_ob1_component.pmlm_version, "accelerator_lazy_init", + "Defer accelerator stream and event creation to the first " + "MPI call that uses a device buffer (1 = defer, 0 = eager)", + MCA_BASE_VAR_TYPE_BOOL, NULL, 0, 0, OPAL_INFO_LVL_5, + MCA_BASE_VAR_SCOPE_READONLY, &mca_pml_ob1.accelerator_lazy_init); + return OMPI_SUCCESS; } diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c b/ompi/mca/pml/ob1/pml_ob1_recvreq.c index 8dcee8f0013..9f7839d3bda 100644 --- a/ompi/mca/pml/ob1/pml_ob1_recvreq.c +++ b/ompi/mca/pml/ob1/pml_ob1_recvreq.c @@ -747,6 +747,10 @@ void mca_pml_ob1_recv_request_progress_rget( mca_pml_ob1_recv_request_t* recvreq } rdma_bml = mca_bml_base_btl_array_find(&bml_endpoint->btl_rdma, btl); + if (recvreq->req_recv.req_base.req_convertor.flags & CONVERTOR_ACCELERATOR) { + (void) mca_pml_ob1_accelerator_ensure_init(); + } + if (OPAL_UNLIKELY(NULL == rdma_bml)) { if (recvreq->req_recv.req_base.req_convertor.flags & CONVERTOR_ACCELERATOR) { mca_bml_base_btl_t *bml_btl; From 8db484a39a8196934abf99ffffb36bb78156cb7b Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Fri, 2 Oct 2026 13:57:01 +0000 Subject: [PATCH 2/4] pml/ob1: fix memory ordering and NULL stream guard for lazy accelerator init Two correctness issues in the lazy accelerator initialization introduced in the previous commit: Add release/acquire memory barriers around the streams-initialized flag to ensure correct visibility on weakly-ordered architectures (aarch64, ppc64). A write barrier (opal_atomic_wmb) is inserted in mca_pml_ob1_accelerator_lazy_init() before setting the flag, so all stream and event stores are published before the flag becomes visible. The matching read barrier (opal_atomic_rmb) is inserted in mca_pml_ob1_accelerator_ensure_init() on the lock-free fast path, so a thread observing the flag is guaranteed to also see the fully-created streams and event arrays. Guard the stream pointer returned by mca_pml_ob1_get_dtoh_stream() and mca_pml_ob1_get_htod_stream() with a NULL check before installing the CONVERTOR_ACCELERATOR_ASYNC flag and the stream pointer in the convertor. If lazy init failed (e.g. due to stream or event creation errors) the getters return NULL, and setting ASYNC with a NULL stream would crash. Fall back silently to synchronous copies in that case. Co-authored-by: George Bosilca Signed-off-by: Edgar Gabriel --- ompi/mca/pml/ob1/pml_ob1_accelerator.c | 18 ++++++++++++++---- ompi/mca/pml/ob1/pml_ob1_recvfrag.c | 8 ++++++-- ompi/mca/pml/ob1/pml_ob1_recvreq.c | 10 ++++++++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ompi/mca/pml/ob1/pml_ob1_accelerator.c b/ompi/mca/pml/ob1/pml_ob1_accelerator.c index 682e25c1889..538d3ff9d51 100644 --- a/ompi/mca/pml/ob1/pml_ob1_accelerator.c +++ b/ompi/mca/pml/ob1/pml_ob1_accelerator.c @@ -123,6 +123,12 @@ static int mca_pml_ob1_accelerator_lazy_init(void) } } + /* Publish the streams and events before the flag that advertises them. + * The matching acquire barrier is in mca_pml_ob1_accelerator_ensure_init(), + * so a thread that observes the flag on the lock-free fast path is + * guaranteed to also see the fully-created streams and event arrays + * (required on weakly-ordered architectures such as aarch64 and ppc64). */ + opal_atomic_wmb(); pml_ob1_accelerator_streams_initialized = true; return OPAL_SUCCESS; @@ -401,11 +407,15 @@ void mca_pml_ob1_accelerator_add_ipc_support(struct mca_btl_base_module_t* btl, int mca_pml_ob1_accelerator_ensure_init(void) { int rc = OPAL_SUCCESS; - if (!pml_ob1_accelerator_streams_initialized) { - OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); - rc = mca_pml_ob1_accelerator_lazy_init(); - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + if (OPAL_LIKELY(pml_ob1_accelerator_streams_initialized)) { + /* Acquire: pair with the release (wmb) in lazy_init so the stream and + * event pointers read after this point are visible and non-stale. */ + opal_atomic_rmb(); + return OPAL_SUCCESS; } + OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); + rc = mca_pml_ob1_accelerator_lazy_init(); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); return rc; } diff --git a/ompi/mca/pml/ob1/pml_ob1_recvfrag.c b/ompi/mca/pml/ob1/pml_ob1_recvfrag.c index 09a673fce0c..06374192ad6 100644 --- a/ompi/mca/pml/ob1/pml_ob1_recvfrag.c +++ b/ompi/mca/pml/ob1/pml_ob1_recvfrag.c @@ -923,8 +923,12 @@ void mca_pml_ob1_recv_frag_callback_ack (mca_btl_base_module_t *btl, * so adjust the convertor accordingly. All the subsequent fragments will * use the asynchronous copy. */ opal_accelerator_stream_t *stream = mca_pml_ob1_get_dtoh_stream(); - sendreq->req_send.req_base.req_convertor.flags |= CONVERTOR_ACCELERATOR_ASYNC; - sendreq->req_send.req_base.req_convertor.stream = stream; + /* Fall back to a synchronous copy if lazy init failed and no stream + * is available, instead of installing a NULL stream. */ + if (NULL != stream) { + sendreq->req_send.req_base.req_convertor.flags |= CONVERTOR_ACCELERATOR_ASYNC; + sendreq->req_send.req_base.req_convertor.stream = stream; + } } /* ensure all prior stores (copy_in_out, rdma_frag, throttle_sends, diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c b/ompi/mca/pml/ob1/pml_ob1_recvreq.c index 9f7839d3bda..420fad660eb 100644 --- a/ompi/mca/pml/ob1/pml_ob1_recvreq.c +++ b/ompi/mca/pml/ob1/pml_ob1_recvreq.c @@ -928,8 +928,14 @@ void mca_pml_ob1_recv_request_progress_rndv( mca_pml_ob1_recv_request_t* recvreq if ((recvreq->req_recv.req_base.req_convertor.flags & CONVERTOR_ACCELERATOR) && (btl->btl_flags & MCA_BTL_FLAGS_ACCELERATOR_COPY_ASYNC_RECV)) { opal_accelerator_stream_t *stream = mca_pml_ob1_get_htod_stream(); - recvreq->req_recv.req_base.req_convertor.flags |= CONVERTOR_ACCELERATOR_ASYNC; - recvreq->req_recv.req_base.req_convertor.stream = stream; + /* Only switch to asynchronous copies if the stream actually exists. + * Lazy init can fail (e.g. stream/event creation errors), in which + * case get_htod_stream() returns NULL; fall back to a synchronous + * copy rather than installing a NULL stream with the ASYNC flag. */ + if (NULL != stream) { + recvreq->req_recv.req_base.req_convertor.flags |= CONVERTOR_ACCELERATOR_ASYNC; + recvreq->req_recv.req_base.req_convertor.stream = stream; + } } } From 3b639fea47294705fbe5a165be2e8647060f33ec Mon Sep 17 00:00:00 2001 From: George Bosilca Date: Mon, 5 Oct 2026 22:13:09 -0700 Subject: [PATCH 3/4] pml/ob1: grow the accelerator event pool on demand The ob1 accelerator path pre-allocated two arrays of mca_pml_ob1_accelerator_events_max (400) events up front, one per direction, whether or not a device buffer was ever used. Replace that with a single event pool that starts empty and grows on demand, in batches of mca_pml_ob1_accelerator_events_batch events (new MCA parameter, default 32), up to the mca_pml_ob1_accelerator_events_max cap. Two counters describe the pool: the current number of allocated events and the cap it may grow to. When a batch cannot be fully created the pool keeps whatever events were made and shrinks to match; the "Out of event handles" error is raised only once the pool sits at the cap and not one event could be created. The growth batch is validated at registration to lie in [1, mca_pml_ob1_accelerator_events_max]. The completion events are not tied to a direction, so a single pool now backs all asynchronous copies and the event machinery is named accordingly (mca_pml_ob1_record_event, mca_pml_ob1_progress_one_event, and so on). The caller of record_event passes the stream the copy was queued on, so an event can be used with any stream. The separate device-to-host event pool, which was never actually fed any events, is removed; only the direction-specific streams (htod_stream, dtoh_stream) remain. Also remove the pml_ob1_accelerator_lazy_init MCA parameter. Deferring stream and event creation until the first device buffer is used is now unconditional; there is no reason to offer an eager mode that binds a process to a device during MPI_Init. Fold the two-function lazy init into a single out-of-line slow path, mca_pml_ob1_accelerator_create_streams(), reached through an inline mca_pml_ob1_accelerator_ensure_init(). The common, already-initialized case is now a lock-free flag read plus an acquire barrier with no function call on the critical path; only the first device-buffer use per process falls through to the slow path. Signed-off-by: George Bosilca --- ompi/mca/pml/ob1/pml_ob1.h | 3 +- ompi/mca/pml/ob1/pml_ob1_accelerator.c | 414 +++++++++++-------------- ompi/mca/pml/ob1/pml_ob1_accelerator.h | 32 +- ompi/mca/pml/ob1/pml_ob1_component.c | 36 ++- ompi/mca/pml/ob1/pml_ob1_progress.c | 2 +- ompi/mca/pml/ob1/pml_ob1_recvreq.c | 8 +- 6 files changed, 252 insertions(+), 243 deletions(-) diff --git a/ompi/mca/pml/ob1/pml_ob1.h b/ompi/mca/pml/ob1/pml_ob1.h index 62712b02104..4dda7749eae 100644 --- a/ompi/mca/pml/ob1/pml_ob1.h +++ b/ompi/mca/pml/ob1/pml_ob1.h @@ -92,8 +92,6 @@ struct mca_pml_ob1_t { unsigned int unexpected_limit; /* Accelerator support initialized */ bool accelerator_enabled; - /* Defer stream/event creation to first device buffer use */ - bool accelerator_lazy_init; }; typedef struct mca_pml_ob1_t mca_pml_ob1_t; @@ -101,6 +99,7 @@ extern mca_pml_ob1_t mca_pml_ob1; extern int mca_pml_ob1_output; extern bool mca_pml_ob1_matching_protection; extern int mca_pml_ob1_accelerator_events_max; +extern int mca_pml_ob1_accelerator_events_batch; /* * PML interface functions. diff --git a/ompi/mca/pml/ob1/pml_ob1_accelerator.c b/ompi/mca/pml/ob1/pml_ob1_accelerator.c index 538d3ff9d51..ceafe520a90 100644 --- a/ompi/mca/pml/ob1/pml_ob1_accelerator.c +++ b/ompi/mca/pml/ob1/pml_ob1_accelerator.c @@ -50,108 +50,124 @@ static opal_accelerator_stream_t *dtoh_stream = NULL; static opal_accelerator_stream_t *htod_stream = NULL; -static opal_mutex_t pml_ob1_accelerator_htod_lock; -static opal_mutex_t pml_ob1_accelerator_dtoh_lock; +static opal_mutex_t pml_ob1_accelerator_event_lock; -/* Set to true once streams and events have been created. */ -static bool pml_ob1_accelerator_streams_initialized = false; +/* Set to true once the accelerator streams have been created. Read on the hot + * path by the inline mca_pml_ob1_accelerator_ensure_init() in the header. */ +bool mca_pml_ob1_accelerator_streams_initialized = false; -/* Array of accelerator events to be queried for sending side and - * receiving side. */ -static opal_accelerator_event_t **accelerator_event_dtoh_array = NULL; -static opal_accelerator_event_t **accelerator_event_htod_array = NULL; +/* Pool of accelerator completion events, shared across directions. An event is + * recorded against whichever stream an asynchronous copy was queued on (see + * mca_pml_ob1_record_event()) and later reaped in + * mca_pml_ob1_progress_one_event(). */ +static opal_accelerator_event_t **accelerator_event_array = NULL; /* Array of fragments currently being moved by accelerator async non-blocking * operations */ -static struct mca_btl_base_descriptor_t **accelerator_event_dtoh_frag_array = NULL; -static struct mca_btl_base_descriptor_t **accelerator_event_htod_frag_array = NULL; +static struct mca_btl_base_descriptor_t **accelerator_event_frag_array = NULL; -/* First free/available location in accelerator_event_status_array */ -static int accelerator_event_dtoh_first_avail, accelerator_event_htod_first_avail; +/* First free/available location in accelerator_event_array */ +static int accelerator_event_first_avail; -/* First currently-being used location in the accelerator_event_status_array */ -static int accelerator_event_dtoh_first_used, accelerator_event_htod_first_used; +/* First currently-being used location in accelerator_event_array */ +static int accelerator_event_first_used; /* Number of status items currently in use */ -static volatile int accelerator_event_dtoh_num_used, accelerator_event_htod_num_used; - -/* Size of array holding events */ -static int accelerator_event_htod_most = 0; - -/* Create streams and events on first use of a device buffer. - * At runtime, callers must hold pml_ob1_accelerator_htod_lock. - * The exception is the eager path in mca_pml_ob1_accelerator_init(), - * which runs single-threaded before MPI_Init completes. */ -static int mca_pml_ob1_accelerator_lazy_init(void) +static volatile int accelerator_event_num_used; + +/* Current number of events allocated in the pool. The pool starts empty and + * grows on demand, in batches of mca_pml_ob1_accelerator_events_batch, up to the + * mca_pml_ob1_accelerator_events_max cap. */ +static int accelerator_event_size = 0; + +/* Grow the event pool by one batch of mca_pml_ob1_accelerator_events_batch + * events, never exceeding the mca_pml_ob1_accelerator_events_max cap. Must be + * called with the ring full (num_used == size) and with + * pml_ob1_accelerator_event_lock held. On success the ring has at least one free + * slot; returns OPAL_ERR_OUT_OF_RESOURCE when already at the cap. */ +static int mca_pml_ob1_accelerator_grow_events(void) { - int result; - int i; + int old_size = accelerator_event_size; + int new_size, i, result; + opal_accelerator_event_t **new_events; + struct mca_btl_base_descriptor_t **new_frags; - if (pml_ob1_accelerator_streams_initialized) { - return OPAL_SUCCESS; + if (old_size >= mca_pml_ob1_accelerator_events_max) { + return OPAL_ERR_OUT_OF_RESOURCE; } - result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &dtoh_stream); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator dtoh_stream stream."); - return result; + /* mca_pml_ob1_accelerator_events_batch is validated at registration to be + * in [1, mca_pml_ob1_accelerator_events_max]. */ + new_size = old_size + mca_pml_ob1_accelerator_events_batch; + if (new_size > mca_pml_ob1_accelerator_events_max) { + new_size = mca_pml_ob1_accelerator_events_max; } - result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &htod_stream); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator htod_stream stream."); - OBJ_RELEASE(dtoh_stream); - dtoh_stream = NULL; - return result; + new_events = (opal_accelerator_event_t **) calloc(new_size, sizeof(opal_accelerator_event_t *)); + new_frags = (struct mca_btl_base_descriptor_t **) malloc(new_size * sizeof(struct mca_btl_base_descriptor_t *)); + if (NULL == new_events || NULL == new_frags) { + opal_output_verbose(1, mca_pml_ob1_output, "No memory to grow accelerator event pool."); + free(new_events); + free(new_frags); + return OPAL_ERR_OUT_OF_RESOURCE; } - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_dtoh_array[i], - false); + /* Re-linearize the ring: the in-flight entries, in order, start at + * first_used and wrap exactly once. Copy them to the front of the new + * arrays as two contiguous runs -- [first_used, old_size) then + * [0, first_used) -- so the occupied region becomes [0, old_size) again. + * (With old_size == 0 the arrays are still NULL, so there is nothing to + * copy.) */ + if (old_size > 0) { + int base = accelerator_event_first_used; + int wrap = old_size - base; /* entries from first_used to end of array */ + memcpy(new_events, &accelerator_event_array[base], wrap * sizeof(*new_events)); + memcpy(&new_events[wrap], accelerator_event_array, base * sizeof(*new_events)); + memcpy(new_frags, &accelerator_event_frag_array[base], wrap * sizeof(*new_frags)); + memcpy(&new_frags[wrap], accelerator_event_frag_array, base * sizeof(*new_frags)); + } + + /* Create the fresh events that make up the new batch. If the accelerator + * runs out partway, keep whatever we did create and shrink the new size to + * match -- a smaller grow is still progress, and the surplus slots are + * simply left unused. Only fail if we could not create a single new event, + * since the ring would then still be full. */ + for (i = old_size; i < new_size; i++) { + result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &new_events[i], false); if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create dtoh event failed."); - goto cleanup; + opal_output_verbose(1, mca_pml_ob1_output, + "Accelerator create event failed after %d of %d new events.", + i - old_size, new_size - old_size); + break; } } - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - result = opal_accelerator.create_event(MCA_ACCELERATOR_NO_DEVICE_ID, &accelerator_event_htod_array[i], - false); - if (OPAL_SUCCESS != result) { - opal_output_verbose(1, mca_pml_ob1_output, "Accelerator create htod event failed."); - goto cleanup; - } + if (i == old_size) { + /* Nothing gained; the old ring arrays are still intact, so drop the + * freshly allocated (empty) ones and report the failure. */ + free(new_events); + free(new_frags); + return result; } + new_size = i; /* however many events we actually created */ - /* Publish the streams and events before the flag that advertises them. - * The matching acquire barrier is in mca_pml_ob1_accelerator_ensure_init(), - * so a thread that observes the flag on the lock-free fast path is - * guaranteed to also see the fully-created streams and event arrays - * (required on weakly-ordered architectures such as aarch64 and ppc64). */ - opal_atomic_wmb(); - pml_ob1_accelerator_streams_initialized = true; - return OPAL_SUCCESS; + free(accelerator_event_array); + free(accelerator_event_frag_array); + accelerator_event_array = new_events; + accelerator_event_frag_array = new_frags; + accelerator_event_first_used = 0; + accelerator_event_first_avail = old_size; /* first fresh slot */ + accelerator_event_size = new_size; + /* num_used is unchanged by a grow. */ -cleanup: - /* Release any events already created, then release the streams. */ - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - if (NULL != accelerator_event_htod_array[i]) { - OBJ_RELEASE(accelerator_event_htod_array[i]); - accelerator_event_htod_array[i] = NULL; - } - if (NULL != accelerator_event_dtoh_array[i]) { - OBJ_RELEASE(accelerator_event_dtoh_array[i]); - accelerator_event_dtoh_array[i] = NULL; - } - } - OBJ_RELEASE(htod_stream); - htod_stream = NULL; - OBJ_RELEASE(dtoh_stream); - dtoh_stream = NULL; - return result; + opal_output_verbose(10, mca_pml_ob1_output, + "Grew accelerator event pool to %d (cap %d)", + accelerator_event_size, mca_pml_ob1_accelerator_events_max); + return OPAL_SUCCESS; } -int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *frag) +int mca_pml_ob1_record_event(char *msg, struct mca_btl_base_descriptor_t *frag, + opal_accelerator_stream_t *stream) { int result; @@ -159,47 +175,41 @@ int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *f return 0; } - /* First make sure there is room to store the event. If not, then - * return an error. The error message will tell the user to try and - * run again, but with a larger array for storing events. */ - result = mca_pml_ob1_accelerator_ensure_init(); - if (OPAL_SUCCESS != result) { - return result; + /* The event pool is direction-agnostic; the caller selects which stream the + * asynchronous operation was queued on. */ + if (NULL == stream) { + return OPAL_ERR_BAD_PARAM; } - OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); - if (accelerator_event_htod_num_used == mca_pml_ob1_accelerator_events_max) { - opal_output_verbose(1, mca_pml_ob1_output, "Out of event handles. Max: %d. Suggested to rerun with new max with --mca pml_ob1_accelerator_events_max %d.", - mca_pml_ob1_accelerator_events_max, mca_pml_ob1_accelerator_events_max + 100); - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); - return OPAL_ERR_OUT_OF_RESOURCE; - } - - if (accelerator_event_htod_num_used > accelerator_event_htod_most) { - accelerator_event_htod_most = accelerator_event_htod_num_used; - /* Just print multiples of 10 */ - if (0 == (accelerator_event_htod_most % 10)) { - opal_output_verbose(20, mca_pml_ob1_output, "Maximum HtoD events used is now %d", - accelerator_event_htod_most); + OPAL_THREAD_LOCK(&pml_ob1_accelerator_event_lock); + /* No free slot: grow the pool by a batch. Only if that fails (we are + * already at the cap) do we return an error telling the user to raise it. */ + if (accelerator_event_num_used == accelerator_event_size) { + result = mca_pml_ob1_accelerator_grow_events(); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Out of event handles. Max: %d. Suggested to rerun with new max with --mca pml_ob1_accelerator_events_max %d.", + mca_pml_ob1_accelerator_events_max, mca_pml_ob1_accelerator_events_max + 100); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); + return result; } } - result = opal_accelerator.record_event(MCA_ACCELERATOR_NO_DEVICE_ID, accelerator_event_htod_array[accelerator_event_htod_first_avail], htod_stream); + result = opal_accelerator.record_event(MCA_ACCELERATOR_NO_DEVICE_ID, accelerator_event_array[accelerator_event_first_avail], stream); if (OPAL_UNLIKELY(OPAL_SUCCESS != result)) { opal_output_verbose(1, mca_pml_ob1_output, "Event Record failed."); - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return OPAL_ERROR; } - accelerator_event_htod_frag_array[accelerator_event_htod_first_avail] = frag; + accelerator_event_frag_array[accelerator_event_first_avail] = frag; /* Bump up the first available slot and number used by 1 */ - accelerator_event_htod_first_avail++; - if (accelerator_event_htod_first_avail >= mca_pml_ob1_accelerator_events_max) { - accelerator_event_htod_first_avail = 0; + accelerator_event_first_avail++; + if (accelerator_event_first_avail >= accelerator_event_size) { + accelerator_event_first_avail = 0; } - accelerator_event_htod_num_used++; + accelerator_event_num_used++; - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return OPAL_SUCCESS; } @@ -216,9 +226,9 @@ opal_accelerator_stream_t *mca_pml_ob1_get_htod_stream(void) } /** - * Progress any htod event completions. + * Progress any event completions. */ -int mca_pml_ob1_progress_one_htod_event(struct mca_btl_base_descriptor_t **frag) +int mca_pml_ob1_progress_one_event(struct mca_btl_base_descriptor_t **frag) { int result; @@ -226,127 +236,66 @@ int mca_pml_ob1_progress_one_htod_event(struct mca_btl_base_descriptor_t **frag) return 0; } - OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); - if (accelerator_event_htod_num_used > 0) { + OPAL_THREAD_LOCK(&pml_ob1_accelerator_event_lock); + if (accelerator_event_num_used > 0) { opal_output_verbose(30, mca_pml_ob1_output, - "mca_pml_ob1_progress_one_htod_event, outstanding_events=%d", - accelerator_event_htod_num_used); + "mca_pml_ob1_progress_one_event, outstanding_events=%d", + accelerator_event_num_used); - result = opal_accelerator.query_event(MCA_ACCELERATOR_NO_DEVICE_ID, accelerator_event_htod_array[accelerator_event_htod_first_used]); + result = opal_accelerator.query_event(MCA_ACCELERATOR_NO_DEVICE_ID, accelerator_event_array[accelerator_event_first_used]); /* We found an event that is not ready, so return. */ if (OPAL_ERR_RESOURCE_BUSY == result) { opal_output_verbose(30, mca_pml_ob1_output, "Accelerator event query returned OPAL_ERR_RESOURCE_BUSY"); *frag = NULL; - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return 0; } else if (OPAL_SUCCESS != result) { opal_output_verbose(1, mca_pml_ob1_output, "Accelerator event query failed: %d,", result); *frag = NULL; - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return OPAL_ERROR; } - *frag = accelerator_event_htod_frag_array[accelerator_event_htod_first_used]; + *frag = accelerator_event_frag_array[accelerator_event_first_used]; /* Bump counters, loop around the circular buffer if necessary */ - --accelerator_event_htod_num_used; - ++accelerator_event_htod_first_used; - if (accelerator_event_htod_first_used >= mca_pml_ob1_accelerator_events_max) { - accelerator_event_htod_first_used = 0; + --accelerator_event_num_used; + ++accelerator_event_first_used; + if (accelerator_event_first_used >= accelerator_event_size) { + accelerator_event_first_used = 0; } /* A return value of 1 indicates an event completed and a frag was returned */ - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return 1; } - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return 0; } int mca_pml_ob1_accelerator_init(void) { - int rc = OPAL_SUCCESS; - if (0 == strcmp(opal_accelerator_base_selected_component.base_version.mca_component_name, "null")) { return 0; } - OBJ_CONSTRUCT(&pml_ob1_accelerator_htod_lock, opal_mutex_t); - OBJ_CONSTRUCT(&pml_ob1_accelerator_dtoh_lock, opal_mutex_t); - - /* Set up an array of pointers to store outstanding async dtoh events. - * Used on the sending side for asynchronous copies. */ - accelerator_event_dtoh_num_used = 0; - accelerator_event_dtoh_first_avail = 0; - accelerator_event_dtoh_first_used = 0; - - accelerator_event_dtoh_array = calloc(mca_pml_ob1_accelerator_events_max, sizeof(opal_accelerator_event_t *)); - if (NULL == accelerator_event_dtoh_array) { - opal_output_verbose(1, mca_pml_ob1_output, "No memory."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - - /* The first available status index is 0. Make an empty frag array. */ - accelerator_event_dtoh_frag_array = (struct mca_btl_base_descriptor_t **) malloc( - sizeof(struct mca_btl_base_descriptor_t *) * mca_pml_ob1_accelerator_events_max); - if (NULL == accelerator_event_dtoh_frag_array) { - opal_output_verbose(1, mca_pml_ob1_output, "No memory."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - - /* Set up an array of pointers to store outstanding async htod events. - * Used on the receiving side for asynchronous copies. */ - accelerator_event_htod_num_used = 0; - accelerator_event_htod_first_avail = 0; - accelerator_event_htod_first_used = 0; - - accelerator_event_htod_array = calloc(mca_pml_ob1_accelerator_events_max, sizeof(opal_accelerator_event_t *)); - if (NULL == accelerator_event_htod_array) { - opal_output_verbose(1, mca_pml_ob1_output, "No memory."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - - /* The first available status index is 0. Make an empty frag array. */ - accelerator_event_htod_frag_array = (struct mca_btl_base_descriptor_t **) malloc( - sizeof(struct mca_btl_base_descriptor_t *) * mca_pml_ob1_accelerator_events_max); - if (NULL == accelerator_event_htod_frag_array) { - opal_output_verbose(1, mca_pml_ob1_output, "No memory."); - rc = OPAL_ERROR; - goto cleanup_and_error; - } - - /* When lazy init is disabled, create streams and events immediately. */ - if (!mca_pml_ob1.accelerator_lazy_init) { - rc = mca_pml_ob1_accelerator_lazy_init(); - } + OBJ_CONSTRUCT(&pml_ob1_accelerator_event_lock, opal_mutex_t); -cleanup_and_error: - if (OPAL_SUCCESS != rc) { - if (NULL != accelerator_event_dtoh_array) { - free(accelerator_event_dtoh_array); - accelerator_event_dtoh_array = NULL; - } - if (NULL != accelerator_event_dtoh_frag_array) { - free(accelerator_event_dtoh_frag_array); - accelerator_event_dtoh_frag_array = NULL; - } - if (NULL != accelerator_event_htod_array) { - free(accelerator_event_htod_array); - accelerator_event_htod_array = NULL; - } - if (NULL != accelerator_event_htod_frag_array) { - free(accelerator_event_htod_frag_array); - accelerator_event_htod_frag_array = NULL; - } - OBJ_DESTRUCT(&pml_ob1_accelerator_htod_lock); - OBJ_DESTRUCT(&pml_ob1_accelerator_dtoh_lock); - } + /* The event pool starts empty. Streams are created on first use of a + * device buffer by mca_pml_ob1_accelerator_ensure_init(), and the event + * array grows on demand (in batches of + * mca_pml_ob1_accelerator_events_batch, up to + * mca_pml_ob1_accelerator_events_max) in + * mca_pml_ob1_accelerator_grow_events(). */ + accelerator_event_size = 0; + accelerator_event_num_used = 0; + accelerator_event_first_avail = 0; + accelerator_event_first_used = 0; + accelerator_event_array = NULL; + accelerator_event_frag_array = NULL; - return rc; + return OPAL_SUCCESS; } void mca_pml_ob1_accelerator_fini(void) @@ -357,28 +306,16 @@ void mca_pml_ob1_accelerator_fini(void) return; } - if (NULL != accelerator_event_htod_array) { - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - if (NULL != accelerator_event_htod_array[i]) { - OBJ_RELEASE(accelerator_event_htod_array[i]); - } - } - free(accelerator_event_htod_array); - } - - if (NULL != accelerator_event_dtoh_array) { - for (i = 0; i < mca_pml_ob1_accelerator_events_max; i++) { - if (NULL != accelerator_event_dtoh_array[i]) { - OBJ_RELEASE(accelerator_event_dtoh_array[i]); + if (NULL != accelerator_event_array) { + for (i = 0; i < accelerator_event_size; i++) { + if (NULL != accelerator_event_array[i]) { + OBJ_RELEASE(accelerator_event_array[i]); } } - free(accelerator_event_dtoh_array); - } - if (NULL != accelerator_event_dtoh_frag_array) { - free(accelerator_event_dtoh_frag_array); + free(accelerator_event_array); } - if (NULL != accelerator_event_htod_frag_array) { - free(accelerator_event_htod_frag_array); + if (NULL != accelerator_event_frag_array) { + free(accelerator_event_frag_array); } if (NULL != htod_stream) { @@ -388,8 +325,7 @@ void mca_pml_ob1_accelerator_fini(void) OBJ_RELEASE(dtoh_stream); } - OBJ_DESTRUCT(&pml_ob1_accelerator_htod_lock); - OBJ_DESTRUCT(&pml_ob1_accelerator_dtoh_lock); + OBJ_DESTRUCT(&pml_ob1_accelerator_event_lock); } size_t mca_pml_ob1_rdma_cuda_btls( @@ -404,19 +340,45 @@ int mca_pml_ob1_accelerator_need_buffers(void * rreq, void mca_pml_ob1_accelerator_add_ipc_support(struct mca_btl_base_module_t* btl, int32_t flags, ompi_proc_t* errproc, char* btlinfo); -int mca_pml_ob1_accelerator_ensure_init(void) +/* Slow path for the inline mca_pml_ob1_accelerator_ensure_init(): create the + * accelerator streams on first use of a device buffer. The event pool is grown + * separately and on demand by mca_pml_ob1_accelerator_grow_events(). */ +int mca_pml_ob1_accelerator_create_streams(void) { - int rc = OPAL_SUCCESS; - if (OPAL_LIKELY(pml_ob1_accelerator_streams_initialized)) { - /* Acquire: pair with the release (wmb) in lazy_init so the stream and - * event pointers read after this point are visible and non-stale. */ - opal_atomic_rmb(); + int result; + + /* Create the streams under the lock, re-checking the flag in case another + * thread won the race while we waited. */ + OPAL_THREAD_LOCK(&pml_ob1_accelerator_event_lock); + if (mca_pml_ob1_accelerator_streams_initialized) { + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); return OPAL_SUCCESS; } - OPAL_THREAD_LOCK(&pml_ob1_accelerator_htod_lock); - rc = mca_pml_ob1_accelerator_lazy_init(); - OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_htod_lock); - return rc; + + result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &dtoh_stream); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator dtoh_stream stream."); + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); + return result; + } + + result = opal_accelerator.create_stream(MCA_ACCELERATOR_NO_DEVICE_ID, &htod_stream); + if (OPAL_SUCCESS != result) { + opal_output_verbose(1, mca_pml_ob1_output, "Failed to create accelerator htod_stream stream."); + OBJ_RELEASE(dtoh_stream); + dtoh_stream = NULL; + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); + return result; + } + + /* Publish the streams before the flag that advertises them, so a thread that + * observes the flag on the lock-free fast path (the inline + * mca_pml_ob1_accelerator_ensure_init()) also sees the fully-created streams + * (required on weakly-ordered architectures such as aarch64 and ppc64). */ + opal_atomic_wmb(); + mca_pml_ob1_accelerator_streams_initialized = true; + OPAL_THREAD_UNLOCK(&pml_ob1_accelerator_event_lock); + return OPAL_SUCCESS; } /** diff --git a/ompi/mca/pml/ob1/pml_ob1_accelerator.h b/ompi/mca/pml/ob1/pml_ob1_accelerator.h index f5ab3d18647..439ae8afa77 100644 --- a/ompi/mca/pml/ob1/pml_ob1_accelerator.h +++ b/ompi/mca/pml/ob1/pml_ob1_accelerator.h @@ -27,16 +27,40 @@ #ifndef OMPI_PML_OB1_ACCELERATOR_H #define OMPI_PML_OB1_ACCELERATOR_H +#include "opal/constants.h" +#include "opal/sys/atomic.h" #include "opal/mca/accelerator/accelerator.h" #include "opal/mca/btl/btl.h" -OPAL_DECLSPEC int mca_pml_ob1_record_htod_event(char *msg, struct mca_btl_base_descriptor_t *frag); +OPAL_DECLSPEC int mca_pml_ob1_record_event(char *msg, struct mca_btl_base_descriptor_t *frag, + opal_accelerator_stream_t *stream); OPAL_DECLSPEC opal_accelerator_stream_t *mca_pml_ob1_get_dtoh_stream(void); OPAL_DECLSPEC opal_accelerator_stream_t *mca_pml_ob1_get_htod_stream(void); -OPAL_DECLSPEC int mca_pml_ob1_progress_one_htod_event(struct mca_btl_base_descriptor_t **); +OPAL_DECLSPEC int mca_pml_ob1_progress_one_event(struct mca_btl_base_descriptor_t **); OPAL_DECLSPEC int mca_pml_ob1_accelerator_init(void); OPAL_DECLSPEC void mca_pml_ob1_accelerator_fini(void); -/* Trigger lazy stream/event creation on first device buffer use. */ -OPAL_DECLSPEC int mca_pml_ob1_accelerator_ensure_init(void); + +/* Set to true once the accelerator streams have been created (defined in + * pml_ob1_accelerator.c). Read on the hot path by the inline below. */ +OPAL_DECLSPEC extern bool mca_pml_ob1_accelerator_streams_initialized; + +/* Slow path for mca_pml_ob1_accelerator_ensure_init(): create the streams on + * first device buffer use. Call it through the inline wrapper, never directly. */ +OPAL_DECLSPEC int mca_pml_ob1_accelerator_create_streams(void); + +/* Ensure the accelerator streams exist before first use of a device buffer. The + * common, already-initialized case is a lock-free inline flag check; only the + * first use per process falls through to the out-of-line slow path. */ +static inline int mca_pml_ob1_accelerator_ensure_init(void) +{ + if (OPAL_LIKELY(mca_pml_ob1_accelerator_streams_initialized)) { + /* Acquire: pair with the release (wmb) in + * mca_pml_ob1_accelerator_create_streams() so the stream pointers read + * after this point are visible and non-stale. */ + opal_atomic_rmb(); + return OPAL_SUCCESS; + } + return mca_pml_ob1_accelerator_create_streams(); +} #endif /* OMPI_PML_OB1_ACCELERATOR_H */ diff --git a/ompi/mca/pml/ob1/pml_ob1_component.c b/ompi/mca/pml/ob1/pml_ob1_component.c index fc1ae5668db..5da89c92d7f 100644 --- a/ompi/mca/pml/ob1/pml_ob1_component.c +++ b/ompi/mca/pml/ob1/pml_ob1_component.c @@ -65,6 +65,7 @@ int mca_pml_ob1_output = 0; static int mca_pml_ob1_verbose = 0; bool mca_pml_ob1_matching_protection = false; int mca_pml_ob1_accelerator_events_max = 400; +int mca_pml_ob1_accelerator_events_batch = 32; mca_pml_base_component_2_1_0_t mca_pml_ob1_component = { /* First, the mca_base_component_t struct containing meta @@ -248,16 +249,37 @@ static int mca_pml_ob1_component_register(void) mca_pml_ob1_accelerator_events_max = 400; (void) mca_base_component_var_register(&mca_pml_ob1_component.pmlm_version, "accelerator_events_max", - "Number of events created by the ob1 component internally", + "Upper bound on the number of outstanding asynchronous " + "accelerator copies tracked by the ob1 component. The " + "event pool starts empty and grows on demand (see " + "accelerator_events_batch) up to this cap; increase it if " + "the \"Out of event handles\" message appears.", MCA_BASE_VAR_TYPE_INT, NULL, 0, 0, OPAL_INFO_LVL_5, MCA_BASE_VAR_SCOPE_READONLY, &mca_pml_ob1_accelerator_events_max); - mca_pml_ob1.accelerator_lazy_init = true; - (void) mca_base_component_var_register(&mca_pml_ob1_component.pmlm_version, "accelerator_lazy_init", - "Defer accelerator stream and event creation to the first " - "MPI call that uses a device buffer (1 = defer, 0 = eager)", - MCA_BASE_VAR_TYPE_BOOL, NULL, 0, 0, OPAL_INFO_LVL_5, - MCA_BASE_VAR_SCOPE_READONLY, &mca_pml_ob1.accelerator_lazy_init); + mca_pml_ob1_accelerator_events_batch = 32; + (void) mca_base_component_var_register(&mca_pml_ob1_component.pmlm_version, "accelerator_events_batch", + "Number of accelerator events added each time the event " + "pool is grown on demand, up to the " + "accelerator_events_max cap. Must be at least 1.", + MCA_BASE_VAR_TYPE_INT, NULL, 0, 0, OPAL_INFO_LVL_5, + MCA_BASE_VAR_SCOPE_READONLY, &mca_pml_ob1_accelerator_events_batch); + + /* The growth batch must be positive and no larger than the cap it grows + * up to; clamp a misconfigured value rather than risk never freeing a + * slot (batch < 1) or overshooting the cap. */ + if (mca_pml_ob1_accelerator_events_batch < 1) { + opal_output_verbose(1, mca_pml_ob1_output, + "pml_ob1_accelerator_events_batch must be positive; clamping %d to 1.", + mca_pml_ob1_accelerator_events_batch); + mca_pml_ob1_accelerator_events_batch = 1; + } + if (mca_pml_ob1_accelerator_events_batch > mca_pml_ob1_accelerator_events_max) { + opal_output_verbose(1, mca_pml_ob1_output, + "pml_ob1_accelerator_events_batch (%d) exceeds pml_ob1_accelerator_events_max (%d); clamping to the cap.", + mca_pml_ob1_accelerator_events_batch, mca_pml_ob1_accelerator_events_max); + mca_pml_ob1_accelerator_events_batch = mca_pml_ob1_accelerator_events_max; + } return OMPI_SUCCESS; } diff --git a/ompi/mca/pml/ob1/pml_ob1_progress.c b/ompi/mca/pml/ob1/pml_ob1_progress.c index e6c32876d68..4a19825dea0 100644 --- a/ompi/mca/pml/ob1/pml_ob1_progress.c +++ b/ompi/mca/pml/ob1/pml_ob1_progress.c @@ -42,7 +42,7 @@ static inline int mca_pml_ob1_process_pending_accelerator_async_copies(void) int progress, count = 0; do { - progress = mca_pml_ob1_progress_one_htod_event(&frag); + progress = mca_pml_ob1_progress_one_event(&frag); if (1 == progress) { /* Call the finish function to make progress. */ mca_pml_ob1_recv_request_frag_copy_finished(NULL, NULL, frag, 0); diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c b/ompi/mca/pml/ob1/pml_ob1_recvreq.c index 420fad660eb..4c7b277b062 100644 --- a/ompi/mca/pml/ob1/pml_ob1_recvreq.c +++ b/ompi/mca/pml/ob1/pml_ob1_recvreq.c @@ -657,9 +657,11 @@ void mca_pml_ob1_recv_request_frag_copy_start( mca_pml_ob1_recv_request_t* recvr /* Store the amount of bytes in unused cbdata pointer */ des->des_cbdata = (void *) (intptr_t) bytes_delivered; /* Then record an event that will get triggered by a PML progress call which - * checks the stream events. If we get an error, abort. Should get message - * from CUDA code about what went wrong. */ - result = mca_pml_ob1_record_htod_event("pml", des); + * checks the stream events. The data was unpacked onto the device via the + * host-to-device stream, so record the completion event against it. If we + * get an error, abort. Should get message from CUDA code about what went + * wrong. */ + result = mca_pml_ob1_record_event("pml", des, mca_pml_ob1_get_htod_stream()); if (OMPI_SUCCESS != result) { opal_output(0, "%s:%d FATAL", __FILE__, __LINE__); ompi_rte_abort(-1, NULL); From 1a298292a19293b55cb15601c2c0c25d56d53851 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Tue, 22 Sep 2026 18:19:00 -0400 Subject: [PATCH 4/4] Add LL/SC support for RISC-V platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduces explicit acquire-ll, linked load with acquire semantics - Adds implementation of ll/sc for 64bit RISC-V platforms. Note: The RISC-V "A" extension spec (Volume I, §A.6 "Eventual Success of Store-Conditional Instructions") defines a constrained LR/SC loop that is guaranteed to eventually succeed: > A constrained LR/SC loop contains at most 16 instructions in a 64-byte aligned region, > has exactly one LR, one SC, and one backward branch to the LR, no > other memory accesses (besides the LR and SC themselves), and no other branches or jumps. The load of the next item pointer strictly makes our loops non-constrained. However, in practice current platforms will correctly handle this and allow the loop to succeed. A configure check attempts to check whether non-constrained loops are safe and disables LL/SC if not. Signed-off-by: Joseph Schuchart --- config/opal_config_asm.m4 | 103 ++++++++++++++++++ docs/release-notes/changelog/v6.1.x.rst | 12 +++ opal/class/opal_fifo.h | 13 ++- opal/class/opal_lifo.h | 3 +- opal/include/opal/sys/arm64/atomic_llsc.h | 35 ++++++- opal/include/opal/sys/atomic.h | 9 ++ opal/include/opal/sys/atomic_impl_ptr_llsc.h | 5 + opal/include/opal/sys/powerpc/atomic_llsc.h | 47 ++++++++- opal/include/opal/sys/riscv64/Makefile.am | 2 + opal/include/opal/sys/riscv64/atomic_llsc.h | 105 +++++++++++++++++++ 10 files changed, 323 insertions(+), 11 deletions(-) create mode 100644 opal/include/opal/sys/riscv64/atomic_llsc.h diff --git a/config/opal_config_asm.m4 b/config/opal_config_asm.m4 index d6acad450c7..be02e524f1e 100644 --- a/config/opal_config_asm.m4 +++ b/config/opal_config_asm.m4 @@ -20,6 +20,7 @@ dnl Copyright (c) 2020 Google, LLC. All rights reserved. dnl Copyright (c) 2020 Intel, Inc. All rights reserved. dnl Copyright (c) 2021 IBM Corporation. All rights reserved. dnl Copyright (c) 2026 Jeffrey M. Squyres. All rights reserved. +dnl Copyright (c) 2026 Stony Brook University. All rights reserved. dnl $COPYRIGHT$ dnl dnl Additional copyrights may follow @@ -505,6 +506,104 @@ AC_DEFUN([OPAL_CHECK_CMPXCHG16B],[ ])dnl +dnl ################################################################# +dnl +dnl OPAL_CHECK_RISCV_LLSC_LIFO +dnl +dnl Check whether a RISC-V LR/SC sequence that includes a plain load +dnl between the LR and SC (as used in opal_lifo/opal_fifo pop) makes +dnl forward progress on this target. Such a sequence lies outside the +dnl RISC-V ISA "constrained LR/SC loop" definition (A-extension §8.3), +dnl so architectural forward-progress guarantees do not apply. All +dnl known implementations handle it correctly. When cross-compiling +dnl we default to disabled; use --enable-riscv-llsc-lifo to override. +dnl +dnl Defines OPAL_HAVE_RISCV_LLSC_LIFO to 0 or 1. +dnl +dnl ################################################################# +AC_DEFUN([OPAL_CHECK_RISCV_LLSC_LIFO],[ + OPAL_VAR_SCOPE_PUSH([riscv_llsc_result opal_check_riscv64]) + riscv_llsc_result=0 + + AC_ARG_ENABLE([riscv-llsc-lifo], + [AS_HELP_STRING([--enable-riscv-llsc-lifo], + [Enable RISC-V LR/SC-based LIFO/FIFO pop. The pop loop + loads the next-pointer between the LR and SC instructions, + which is outside the RISC-V ISA constrained LR/SC loop + definition (A-extension §8.3) and not architecturally + guaranteed to make forward progress. All known + implementations handle it correctly. Defaults to enabled + on native builds where a run-test succeeds; defaults to + disabled when cross-compiling. (default: auto)])]) + + dnl Use the same oracle as opal/include/opal/sys/atomic.h + dnl (PLATFORM_ARCH_RISCV && PLATFORM_ARCH_64, both derived from + dnl compiler-predefined macros), not the configure ${host} triplet, + dnl so this check and the header agree even with a custom or + dnl mismatched --host triplet. + AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[ +#if !(defined(__riscv) && __riscv_xlen == 64) +#error "not a 64-bit RISC-V target" +#endif + ]], [[]])], + [opal_check_riscv64=yes], + [opal_check_riscv64=no]) + + AS_IF([test "$opal_check_riscv64" = "yes"], + [AS_IF([test "$enable_riscv_llsc_lifo" = "no"], + [riscv_llsc_result=0], + [test "$enable_riscv_llsc_lifo" = "yes"], + [riscv_llsc_result=1], + [dnl auto: run a smoke test + AC_MSG_CHECKING([if RISC-V LR/SC with intermediate load makes forward progress]) + AC_RUN_IFELSE( + [AC_LANG_SOURCE([[ + struct opal_riscv_llsc_test_node { + volatile long next; + }; + + int main(void) { + struct opal_riscv_llsc_test_node n0 = { .next = 0 }; + volatile long head = (long) &n0; + long val, next; + int i, r; + + for (i = 0; i < 1000; i++) { + /* Mirrors opal_lifo_pop_atomic() / + * opal_fifo_pop_atomic(): LR the head + * pointer, then dereference it (a load + * through the address the LR just + * returned) between the LR and SC. */ + __asm__ __volatile__("lr.d.aq %0, (%1)" + : "=&r"(val) : "r"(&head) : "memory"); + next = ((struct opal_riscv_llsc_test_node *) val)->next; + __asm__ __volatile__("sc.d %0, %2, (%1)" + : "=&r"(r) : "r"(&head), "r"(next) : "memory"); + if (r == 0) { + return 0; + } + } + return 1; + } + ]])], + [riscv_llsc_result=1 + AC_MSG_RESULT([yes])], + [riscv_llsc_result=0 + AC_MSG_RESULT([no])], + [riscv_llsc_result=0 + AC_MSG_RESULT([unknown (cross-compiling) -- defaulting to disabled; use --enable-riscv-llsc-lifo to override])]) + ]) + ]) + + AC_DEFINE_UNQUOTED([OPAL_HAVE_RISCV_LLSC_LIFO], + [$riscv_llsc_result], + [Whether the RISC-V LR/SC pop path is enabled for LIFO/FIFO]) + + OPAL_VAR_SCOPE_POP +])dnl + + dnl ################################################################# dnl dnl OPAL_CHECK_INLINE_GCC([action-if-found], [action-if-not-found]) @@ -713,6 +812,10 @@ AC_DEFUN([OPAL_CONFIG_ASM],[ [$want_asm_atomics], [Whether to use assembly-coded atomics for atomics implementation]) + # Check for RISC-V LR/SC support for LIFO/FIFO operations (independent + # of the main atomics backend; applies even when using GCC/C11 builtins) + OPAL_CHECK_RISCV_LLSC_LIFO + OPAL_SUMMARY_ADD([Miscellaneous], [Atomics], [], [$atomics_found]) OPAL_VAR_SCOPE_POP diff --git a/docs/release-notes/changelog/v6.1.x.rst b/docs/release-notes/changelog/v6.1.x.rst index 3d838a6462d..3750bfaeb73 100644 --- a/docs/release-notes/changelog/v6.1.x.rst +++ b/docs/release-notes/changelog/v6.1.x.rst @@ -74,6 +74,18 @@ Open MPI version v6.1.0 MPI tree that still calls ``opal_bitmap_set_bit()`` on it will compile and misreport; such components must be updated. +- Added a new ``--enable-riscv-llsc-lifo`` / ``--disable-riscv-llsc-lifo`` + configure option controlling whether ``opal_lifo`` / ``opal_fifo`` use + a RISC-V LR/SC-based pop path. That pop loop loads the next-pointer + between the LR and SC instructions, which lies outside the RISC-V ISA + "constrained LR/SC loop" definition (A-extension section 8.3), so the + architecturally guaranteed forward-progress property does not apply, + even though all known implementations handle it correctly. On a + native build, configure now probes for forward progress and enables + the path automatically when the probe succeeds; cross-compiling + disables it by default since the probe cannot run, and either configure + option overrides the auto-detection in both cases. + - Fixed ``btl/tcp`` failing to connect, or hanging, on nodes that have an interface whose IP address is not unique across the job -- a container bridge such as ``docker0``, which is commonly ``172.17.0.1`` on every diff --git a/opal/class/opal_fifo.h b/opal/class/opal_fifo.h index 330636517bc..0ad18092327 100644 --- a/opal/class/opal_fifo.h +++ b/opal/class/opal_fifo.h @@ -16,6 +16,7 @@ * reserved. * Copyright (c) 2021 Triad National Security, LLC. All rights reserved. * Copyright (c) 2021 Google, LLC. All rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -229,7 +230,9 @@ static inline opal_list_item_t *opal_fifo_pop_atomic(opal_fifo_t *fifo) attempt = 0; } - opal_atomic_ll_ptr(&fifo->opal_fifo_head.data.item, item); + /* ll_acq provides acquire ordering: item fields (opal_list_next, + * payload) are visible after the load, paired with the wmb in push. */ + opal_atomic_ll_acq_ptr(&fifo->opal_fifo_head.data.item, item); if (ghost == item) { if ((intptr_t) ghost == fifo->opal_fifo_tail.data.item) { return NULL; @@ -269,6 +272,14 @@ static inline opal_list_item_t *opal_fifo_pop_atomic(opal_fifo_t *fifo) # endif if (ghost == next) { + /* Ensure the head-pointer update is visible to all threads before we + * change the tail to ghost. Without this barrier a concurrent push on + * a weak-memory architecture could observe tail=ghost while head still + * shows the old value; the push would then non-atomically write the new + * item as the head, racing with the store that set head=ghost, and the + * new item could become unreachable. */ + opal_atomic_wmb(); + void *tmp = item; if (!opal_atomic_compare_exchange_strong_ptr(&fifo->opal_fifo_tail.data.item, diff --git a/opal/class/opal_lifo.h b/opal/class/opal_lifo.h index dd827f665ad..86299c16f4b 100644 --- a/opal/class/opal_lifo.h +++ b/opal/class/opal_lifo.h @@ -18,6 +18,7 @@ * and Technology (RIST). All rights reserved. * Copyright (c) 2021 Triad National Security, LLC. All rights reserved. * Copyright (c) 2021 Google, LLC. All rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -219,7 +220,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo) attempt = 0; } - opal_atomic_ll_ptr(&lifo->opal_lifo_head.data.item, item); + opal_atomic_ll_acq_ptr(&lifo->opal_lifo_head.data.item, item); if (&lifo->opal_lifo_ghost == item) { return NULL; } diff --git a/opal/include/opal/sys/arm64/atomic_llsc.h b/opal/include/opal/sys/arm64/atomic_llsc.h index 8891ffbbdfd..76164bcca09 100644 --- a/opal/include/opal/sys/arm64/atomic_llsc.h +++ b/opal/include/opal/sys/arm64/atomic_llsc.h @@ -18,6 +18,7 @@ * Copyright (c) 2021 Google, LLC. All rights reserved. * Copyright (c) 2022 Amazon.com, Inc. or its affiliates. * All Rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -39,10 +40,15 @@ # define OPAL_HAVE_ATOMIC_LLSC_32 1 # define OPAL_HAVE_ATOMIC_LLSC_64 1 +/* Plain LL/SC: no hardware ordering. Use these when the caller provides + * its own barriers (opal_atomic_rmb / opal_atomic_wmb) or when no ordering + * is needed (e.g. a spin-retry that does not dereference the loaded pointer). + * The "memory" clobber acts as a compiler barrier so that the compiler does + * not reorder surrounding memory accesses across the asm block. */ # define opal_atomic_ll_32(addr, ret) \ do { \ opal_atomic_int32_t *_addr = (addr); \ - __asm__ __volatile__("ldaxr %w0, [%1] \n" \ + __asm__ __volatile__("ldxr %w0, [%1] \n" \ : "=&r"(ret) : "r"(_addr) : "memory"); \ } while (0) @@ -52,7 +58,7 @@ int32_t _newval = (int32_t) newval; \ int _ret; \ \ - __asm__ __volatile__("stlxr %w0, %w2, [%1] \n" \ + __asm__ __volatile__("stxr %w0, %w2, [%1] \n" \ : "=&r"(_ret) \ : "r"(_addr), "r"(_newval) \ : "cc", "memory"); \ @@ -63,8 +69,7 @@ # define opal_atomic_ll_64(addr, ret) \ do { \ opal_atomic_int64_t *_addr = (addr); \ - \ - __asm__ __volatile__("ldaxr %0, [%1] \n" \ + __asm__ __volatile__("ldxr %0, [%1] \n" \ : "=&r"(ret) : "r"(_addr) : "memory"); \ } while (0) @@ -74,7 +79,7 @@ int64_t _newval = (int64_t) newval; \ int _ret; \ \ - __asm__ __volatile__("stlxr %w0, %2, [%1] \n" \ + __asm__ __volatile__("stxr %w0, %2, [%1] \n" \ : "=&r"(_ret) \ : "r"(_addr), "r"(_newval) \ : "cc", "memory"); \ @@ -82,6 +87,26 @@ ret = (_ret == 0); \ } while (0) +/* Acquire-LL: embed acquire ordering in the load so that callers do not need + * a separate opal_atomic_rmb() between the LL and their first dereference of + * the loaded pointer. ldaxr (load-acquire exclusive) prevents subsequent + * memory accesses from being observed before the load. + * Use opal_atomic_ll_acq_* when the loaded pointer will be immediately + * dereferenced (e.g. reading item->opal_list_next in lifo/fifo pop). */ +# define opal_atomic_ll_acq_32(addr, ret) \ + do { \ + opal_atomic_int32_t *_addr = (addr); \ + __asm__ __volatile__("ldaxr %w0, [%1] \n" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +# define opal_atomic_ll_acq_64(addr, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + __asm__ __volatile__("ldaxr %0, [%1] \n" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + #include "opal/sys/atomic_impl_ptr_llsc.h" # endif /* OPAL_C_GCC_INLINE_ASSEMBLY */ diff --git a/opal/include/opal/sys/atomic.h b/opal/include/opal/sys/atomic.h index 7ae4d8e40f7..37416c05def 100644 --- a/opal/include/opal/sys/atomic.h +++ b/opal/include/opal/sys/atomic.h @@ -19,6 +19,7 @@ * Copyright (c) 2020-2021 Google, LLC. All rights reserved. * Copyright (c) 2022 Amazon.com, Inc. or its affiliates. * All Rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -397,14 +398,20 @@ static inline void opal_atomic_ll_32(opal_atomic_int32_t *addr, int32_t &ret); static inline void opal_atomic_sc_32(opal_atomic_int32_t *addr, int32_t newval, int &ret); +static inline void opal_atomic_ll_acq_32(opal_atomic_int32_t *addr, int32_t &ret); + static inline void opal_atomic_ll_64(opal_atomic_int64_t *addr, int64_t &ret); static inline void opal_atomic_sc_64(opal_atomic_int64_t *addr, int64_t newval, int &ret); +static inline void opal_atomic_ll_acq_64(opal_atomic_int64_t *addr, int64_t &ret); + static inline void opal_atomic_ll_ptr(opal_atomic_intptr_t *addr, intptr_t &ret); static inline void opal_atomic_sc_ptr(opal_atomic_intptr_t *addr, intptr_t newval, int &ret); +static inline void opal_atomic_ll_acq_ptr(opal_atomic_intptr_t *addr, intptr_t &ret); + #endif @@ -439,6 +446,8 @@ static inline void opal_atomic_sc_ptr(opal_atomic_intptr_t *addr, intptr_t newva # include "opal/sys/arm64/atomic_llsc.h" #elif defined(PLATFORM_ARCH_POWERPC) && defined(PLATFORM_ARCH_64) # include "opal/sys/powerpc/atomic_llsc.h" +#elif defined(PLATFORM_ARCH_RISCV) && defined(PLATFORM_ARCH_64) && OPAL_HAVE_RISCV_LLSC_LIFO +# include "opal/sys/riscv64/atomic_llsc.h" #endif diff --git a/opal/include/opal/sys/atomic_impl_ptr_llsc.h b/opal/include/opal/sys/atomic_impl_ptr_llsc.h index 82932e3f321..d6a2bbdc34f 100644 --- a/opal/include/opal/sys/atomic_impl_ptr_llsc.h +++ b/opal/include/opal/sys/atomic_impl_ptr_llsc.h @@ -18,6 +18,7 @@ * Copyright (c) 2020-2021 Google, LLC. All rights reserved. * Copyright (c) 2022 Amazon.com, Inc. or its affiliates. * All Rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -39,6 +40,8 @@ # define opal_atomic_ll_ptr(addr, ret) opal_atomic_ll_32((opal_atomic_int32_t *) (addr), ret) # define opal_atomic_sc_ptr(addr, value, ret) \ opal_atomic_sc_32((opal_atomic_int32_t *) (addr), (intptr_t)(value), ret) +# define opal_atomic_ll_acq_ptr(addr, ret) \ + opal_atomic_ll_acq_32((opal_atomic_int32_t *) (addr), ret) # define OPAL_HAVE_ATOMIC_LLSC_PTR 1 @@ -47,6 +50,8 @@ # define opal_atomic_ll_ptr(addr, ret) opal_atomic_ll_64((opal_atomic_int64_t *) (addr), ret) # define opal_atomic_sc_ptr(addr, value, ret) \ opal_atomic_sc_64((opal_atomic_int64_t *) (addr), (intptr_t)(value), ret) +# define opal_atomic_ll_acq_ptr(addr, ret) \ + opal_atomic_ll_acq_64((opal_atomic_int64_t *) (addr), ret) # define OPAL_HAVE_ATOMIC_LLSC_PTR 1 diff --git a/opal/include/opal/sys/powerpc/atomic_llsc.h b/opal/include/opal/sys/powerpc/atomic_llsc.h index 2fdafc8a650..d31f6041e93 100644 --- a/opal/include/opal/sys/powerpc/atomic_llsc.h +++ b/opal/include/opal/sys/powerpc/atomic_llsc.h @@ -16,6 +16,7 @@ * Copyright (c) 2021 Google, LLC. All rights reserved. * Copyright (c) 2022 Amazon.com, Inc. or its affiliates. * All Rights reserved. + * Copyright (c) 2026 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -43,6 +44,10 @@ * is that even with an always_inline attribute the compiler may still emit instructions to store * then load the arguments to/from the stack. This sequence may cause the ll reservation to be * cancelled. */ + +/* Plain LL/SC: no hardware ordering. Use these when the caller provides + * its own barriers or when no ordering is needed. The "memory" clobber + * acts as a compiler barrier. */ #define opal_atomic_ll_32(addr, ret) \ do { \ opal_atomic_int32_t *_addr = (addr); \ @@ -65,10 +70,13 @@ ret = _ret; \ } while (0) -#define opal_atomic_ll_64(addr, ret) \ - do { \ - opal_atomic_int64_t *_addr = (addr); \ - __asm__ __volatile__("ldarx %0, 0, %1 \n\t" : "=&r"(ret) : "r"(_addr) : "memory"); \ +#define opal_atomic_ll_64(addr, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + __asm__ __volatile__("ldarx %0, 0, %1 \n\t" \ + : "=&r"(ret) \ + : "r"(_addr), "m"(*_addr) \ + : "memory"); \ } while (0) #define opal_atomic_sc_64(addr, value, ret) \ @@ -88,6 +96,37 @@ ret = _ret; \ } while (0) +/* Acquire-LL variants: lwarx/ldarx followed immediately by lwsync. + * + * lwsync (lightweight sync) provides load→load and load→store ordering + * unconditionally and requires no conditional branch, unlike isync. + * The Power ISA isync idiom requires "lwarx → branch-on-result → isync"; + * because OPAL splits LL and SC into separate macros around arbitrary C + * code, that branch occurs outside the asm block, making isync inside + * the asm architecturally incorrect. lwsync is the correct alternative. + * lwsync does not cancel the exclusive reservation set by lwarx/ldarx. + * + * Use opal_atomic_ll_acq_* when the loaded pointer will be immediately + * dereferenced (e.g. reading item->opal_list_next in lifo/fifo pop). + * Callers needing store-store ordering after a plain SC (e.g. ordering + * head=ghost before a subsequent tail CAS) should use an explicit + * opal_atomic_wmb() after the SC, outside the LL-SC retry loop. */ +#define opal_atomic_ll_acq_32(addr, ret) \ + do { \ + opal_atomic_int32_t *_addr = (addr); \ + __asm__ __volatile__("lwarx %0, 0, %1 \n\t" \ + "lwsync \n\t" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +#define opal_atomic_ll_acq_64(addr, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + __asm__ __volatile__("ldarx %0, 0, %1 \n\t" \ + "lwsync \n\t" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + #include "opal/sys/atomic_impl_ptr_llsc.h" #endif /* OPAL_C_GCC_INLINE_ASSEMBLY */ diff --git a/opal/include/opal/sys/riscv64/Makefile.am b/opal/include/opal/sys/riscv64/Makefile.am index 783d0262864..509559dfa2d 100644 --- a/opal/include/opal/sys/riscv64/Makefile.am +++ b/opal/include/opal/sys/riscv64/Makefile.am @@ -1,6 +1,7 @@ # # Copyright (c) 2025 Software System Team, SANECHIPS. # All rights reserved. +# Copyright (c) 2026 Stony Brook University. All rights reserved. # $COPYRIGHT$ # # Additional copyrights may follow @@ -12,5 +13,6 @@ # This makefile.am does not stand on its own - it is included from opal/include/Makefile.am headers += \ + opal/sys/riscv64/atomic_llsc.h \ opal/sys/riscv64/timer.h diff --git a/opal/include/opal/sys/riscv64/atomic_llsc.h b/opal/include/opal/sys/riscv64/atomic_llsc.h new file mode 100644 index 00000000000..998d58307e4 --- /dev/null +++ b/opal/include/opal/sys/riscv64/atomic_llsc.h @@ -0,0 +1,105 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ +/* + * Copyright (c) 2026 Stony Brook University. All rights reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + * SPDX-License-Identifier: BSD-3-Clause-Open-MPI + */ + +#ifndef OPAL_SYS_ARCH_ATOMIC_LLSC_H +#define OPAL_SYS_ARCH_ATOMIC_LLSC_H 1 + +/* + * This file is included even when C11 or GCC built-in atomics are + * used for the main atomic operations, which is why we check for + * gcc inline assembly support explicitly. + * + * NOTE: The LR/SC loops in opal_lifo_pop_atomic / opal_fifo_pop_atomic + * load item->opal_list_next between the LR and SC instructions. That + * load is a memory access not permitted inside a RISC-V "constrained + * LR/SC loop" (ISA spec A-extension §8.3), so the architecturally + * guaranteed forward-progress property does not apply. All known + * RISC-V implementations handle this correctly, matching the behaviour + * of ARM64 (ldxr/stxr) and PowerPC (lwarx/stwcx.) where the same + * split-macro pattern is used and intermediate loads are likewise + * permitted by those architectures' respective ISA specifications. + */ + +#if OPAL_C_GCC_INLINE_ASSEMBLY + +#define OPAL_HAVE_ATOMIC_LLSC_32 1 +#define OPAL_HAVE_ATOMIC_LLSC_64 1 + +/* Plain LL/SC: no hardware ordering. Use these when the caller provides + * its own barriers (opal_atomic_rmb / opal_atomic_wmb) or when no ordering + * is needed (e.g. a spin-retry that does not dereference the loaded pointer). + * The "memory" clobber acts as a compiler barrier so that the compiler does + * not reorder surrounding memory accesses across the asm block. */ + +#define opal_atomic_ll_32(addr, ret) \ + do { \ + opal_atomic_int32_t *_addr = (addr); \ + __asm__ __volatile__("lr.w %0, (%1)" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +#define opal_atomic_sc_32(addr, newval, ret) \ + do { \ + opal_atomic_int32_t *_addr = (addr); \ + int32_t _newval = (int32_t)(newval); \ + int32_t _ret; \ + __asm__ __volatile__("sc.w %0, %2, (%1)" \ + : "=&r"(_ret) \ + : "r"(_addr), "r"(_newval) \ + : "memory"); \ + ret = (_ret == 0); \ + } while (0) + +#define opal_atomic_ll_64(addr, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + __asm__ __volatile__("lr.d %0, (%1)" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +#define opal_atomic_sc_64(addr, newval, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + int64_t _newval = (int64_t)(newval); \ + int32_t _ret; \ + __asm__ __volatile__("sc.d %0, %2, (%1)" \ + : "=&r"(_ret) \ + : "r"(_addr), "r"(_newval) \ + : "memory"); \ + ret = (_ret == 0); \ + } while (0) + +/* Acquire-LL: embed acquire ordering in the load so that callers do not need + * a separate opal_atomic_rmb() between the LL and their first dereference of + * the loaded pointer. lr.w.aq / lr.d.aq prevent subsequent memory accesses + * from being observed before the load. + * Use opal_atomic_ll_acq_* when the loaded pointer will be immediately + * dereferenced (e.g. reading item->opal_list_next in lifo/fifo pop). */ + +#define opal_atomic_ll_acq_32(addr, ret) \ + do { \ + opal_atomic_int32_t *_addr = (addr); \ + __asm__ __volatile__("lr.w.aq %0, (%1)" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +#define opal_atomic_ll_acq_64(addr, ret) \ + do { \ + opal_atomic_int64_t *_addr = (addr); \ + __asm__ __volatile__("lr.d.aq %0, (%1)" \ + : "=&r"(ret) : "r"(_addr) : "memory"); \ + } while (0) + +#include "opal/sys/atomic_impl_ptr_llsc.h" + +#endif /* OPAL_C_GCC_INLINE_ASSEMBLY */ + +#endif /* ! OPAL_SYS_ARCH_ATOMIC_LLSC_H */