From 7750d507ff7e34ce354ca66954be447b50d313b2 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 29 Nov 2025 22:55:25 +0100 Subject: [PATCH 1/2] chore: add test case for exec_blas_async after fork most functions are using exec_blas but dgetrf uses exec_blas_async directly and the behavior after fork is not the same. It's currently deadlocking. --- utest/CMakeLists.txt | 1 + utest/Makefile | 2 +- utest/test_post_fork_async.c | 94 ++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 utest/test_post_fork_async.c diff --git a/utest/CMakeLists.txt b/utest/CMakeLists.txt index 6a61899da5..c73152d79a 100644 --- a/utest/CMakeLists.txt +++ b/utest/CMakeLists.txt @@ -101,6 +101,7 @@ if (NOT USE_OPENMP) set(OpenBLAS_utest_src ${OpenBLAS_utest_src} test_fork.c + test_post_fork_async.c ) endif() set(OpenBLAS_utest_src diff --git a/utest/Makefile b/utest/Makefile index b82937093f..e92fb67dc1 100644 --- a/utest/Makefile +++ b/utest/Makefile @@ -45,7 +45,7 @@ endif # FIXME TBD if this works on OSX, SunOS, POWER and zarch ifeq ($(OSNAME), $(filter $(OSNAME),Linux CYGWIN_NT)) ifneq ($(USE_OPENMP), 1) -OBJS += test_fork.o +OBJS += test_fork.o test_post_fork_async.o endif OBJS += test_post_fork.o endif diff --git a/utest/test_post_fork_async.c b/utest/test_post_fork_async.c new file mode 100644 index 0000000000..c283a6455f --- /dev/null +++ b/utest/test_post_fork_async.c @@ -0,0 +1,94 @@ +/***************************************************************************** +Copyright (c) 2011-2025, The OpenBLAS Project +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + 1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + 3. Neither the name of the OpenBLAS project nor the names of + its contributors may be used to endorse or promote products + derived from this software without specific prior written + permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +**********************************************************************************/ + +#include +#include +#include +#include +#include "openblas_utest.h" + +static void* xmalloc(size_t n) +{ + void* tmp; + tmp = calloc(1, n); + if (tmp == NULL) { + fprintf(stderr, "Failed to allocate memory for the testcase.\n"); + exit(1); + } else { + return tmp; + } +} + +CTEST(fork, safety_after_fork_async) +{ +#ifdef __UCLIBC__ +#if !defined __UCLIBC_HAS_STUBS__ && !defined __ARCH_USE_MMU__ +exit(0); +#endif +#endif +#ifndef BUILD_DOUBLE +exit(0); +#else + blasint n = 200; + blasint info; + blasint i; + + blasint *ipiv; + double *arr; + + pid_t fork_pid; + + arr = xmalloc(sizeof(*arr) * n * n); + ipiv = xmalloc(sizeof(*ipiv) * n); + + // array is an identity matrix + for (i = 0; i < n*n; i += n + 1) { + arr[i] = 1.0; + } + + fork_pid = fork(); + if (fork_pid == -1) { + perror("fork"); + CTEST_ERR("Failed to fork process."); + } else if (fork_pid == 0) { + exit(0); + } else { + // Wait for the child to finish and check the exit code. + int child_status = 0; + pid_t wait_pid = wait(&child_status); + ASSERT_EQUAL(wait_pid, fork_pid); + ASSERT_EQUAL(0, WEXITSTATUS (child_status)); + } + BLASFUNC(dgetrf)(&n, &n, arr, &n, ipiv, &info); +#endif +} From 396137ff27793f7bce2b0155451f14fecf1f7cd6 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 29 Nov 2025 23:00:17 +0100 Subject: [PATCH 2/2] revert locks introduced in #5170 --- driver/others/blas_server.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/driver/others/blas_server.c b/driver/others/blas_server.c index 4a31823543..01a6e91ee7 100644 --- a/driver/others/blas_server.c +++ b/driver/others/blas_server.c @@ -637,9 +637,7 @@ int exec_blas_async(BLASLONG pos, blas_queue_t *queue){ #ifdef SMP_SERVER // Handle lazy re-init of the thread-pool after a POSIX fork - LOCK_COMMAND(&server_lock); if (unlikely(blas_server_avail == 0)) blas_thread_init(); - UNLOCK_COMMAND(&server_lock); #endif BLASLONG i = 0; blas_queue_t *current = queue;