From 5628d6223ee7c9fc51dbe31c6ae15ac0b57a1357 Mon Sep 17 00:00:00 2001 From: Arthur Lacote Date: Tue, 13 Oct 2026 17:39:12 +0200 Subject: [PATCH 1/2] Claim and release buffer slots without alloc_lock in pthreads builds blas_memory_alloc and blas_memory_free took the global alloc_lock mutex on every call in non-OpenMP threaded builds, so that concurrent callers serialized on it twice per BLAS call. Use the per-slot lock that OpenMP builds already use instead, and keep alloc_lock for the rare overflow path. Co-Authored-By: Claude Opus 5.5 --- driver/others/memory.c | 64 +++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/driver/others/memory.c b/driver/others/memory.c index 8e24773841..f1288cec08 100644 --- a/driver/others/memory.c +++ b/driver/others/memory.c @@ -2825,25 +2825,25 @@ void *blas_memory_alloc(int procpos){ position = 0; -#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - LOCK_COMMAND(&alloc_lock); -#endif + /* Slots of memory[] are claimed with their own lock rather than alloc_lock, + so that threads of the application calling OpenBLAS concurrently (e.g. + from an OpenMP parallel region) don't all take the same mutex twice per + call. */ do { RMB; -#if defined(USE_OPENMP) if (!memory[position].used) { blas_lock((BLASULONG *)&memory[position].lock); -#endif if (!memory[position].used) goto allocation; -#if defined(USE_OPENMP) blas_unlock((BLASULONG *)&memory[position].lock); } -#endif position ++; } while (position < NUM_BUFFERS); +#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) + LOCK_COMMAND(&alloc_lock); +#endif if (memory_overflowed) { do { @@ -2874,11 +2874,7 @@ void *blas_memory_alloc(int procpos){ #endif memory[position].used = 1; -#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - UNLOCK_COMMAND(&alloc_lock); -#else blas_unlock((BLASULONG *)&memory[position].lock); -#endif if (!memory[position].addr) { int failcount = 0; do { @@ -3127,48 +3123,46 @@ void blas_memory_free(void *free_area){ #endif position = 0; -#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - LOCK_COMMAND(&alloc_lock); -#endif + /* Only the owner of a slot of memory[] frees it, so it is released + without alloc_lock, as it is claimed in blas_memory_alloc. */ while ((position < NUM_BUFFERS) && (memory[position].addr != free_area)) position++; - if (position >= NUM_BUFFERS && !memory_overflowed) goto error; + if (likely(position < NUM_BUFFERS)) { +#ifdef DEBUG + printf(" Position : %d\n", position); +#endif + // arm: ensure all writes are finished before other thread takes this memory + WMB; + + memory[position].used = 0; #ifdef DEBUG - if (memory[position].addr != free_area) goto error; - printf(" Position : %d\n", position); + printf("Unmap Succeeded.\n\n"); #endif - if (unlikely(memory_overflowed && position >= NUM_BUFFERS)) { - while ((position < NUM_BUFFERS+NEW_BUFFERS) && (newmemory[position-NUM_BUFFERS].addr != free_area)) - position++; - // arm: ensure all writes are finished before other thread takes this memory - WMB; -if (position - NUM_BUFFERS >= NEW_BUFFERS) goto error; - newmemory[position-NUM_BUFFERS].used = 0; + return; + } + #if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - UNLOCK_COMMAND(&alloc_lock); + LOCK_COMMAND(&alloc_lock); #endif + if (!memory_overflowed) goto error; -#ifdef DEBUG - printf("Unmap from overflow area succeeded.\n\n"); -#endif - return; -} else { + while ((position < NUM_BUFFERS+NEW_BUFFERS) && (newmemory[position-NUM_BUFFERS].addr != free_area)) + position++; // arm: ensure all writes are finished before other thread takes this memory WMB; - - memory[position].used = 0; + if (position - NUM_BUFFERS >= NEW_BUFFERS) goto error; + newmemory[position-NUM_BUFFERS].used = 0; #if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) UNLOCK_COMMAND(&alloc_lock); #endif #ifdef DEBUG - printf("Unmap Succeeded.\n\n"); + printf("Unmap from overflow area succeeded.\n\n"); #endif - return; -} + error: printf("BLAS : Bad memory unallocation! : %4d %p\n", position, free_area); From 5e2694375de36deebcc22ce2c2cef8256665b1ba Mon Sep 17 00:00:00 2001 From: Arthur Lacote Date: Wed, 21 Oct 2026 03:06:33 +0200 Subject: [PATCH 2/2] Don't take alloc_lock for memory[] addr, nor the slot lock without locking addr is _Atomic and blas_memory_free reads it without alloc_lock, so the lock around its first store no longer protects anything. Builds with neither SMP nor USE_LOCKING took no lock before and still take none. Co-Authored-By: Claude Opus 5.5 --- driver/others/memory.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/driver/others/memory.c b/driver/others/memory.c index f1288cec08..eea073259e 100644 --- a/driver/others/memory.c +++ b/driver/others/memory.c @@ -2831,12 +2831,16 @@ void *blas_memory_alloc(int procpos){ call. */ do { RMB; +#if defined(SMP) || defined(USE_LOCKING) if (!memory[position].used) { blas_lock((BLASULONG *)&memory[position].lock); +#endif if (!memory[position].used) goto allocation; +#if defined(SMP) || defined(USE_LOCKING) blas_unlock((BLASULONG *)&memory[position].lock); } +#endif position ++; } while (position < NUM_BUFFERS); @@ -2874,7 +2878,9 @@ void *blas_memory_alloc(int procpos){ #endif memory[position].used = 1; +#if defined(SMP) || defined(USE_LOCKING) blas_unlock((BLASULONG *)&memory[position].lock); +#endif if (!memory[position].addr) { int failcount = 0; do { @@ -2942,13 +2948,7 @@ void *blas_memory_alloc(int procpos){ } while ((BLASLONG)map_address == -1); -#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - LOCK_COMMAND(&alloc_lock); -#endif memory[position].addr = map_address; -#if (defined(SMP) || defined(USE_LOCKING)) && !defined(USE_OPENMP) - UNLOCK_COMMAND(&alloc_lock); -#endif #ifdef DEBUG printf(" Mapping Succeeded. %p(%d)\n", (void *)memory[position].addr, position); @@ -3123,8 +3123,8 @@ void blas_memory_free(void *free_area){ #endif position = 0; - /* Only the owner of a slot of memory[] frees it, so it is released - without alloc_lock, as it is claimed in blas_memory_alloc. */ + /* Only the thread holding a buffer of memory[] frees it, so its slot is + released without alloc_lock, as it is claimed in blas_memory_alloc. */ while ((position < NUM_BUFFERS) && (memory[position].addr != free_area)) position++;