Skip to content

Commit f4828f7

Browse files
lindsayadcodex
andcommitted
Fix thread deadlock
Closes #5947 Co-authored-by: Codex <codex@openai.com>
1 parent 181aadf commit f4828f7

4 files changed

Lines changed: 263 additions & 23 deletions

File tree

common_thread.h

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,41 @@ extern int blas_is_num_threads_set_explicitly;
143143
static __inline int num_cpu_avail(int level) {
144144

145145
#ifdef USE_OPENMP
146-
/* If the user explicitly called openblas_set_num_threads(),
147-
respect that setting instead of overriding it with
148-
`omp_get_max_threads()` below (which is to get a default
149-
in case the user hasn't made an explicit choice). */
150-
if (blas_is_num_threads_set_explicitly) {
151-
return blas_cpu_number;
146+
int in_parallel = omp_in_parallel();
147+
int openmp_nthreads;
148+
149+
/*
150+
* An active outer OpenMP region takes precedence and uses the local
151+
* setting, which defaults to one. Outside a region, an explicit
152+
* openblas_set_num_threads() setting takes precedence over the OpenMP
153+
* runtime default. Without either, keep following omp_get_max_threads().
154+
*/
155+
if (in_parallel)
156+
openmp_nthreads = blas_omp_threads_local;
157+
else if (blas_is_num_threads_set_explicitly)
158+
openmp_nthreads = blas_cpu_number;
159+
else
160+
openmp_nthreads = omp_get_max_threads();
161+
162+
if (openmp_nthreads < 1)
163+
openmp_nthreads = 1;
164+
if (openmp_nthreads > blas_omp_number_max && blas_omp_number_max > 0) {
165+
#ifdef DEBUG
166+
fprintf(stderr,
167+
"WARNING - more OpenMP threads requested (%d) than available (%d)\n",
168+
openmp_nthreads, blas_omp_number_max);
169+
#endif
170+
openmp_nthreads = blas_omp_number_max;
152171
}
153-
154-
int openmp_nthreads;
155-
openmp_nthreads=omp_get_max_threads();
156-
if (omp_in_parallel()) openmp_nthreads = blas_omp_threads_local;
172+
if (openmp_nthreads > MAX_CPU_NUMBER)
173+
openmp_nthreads = MAX_CPU_NUMBER;
174+
175+
/*
176+
* The nested count is per-call policy. Do not make a serialized nested
177+
* call overwrite the durable global setting.
178+
*/
179+
if (in_parallel)
180+
return openmp_nthreads;
157181
#endif
158182

159183
#ifndef USE_OPENMP
@@ -164,18 +188,14 @@ int openmp_nthreads;
164188
) return 1;
165189

166190
#ifdef USE_OPENMP
167-
if (openmp_nthreads > blas_omp_number_max){
168-
#ifdef DEBUG
169-
fprintf(stderr,"WARNING - more OpenMP threads requested (%d) than available (%d)\n",openmp_nthreads,blas_omp_number_max);
170-
#endif
171-
openmp_nthreads = blas_omp_number_max;
172-
}
173-
if (blas_cpu_number != openmp_nthreads) {
174-
goto_set_num_threads(openmp_nthreads);
191+
if (!blas_is_num_threads_set_explicitly &&
192+
blas_cpu_number != openmp_nthreads) {
193+
goto_set_num_threads(openmp_nthreads);
175194
}
176-
#endif
177-
195+
return openmp_nthreads;
196+
#else
178197
return blas_cpu_number;
198+
#endif
179199

180200
}
181201

utest/CMakeLists.txt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,24 @@ add_executable(${OpenBLAS_utest_ext_bin} ${OpenBLAS_utest_ext_src})
144144
target_link_libraries(${OpenBLAS_utest_bin} ${OpenBLAS_LIBNAME})
145145
target_link_libraries(${OpenBLAS_utest_ext_bin} ${OpenBLAS_LIBNAME})
146146

147+
if (USE_OPENMP AND NOT NO_CBLAS)
148+
set(OpenBLAS_utest_openmp_bin openblas_utest_openmp)
149+
add_executable(${OpenBLAS_utest_openmp_bin} test_openmp_thread_selection.c)
150+
target_link_libraries(${OpenBLAS_utest_openmp_bin}
151+
${OpenBLAS_LIBNAME}
152+
OpenMP::OpenMP_C
153+
)
154+
set_target_properties(${OpenBLAS_utest_openmp_bin} PROPERTIES
155+
RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
156+
)
157+
add_test(${OpenBLAS_utest_openmp_bin}
158+
${CMAKE_CURRENT_BINARY_DIR}/${OpenBLAS_utest_openmp_bin}
159+
)
160+
set_tests_properties(${OpenBLAS_utest_openmp_bin} PROPERTIES
161+
ENVIRONMENT "OMP_NUM_THREADS=2"
162+
)
163+
endif()
164+
147165
if(${CMAKE_SYSTEM_NAME} MATCHES "Linux" OR ${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD" OR ${CMAKE_SYSTEM_NAME} MATCHES "QNX" )
148166
target_link_libraries(${OpenBLAS_utest_bin} m)
149167
endif()

utest/Makefile

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,19 @@ override TARGET_MACH=
77

88
UTESTBIN=openblas_utest$(EXE)
99
UTESTEXTBIN=openblas_utest_ext$(EXE)
10+
UTESTOPENMPBIN=openblas_utest_openmp$(EXE)
1011

1112
.PHONY : all
12-
.NOTPARALLEL : all run_test $(UTESTBIN) $(UTESTEXTBIN)
13+
.NOTPARALLEL : all run_test $(UTESTBIN) $(UTESTEXTBIN) $(UTESTOPENMPBIN)
1314

1415
include $(TOPDIR)/Makefile.system
1516

17+
ifeq ($(USE_OPENMP), 1)
18+
ifneq ($(NO_CBLAS), 1)
19+
OPENMP_TEST_TARGET=$(UTESTOPENMPBIN)
20+
endif
21+
endif
22+
1623
OBJS=utest_main.o test_min.o test_amax.o test_ismin.o test_rotmg.o test_axpy.o test_dotu.o test_dsdot.o test_swap.o test_rot.o test_dnrm2.o test_zscal.o \
1724
test_amin.o test_axpby.o test_gemv.o
1825
#test_rot.o test_swap.o test_axpy.o test_dotu.o test_dsdot.o test_fork.o
@@ -84,14 +91,20 @@ $(UTESTEXTBIN): $(OBJS_EXT)
8491
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB)
8592
endif
8693

87-
run_test: $(UTESTBIN) $(UTESTEXTBIN)
94+
$(UTESTOPENMPBIN): test_openmp_thread_selection.o
95+
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB)
96+
97+
run_test: $(UTESTBIN) $(UTESTEXTBIN) $(OPENMP_TEST_TARGET)
8898
ifneq ($(CROSS), 1)
8999
./$(UTESTBIN)
90100
./$(UTESTEXTBIN)
101+
ifneq ($(OPENMP_TEST_TARGET),)
102+
OMP_NUM_THREADS=2 ./$(UTESTOPENMPBIN)
103+
endif
91104
endif
92105

93106
clean:
94-
-rm -f *.o $(UTESTBIN) $(UTESTEXTBIN)
107+
-rm -f *.o $(UTESTBIN) $(UTESTEXTBIN) $(UTESTOPENMPBIN)
95108
-rm -f $(DIR_EXT)/*.o
96109

97110
libs:
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/*****************************************************************************
2+
Copyright (c) 2026, The OpenBLAS Project
3+
All rights reserved.
4+
5+
Redistribution and use in source and binary forms, with or without
6+
modification, are permitted provided that the following conditions are
7+
met:
8+
9+
1. Redistributions of source code must retain the above copyright
10+
notice, this list of conditions and the following disclaimer.
11+
12+
2. Redistributions in binary form must reproduce the above copyright
13+
notice, this list of conditions and the following disclaimer in
14+
the documentation and/or other materials provided with the
15+
distribution.
16+
3. Neither the name of the OpenBLAS project nor the names of its
17+
contributors may be used to endorse or promote products derived
18+
from this software without specific prior written permission.
19+
20+
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
21+
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
22+
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
23+
ARE DISCLAIMED. IN NO EVENT SHALL THE OPENBLAS PROJECT OR CONTRIBUTORS BE
24+
LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
25+
CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
26+
SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
27+
INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
28+
CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
29+
ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
30+
POSSIBILITY OF SUCH DAMAGE.
31+
*****************************************************************************/
32+
33+
#include <cblas.h>
34+
#include <omp.h>
35+
#include <stdio.h>
36+
#include <stdlib.h>
37+
#include <string.h>
38+
39+
static int callback_calls;
40+
static int callback_numjobs = 1;
41+
42+
static void test_threads_callback(int sync, openblas_dojob_callback dojob,
43+
int numjobs, size_t jobdata_elsize,
44+
void *jobdata, int dojob_data)
45+
{
46+
int i;
47+
48+
(void)sync;
49+
callback_calls++;
50+
callback_numjobs = numjobs;
51+
if (omp_in_parallel()) {
52+
/*
53+
* Use tasks from the existing team. DGEMM jobs exchange packed blocks,
54+
* so they must be allowed to make progress concurrently.
55+
*/
56+
for (i = 0; i < numjobs; i++) {
57+
#pragma omp task firstprivate(i, dojob, jobdata_elsize, jobdata, dojob_data)
58+
dojob(i, (char *)jobdata + (size_t)i * jobdata_elsize, dojob_data);
59+
}
60+
#pragma omp taskwait
61+
} else {
62+
#pragma omp parallel for num_threads(numjobs) schedule(static, 1) \
63+
firstprivate(dojob, jobdata_elsize, jobdata, dojob_data)
64+
for (i = 0; i < numjobs; i++)
65+
dojob(i, (char *)jobdata + (size_t)i * jobdata_elsize, dojob_data);
66+
}
67+
}
68+
69+
static int check_dgemm(const char *label, int expected_jobs, blasint n,
70+
const double *a, const double *b, double *c)
71+
{
72+
size_t i;
73+
size_t elements = (size_t)n * (size_t)n;
74+
int failed = 0;
75+
76+
memset(c, 0, elements * sizeof(*c));
77+
callback_calls = 0;
78+
callback_numjobs = 1;
79+
80+
printf("Running %s\n", label);
81+
fflush(stdout);
82+
83+
/*
84+
* TN avoids architecture-specific small-matrix kernels that would bypass
85+
* the threaded DGEMM dispatcher at this deliberately small test size.
86+
*/
87+
cblas_dgemm(CblasColMajor, CblasTrans, CblasNoTrans, n, n, n,
88+
1.0, a, n, b, n, 0.0, c, n);
89+
90+
if (callback_numjobs != expected_jobs) {
91+
fprintf(stderr, "%s: expected %d logical job(s), observed %d\n",
92+
label, expected_jobs, callback_numjobs);
93+
failed = 1;
94+
}
95+
if (expected_jobs > 1 && callback_calls == 0) {
96+
fprintf(stderr, "%s: threaded callback was not invoked\n", label);
97+
failed = 1;
98+
}
99+
100+
for (i = 0; i < elements; i++) {
101+
if (c[i] != (double)n) {
102+
fprintf(stderr,
103+
"%s: incorrect DGEMM result at element %zu: %.17g != %d\n",
104+
label, i, c[i], (int)n);
105+
failed = 1;
106+
break;
107+
}
108+
}
109+
110+
return failed;
111+
}
112+
113+
int main(void)
114+
{
115+
const double two_thread_work =
116+
2.0 * 65536.0 * (double)GEMM_MULTITHREAD_THRESHOLD;
117+
blasint n = 1;
118+
size_t elements;
119+
double *a;
120+
double *b;
121+
double *c;
122+
int failed = 0;
123+
int nested_failed = 0;
124+
int outer_team_size = 0;
125+
size_t i;
126+
127+
/*
128+
* This must be the first OpenBLAS call. The callback records the logical
129+
* job count and runs the jobs without creating a nested OpenMP team.
130+
*/
131+
openblas_set_threads_callback_function(test_threads_callback);
132+
133+
omp_set_dynamic(0);
134+
while ((double)n * (double)n * (double)n <= two_thread_work)
135+
n++;
136+
n++; /* Keep the test just beyond the threshold boundary. */
137+
138+
elements = (size_t)n * (size_t)n;
139+
a = malloc(elements * sizeof(*a));
140+
b = malloc(elements * sizeof(*b));
141+
c = malloc(elements * sizeof(*c));
142+
if (a == NULL || b == NULL || c == NULL) {
143+
fprintf(stderr, "failed to allocate DGEMM test matrices\n");
144+
free(c);
145+
free(b);
146+
free(a);
147+
return 1;
148+
}
149+
150+
for (i = 0; i < elements; i++)
151+
a[i] = b[i] = 1.0;
152+
153+
openblas_set_num_threads(1);
154+
failed |= check_dgemm("global count 1 outside OpenMP", 1, n, a, b, c);
155+
156+
openblas_set_num_threads(2);
157+
failed |= check_dgemm("global count 2 outside OpenMP", 2, n, a, b, c);
158+
159+
#pragma omp parallel num_threads(2) shared(outer_team_size, nested_failed)
160+
{
161+
#pragma omp single
162+
{
163+
outer_team_size = omp_get_num_threads();
164+
if (outer_team_size == 2) {
165+
#pragma omp task
166+
nested_failed =
167+
check_dgemm("global count 2 inside OpenMP task", 1, n, a, b, c);
168+
#pragma omp taskwait
169+
}
170+
}
171+
}
172+
173+
if (outer_team_size == 2)
174+
failed |= nested_failed;
175+
else
176+
printf("SKIP: OpenMP runtime formed an outer team of %d thread(s)\n",
177+
outer_team_size);
178+
179+
failed |= check_dgemm("global count 2 after OpenMP region", 2, n, a, b, c);
180+
181+
free(c);
182+
free(b);
183+
free(a);
184+
185+
if (failed == 0)
186+
printf("OpenMP thread selection test passed (DGEMM dimension %d)\n",
187+
(int)n);
188+
return failed;
189+
}

0 commit comments

Comments
 (0)