Repository navigation
fix(rocm): make hipBLASLt GEMM state safe across threads and streams #2200
Description
Part of #1801
Problem / Background
The review of #2198 (rocBLAS handle init and stream rebind races in the ROCm overlay Device) found the same class of race in the hipBLASLt GEMM layer, which lives outside Device and is out of that issue's scope. hipBLASLt is the default route for every bf16/fp16 GEMM, because use_hip_graphs() always returns false (device.cpp:43-54) and the graph gate in is_hipblaslt_available() (gemms/hipblaslt_gemm.cpp:1036) never trips. The server runs forwards on several OS threads that share one Device, each on its own stream: BatchScheduler, EmbeddingWorker, RerankWorker and AudioWorker (#2183, #2196). So -m <chat> --embedding-model <emb> puts two threads into this file at once, and the opt-in async MoE path (MLX_ROCM_MOE_ASYNC=1, matmul.cpp:53-100, :1454-1462) adds a third caller: the HIP host-callback thread, which serves every stream.
Current Behavior
Paths are under src/lib/mlx-cpp/patches-rocm/mlx/backend/rocm/gemms/hipblaslt_gemm.cpp, read on main at 84d3a7b. Shared mutable state:
| State | Lines | Guard today | Problem |
|---|---|---|---|
g_state[d].initialized |
:30, set :58, read :55, :89, :1044 | written under state.mutex, read unlocked at :89 and :1044 |
set before hipblasLtCreate (:60); plain bool, a data race |
g_state[d].handle, .available |
:29, :31, written :60-68, read :93-96, :1048 | written under state.mutex, read unlocked |
a reader that sees initialized can see available == false or a null handle |
g_state[d].workspace, .workspace_size |
:35-36, init :75-84, ensure_workspace :104-126 |
init under state.mutex; ensure_workspace unlocked (its comment at :101-103 assumes one submitting thread per device) |
one buffer for every stream on the device; unlocked hipFree/hipMalloc |
g_caps[d] |
:156, gemm_caps :208-244 |
g_caps_mutex for the whole probe |
sound, but probed is set at :214 before get_handle (:217); if that throws in the init window, every capability stays false for the process |
g_pipe_cache and the descriptors each GemmPipe owns |
:330-339 | map lookups under g_pipe_mutex; the pipe and its descriptors are used after unlock (:409-437, :905-945) |
:443-454 destroys and erases an entry another thread may be passing to hipblasLtMatmul |
algo_cache in hipblaslt_gemm_impl |
:716-719 | algo_mutex, entries copied out by value |
sound |
fp8 algo_cache |
:1510-1512 | mtx, copied by value |
sound |
| env-flag function statics | :258, :377, :384, :691, :768, :1027, :1252 | C++11 static init, never written after | sound |
the hipblasLtHandle_t itself |
used by every thread | none | hipblaslt.h:524-525 asks callers to synchronize only handle-mutating helpers; this file never mutates the handle after create |
Failure modes, worst first:
- Shared workspace (corrupts output). Every
hipblasLtMatmulon the device gets the same 32 MB buffer (:417, :858, :986, :1633, and the tune loops at :777 and :1562). The call only enqueues, so two GEMMs on two streams whose algorithms request workspace run on the GPU at the same time over the same scratch memory, and both outputs are wrong. Stream order protects reuse only within one stream. - Init window, worse interleaving (corrupts output or faults). Thread A sets
initialized(:58), creates the handle, setsavailable(:68), and only then allocates the workspace (:78-83). Thread B arriving after :68 skips the lock, runs a GEMM needing workspace, seesworkspace == nullptr(:106), and callshipMalloc(&state.workspace, required)(:118) unlocked while A writes the same fields. The ordering A ptr, B ptr, B size (:124), A size (:79) leaves{B's small buffer, 32 MB}, and later GEMMs with a larger workspace write past B's allocation. Other orderings leak one buffer. - Workspace growth (GPU use-after-free). When the recorded size is below a request (init allocation failed at :78, or the interleaving above),
ensure_workspacefrees the buffer (:111) that another stream's queued GEMM still uses. - Pipe erase (host use-after-free). On a failed cached-pipe matmul (stale bucketed algo, the case :441-442 describes), the thread destroys the pipe's layouts and desc (:447-451) and erases it while another thread holds
pipefrom :409, oruse_la..use_descfrom :905-923, and passes them tohipblasLtMatmul. The erase is reachable: the retry at :949-1008 fixesalgo_cachebut leaves the bad algo in the pipe, so the next call on that key fails and erases. The comment at :295 ("Process-lifetime; never destroyed") is stale. - Init window, common interleaving (wrong route, no corruption). Thread B arriving between :58 and :68 reads
initialized == true, skips the lock, and seesavailable == false:is_hipblaslt_available()returnsfalseand that GEMM falls back to rocBLAS, andget_handle()throws "hipBLASLt is not available on this device." (callers such asmatmul.cpp:199catch and fall back). If B is insidegemm_caps, the throw leavesg_caps[d]allfalseforever, sodevice_has_fp8_gemm()(:1426) disables the fp8 prefill route (quantized/qmm.hip:884) for the process. init_handlecallshipblasLtCreatewithout makingdevice_idcurrent (hipSetDevicewraps only thehipMalloc, :75-84).
Measured on the only AMD host available (gfx1151, ROCm 7.15, hipBLASLt 1.4): with HIPBLASLT_MATMUL_PREF_MAX_WORKSPACE_BYTES at 32 MB, hipblasLtMatmulAlgoGetHeuristic returned workspaceSize == 0 for all 8 candidates of each of 15 shapes (bf16 TN decode, prefill and MLP shapes from [4096, 1] x 4096 to [11008, 512] x 4096 and K up to 1M; f32 NN up to 2048^3). So on that device the three workspace races never run, and the reachable races are the pipe erase and the init window. Archs whose Tensile libraries pick split-K algorithms with global scratch (CDNA, gfx1201) were not measured.
Upstream MLX at the pin (81ba1c6a): mlx/backend/cuda/cublas_utils.cpp:44-79 keeps a cuBLASLt handle and preference per thread per device in static thread_local std::vector<CublasHandles>, creation throws through CHECK_CUBLAS_ERROR, and the stream is passed per call (:197-215). The workspace is allocated per call by allocate_workspace (mlx/backend/cuda/utils.cpp:82-99): cu::malloc_async from the MLX allocator, rounded up to 256 bytes, registered with encoder.add_temporary so it lives until that command buffer completes, with a debug assert that the stream is not capturing.
Proposed Solution
All changes are in gemms/hipblaslt_gemm.cpp and gemms/hipblaslt_gemm.h, plus one call in device.cpp.
Handle init. Replace initialized and available with std::atomic<int> init_state{0} (0 not tried, 1 ready, 2 unavailable). Add void ensure_handle(HipblasltState&, int device_id): return on a nonzero acquire load; otherwise lock state.mutex, recheck, save and set the current device, hipblasLtCreate into a local, restore the device, then on success store state.handle and init_state.store(1, std::memory_order_release); on failure print today's warning and store 2. Failure stays permanent as today, because is_hipblaslt_available() runs on every GEMM route decision and must not retry a create each time. get_handle() and is_hipblaslt_available() call ensure_handle and then read init_state; neither reads a field outside the lock before the flag says ready. Remove the workspace allocation from init. Lock order stays g_caps_mutex then state.mutex. Keep one handle per device. Rejected: per-thread handles as upstream does, because algo_cache and g_pipe_cache hold heuristic results computed on one handle and shared across threads, and the async MoE path would create handles inside a HIP host callback.
Workspace: one per stream. Replace ensure_workspace and the two HipblasltState workspace fields with std::pair<void*, size_t> stream_workspace(int device_id, hipStream_t stream, size_t required) over static std::mutex g_ws_mutex and static std::unordered_map<hipStream_t, void*> g_ws[kMaxDevices]. required == 0 returns {nullptr, 0} without locking. Otherwise, under the lock, return the stream's buffer with size kMaxWorkspaceBytes; if it has none, allocate exactly kMaxWorkspaceBytes with device_id current and store it. Every heuristic preference already caps at that size (:186, :545-550, :1485), so a buffer never grows and never moves while its stream lives. Under stream capture (hipStreamGetCaptureInfo not None) with no buffer yet, throw std::runtime_error("hipBLASLt: algorithm needs N bytes of workspace on a capturing stream with none allocated") rather than call hipMalloc; today's capture-capable hipBLASLt call is the fp8 path (:1550-1553), which requires M >= 64 and is not reached from decode capture. On allocation failure return {nullptr, 0}: the tune loops keep skipping that algo (:780, :1565), and every other caller (:417, :858, :986, :1633) throws the existing "hipBLASLt: failed to allocate workspace of N bytes" message instead of passing a null buffer to hipblasLtMatmul. Add void hipblaslt_release_stream_workspace(hipStream_t stream) to hipblaslt_gemm.h, which synchronizes the stream, frees its buffer and erases the entry, and call it from CommandEncoder::~CommandEncoder (device.cpp:477) so a later stream that reuses the handle value cannot inherit a buffer its predecessor is still draining. Cost: 32 MB per stream that runs a workspace-needing GEMM, and nothing on devices like gfx1151 whose heuristics request none (which also drops the 32 MB per device that init allocates today).
Why per-stream and not a lease: a lease held for the duration of the hipblasLtMatmul call ends when the call returns, before the kernel runs, so it does not stop the next lessee's kernel on another stream from overlapping on the GPU. Making it safe needs either hipStreamSynchronize inside the lease, which serializes GEMMs and is illegal under capture, or event-ordered handoff (hipStreamWaitEvent between unrelated workers' streams), which serializes GEMMs on the GPU. A thread-local buffer is also wrong, because the HIP callback thread runs the async MoE GEMMs for every stream. Per-call allocator temporaries as upstream does are not available here, because hipblaslt_gemm_raw, hipblaslt_gemm_fp8_raw and hipblaslt_gemm_rowmajor_on_stream receive a bare hipStream_t with no CommandEncoder, and a per-call hipMalloc is not capturable.
Pipe lifetime: shared ownership. Store std::shared_ptr<GemmPipe> in g_pipe_cache. Give GemmPipe a destructor that destroys its four layouts and desc, and delete its copy operations. The fast path copies the shared_ptr under g_pipe_mutex and uses it after unlock; the failure path at :443-454 only erases the map entry, and the descriptors die with the last holder. The miss path builds a std::make_shared<GemmPipe>, emplaces it, adopts the existing entry when !inserted, and holds that shared_ptr through the matmul and the retry (:929-1008). Delete the manual destroy calls at :447-451 and :914-918 and fix the comment at :292-295. Rejected: holding g_pipe_mutex across hipblasLtMatmul, which would serialize host enqueue of every hipBLASLt GEMM in the process, including the MoE segment loop.
Scope
In scope: gemms/hipblaslt_gemm.cpp, gemms/hipblaslt_gemm.h, the release call in device.cpp, a new test file, a LOCAL_FIXES entry, and removing MLX_NO_HIPBLASLT=1 from tests/rocm_rocblas_handle_concurrency.rs once #2198 has landed.
Out of scope: the rocBLAS handle and stream races (#2198), the leaked new float[2] alpha/beta pairs (:1093, :1161), the retry path not refreshing a pipe's stale algo (:949-1008, which only makes the erase reachable), and the CUDA and Metal backends.
Ordering: land #2198 first. Both touch adjacent GEMM code, every bf16 Matmul GEMM first passes is_rocblas_available() (matmul.cpp:775) before reaching hipBLASLt, and the #2198 test runs with MLX_NO_HIPBLASLT=1 because of this issue.
Implementation Notes
- Reuse: model the test on fix(rocm): initialize Device's rocBLAS handle once, under a lock #2198's
tests/rocm_rocblas_handle_concurrency.rsand ontests/rocm_qmm_env.rs(run_childat :213-283: re-runstd::env::current_exe()with<child> --exact --ignored, aCHILD_ENVmarker, captured stderr, 120 s deadline). Per-thread streams and the barrier come fromtests/rocm_jit_module_concurrency.rs(PR fix(rocm): lock the JIT module cache and the HIP event pool #2196):mlxcel_core::streams::new_thread_local_generation_streamandinstall_thread_local_default_stream. Build inputs withmlxcel_core::from_slice_f32,astype(.., dtype::BFLOAT16),matmulandtranspose. - Edge cases: two threads initializing at once (one create, both get the handle); create failure (warning once, state 2, no lock after); two streams needing workspace (distinct buffers); the same stream from the encoding thread and the HIP callback thread (same buffer, stream-ordered); a stream destroyed and its handle value reused (entry released first); a pipe erased while another thread uses it (descriptors live until that thread drops its
shared_ptr). - Error handling: callers that catch hipBLASLt exceptions keep their rocBLAS fallback (
matmul.cpp:199,quantized/qmm.hip:1293); the workspace and capture errors above arestd::runtime_errors naming the byte count. - LOCAL_FIXES: add an entry to
src/lib/mlx-cpp/patches-rocm/LOCAL_FIXES.mdunder "Fixes to the fork's kernels" with the next free number at merge time, namingmlx/backend/rocm/gemms/hipblaslt_gemm.cpp,gemms/hipblaslt_gemm.handdevice.cpp, stating what was shared or unlocked and how each is now owned, and ending with "Applies to the fork; kept in mlxcelverse under the 2026-10-06 fork policy, not proposed there." Add nothing underdocs/mlxcelverse/upstream/.
Acceptance Criteria
-
init_stateis published with release after a successfulhipblasLtCreatemade with the target device current; no field ofHipblasltStateis read outsidestate.mutexbefore an acquire load sees it set. -
ensure_workspaceis gone; everyhipblasLtMatmulin the file takes its buffer fromstream_workspace(device_id, stream, ..), two streams never share one, and~CommandEncoderreleases its stream's buffer. -
g_pipe_cacheholdsstd::shared_ptr<GemmPipe>; no rawGemmPipe*or descriptor copied from a map entry is used afterg_pipe_mutexis released without ashared_ptrkeeping it alive. (Superseded by measurement in PR fix(rocm): make hipBLASLt GEMM state safe across threads and streams #2208:hipblasLtMatmulwrites into the descriptor objects, so a pipe shared across threads fails however it is owned,shared_ptrincluded, 0 of 5 children; the pipe cache is thread-local instead, 5 of 5 and 20 of 20 runs.) -
tests/rocm_hipblaslt_concurrency.rs(#![cfg(feature = "rocm")], skips whengpu_backend_kind() != GpuBackendKind::Rocm) has a parent testhipblaslt_gemms_from_many_threads_in_fresh_processesthat runs the#[ignore]child 10 times, each in a fresh process withMLX_NO_HIPBLASLTandMLX_HIPBLASLT_NO_PIPE_CACHEremoved from its environment. The childchild_hipblaslt_gemms_from_many_threadsstarts 8 threads; each installs its own stream and waits on aBarrierbefore any GPU matmul has run in the process, then runs 33 rounds of a bf16[64, 64] x transpose([64, 64])(hipBLASLt TN pipe), a bf16 batched[4, 64, 64] x [4, 64, 64](hipblaslt_gemm_batched) and an f32[64, 128] x [128, 64](rocBLAS), all threads using the same shapes so they contend on the same pipe keys. Inputs are integers from a per-thread seed, in -2..=2 for bf16 (K = 64, so every output is an integer of magnitude at most 256 and exact in bf16) and -4..=4 for f32, and each output cast to f32 must equal the host i32 reference exactly. (bf16 inputs are-1..=1in PR fix(rocm): make hipBLASLt GEMM state safe across threads and streams #2208: on gfx1151 both libraries return residues near 2^-22 at exact-zero outputs when bf16 inputs mix zeros with magnitude 2, fix(rocm): bf16 GEMMs return 2^-22 residue at exact-zero outputs on gfx1151 #2206.) The parent also asserts each child's stderr contains[hipBLASLt caps] deviceexactly once (printed bygemm_caps, :234-242), which proves the bf16 GEMMs reached hipBLASLt. - With the fix removed (the old
hipblaslt_gemm.cpprestored), run the test file 20 times and report in the PR how many runs failed. If none did, say so: the test is then a guard, not a reproduction. The PR also reports theworkspaceSizevalues the heuristics return on the test host for the test shapes, so the reader knows whether the workspace path ran. -
tests/rocm_rocblas_handle_concurrency.rsno longer setsMLX_NO_HIPBLASLTand still passes. - The LOCAL_FIXES entry exists with the fork-policy wording and no upstream marking, and
make verify-rocm-overlaypasses. - Existing ROCm GEMM, MoE and qmm tests pass under
make verify-rocm.
Verification
cargo test --profile test-fast --features rocm --test rocm_hipblaslt_concurrency
cargo test --profile test-fast --features rocm --test rocm_rocblas_handle_concurrency
cargo clippy --features rocm --test rocm_hipblaslt_concurrency -- -D warnings
grep -n "ensure_workspace\|GemmPipe\*\|initialized" src/lib/mlx-cpp/patches-rocm/mlx/backend/rocm/gemms/hipblaslt_gemm.cpp
make verify-rocm-overlay
make verify-rocmA pass is all ten child runs exact with one caps line each, the grep empty, and both make targets green on an AMD host.
Priority
Medium. Output corruption needs an algorithm that requests workspace, and on gfx1151 every candidate measured requested none, so the shared-workspace and growth races do not run there. What is reachable on one GPU today is the pipe-erase use-after-free (needs a stale cached algo plus a concurrent same-geometry GEMM) and the init window, which only misroutes one GEMM or permanently disables the fp8 route. Raise to high if the PR's workspace report, or a run on gfx1201 or CDNA, shows nonzero workspaceSize for a shape the server runs.
Activity
docs: add technical report for PR #2196
fix(rocm): make hipBLASLt GEMM state safe across threads and streams
docs(rocm): explain the bf16 GEMM residue at exact zeros on gfx1151 (#…
fix(rocm): make hipBLASLt GEMM state safe across threads and streams (l…