Skip to content

Commit

Permalink
Disable bthread sche safety debug by default
Browse files Browse the repository at this point in the history
  • Loading branch information
chenBright committed Oct 8, 2024
1 parent 53d4192 commit f3b23c4
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 17 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/ci-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
- uses: ./.github/actions/install-all-dependences
- uses: ./.github/actions/init-make-config
with:
options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma
options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety
- name: compile
run: |
make -j ${{env.proc_num}}
Expand All @@ -76,7 +76,7 @@ jobs:
export CC=gcc && export CXX=g++
mkdir build
cd build
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON ..
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON ..
- name: compile
run: |
cd build
Expand All @@ -86,7 +86,7 @@ jobs:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
- run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true -- //... -//example/...
- run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true -- //... -//example/...

clang-compile-with-make:
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -135,7 +135,7 @@ jobs:
- uses: ./.github/actions/install-all-dependences
- uses: ./.github/actions/init-make-config
with:
options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma
options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety
- name: compile
run: |
make -j ${{env.proc_num}}
Expand All @@ -150,7 +150,7 @@ jobs:
export CC=clang && export CXX=clang++
mkdir build
cd build
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON ..
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON ..
- name: compile
run: |
cd build
Expand All @@ -160,7 +160,7 @@ jobs:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
- run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true -- //... -//example/...
- run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true -- //... -//example/...

clang-unittest:
runs-on: ubuntu-20.04
Expand Down
3 changes: 3 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ COPTS = [
}) + select({
"//bazel/config:brpc_with_rdma": ["-DBRPC_WITH_RDMA=1"],
"//conditions:default": [""],
}) + select({
"//bazel/config:brpc_with_debug_bthread_sche_safety": ["-DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=1"],
"//conditions:default": ["-DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=0"],
})

LINKOPTS = [
Expand Down
8 changes: 7 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ option(WITH_DEBUG_SYMBOLS "With debug symbols" ON)
option(WITH_THRIFT "With thrift framed protocol supported" OFF)
option(WITH_SNAPPY "With snappy" OFF)
option(WITH_RDMA "With RDMA" OFF)
option(WITH_DEBUG_BTHREAD_SCHE_SAFETY "With debugging bthread sche safety" OFF)
option(BUILD_UNIT_TESTS "Whether to build unit tests" OFF)
option(BUILD_FUZZ_TESTS "Whether to build fuzz tests" OFF)
option(BUILD_BRPC_TOOLS "Whether to build brpc tools" ON)
Expand Down Expand Up @@ -79,6 +80,11 @@ if(WITH_RDMA)
set(WITH_RDMA_VAL "1")
endif()

set(WITH_DEBUG_BTHREAD_SCHE_SAFETY_VAL "0")
if(WITH_DEBUG_BTHREAD_SCHE_SAFETY)
set(WITH_DEBUG_BTHREAD_SCHE_SAFETY_VAL "1")
endif()

include(GNUInstallDirs)

configure_file(${PROJECT_SOURCE_DIR}/config.h.in ${PROJECT_SOURCE_DIR}/src/butil/config.h @ONLY)
Expand Down Expand Up @@ -117,7 +123,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -Wno-deprecated-declarations -Wno-inconsistent-missing-override")
endif()

set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} ${DEFINE_CLOCK_GETTIME} -DBRPC_WITH_GLOG=${WITH_GLOG_VAL} -DBRPC_WITH_RDMA=${WITH_RDMA_VAL} -DGFLAGS_NS=${GFLAGS_NS}")
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} ${DEFINE_CLOCK_GETTIME} -DBRPC_WITH_GLOG=${WITH_GLOG_VAL} -DBRPC_WITH_RDMA=${WITH_RDMA_VAL} -DGFLAGS_NS=${GFLAGS_NS} -DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=${WITH_DEBUG_BTHREAD_SCHE_SAFETY_VAL}")
if(WITH_MESALINK)
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -DUSE_MESALINK")
endif()
Expand Down
6 changes: 6 additions & 0 deletions bazel/config/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,10 @@ config_setting(
name = "brpc_with_boringssl",
define_values = {"BRPC_WITH_BORINGSSL": "true"},
visibility = ["//visibility:public"],
)

config_setting(
name = "brpc_with_debug_bthread_sche_safety",
define_values = {"with_debug_bthread_sche_safety": "true"},
visibility = ["//visibility:public"],
)
6 changes: 4 additions & 2 deletions config_brpc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ else
LDD=ldd
fi

TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,nodebugsymbols -n 'config_brpc' -- "$@"`
TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,with-debug-bthread-sche-safety,nodebugsymbols -n 'config_brpc' -- "$@"`
WITH_GLOG=0
WITH_THRIFT=0
WITH_RDMA=0
WITH_MESALINK=0
BRPC_DEBUG_BTHREAD_SCHE_SAFETY=0
DEBUGSYMBOLS=-g

if [ $? != 0 ] ; then >&2 $ECHO "Terminating..."; exit 1 ; fi
Expand All @@ -67,6 +68,7 @@ while true; do
--with-thrift) WITH_THRIFT=1; shift 1 ;;
--with-rdma) WITH_RDMA=1; shift 1 ;;
--with-mesalink) WITH_MESALINK=1; shift 1 ;;
--with-debug-bthread-sche-safety ) BRPC_DEBUG_BTHREAD_SCHE_SAFETY=1; shift 1 ;;
--nodebugsymbols ) DEBUGSYMBOLS=; shift 1 ;;
-- ) shift; break ;;
* ) break ;;
Expand Down Expand Up @@ -407,7 +409,7 @@ append_to_output "STATIC_LINKINGS=$STATIC_LINKINGS"
append_to_output "DYNAMIC_LINKINGS=$DYNAMIC_LINKINGS"

# CPP means C PreProcessing, not C PlusPlus
CPPFLAGS="-DBRPC_WITH_GLOG=$WITH_GLOG -DGFLAGS_NS=$GFLAGS_NS"
CPPFLAGS="-DBRPC_WITH_GLOG=$WITH_GLOG -DGFLAGS_NS=$GFLAGS_NS -DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=$BRPC_DEBUG_BTHREAD_SCHE_SAFETY"

# Avoid over-optimizations of TLS variables by GCC>=4.8
# See: https://github.com/apache/brpc/issues/1693
Expand Down
27 changes: 19 additions & 8 deletions src/bthread/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,11 +505,17 @@ static __thread bool tls_inside_lock = false;
// to avoid deadlock in malloc call stack.
static __thread bool tls_warn_up = false;

#if BRPC_DEBUG_BTHREAD_SCHE_SAFETY
// ++tls_pthread_lock_count when pthread locking,
// --tls_pthread_lock_count when pthread unlocking.
// Only when it is equal to 0, it is safe for the bthread to be scheduled.
// Note: If a mutex is locked/unlocked in different thread,
// `tls_pthread_lock_count' is inaccurate, so this feature cannot be used.
static __thread int tls_pthread_lock_count = 0;

#define ADD_TLS_PTHREAD_LOCK_COUNT ++tls_pthread_lock_count
#define SUB_TLS_PTHREAD_LOCK_COUNT --tls_pthread_lock_count

void CheckBthreadScheSafety() {
if (BAIDU_LIKELY(0 == tls_pthread_lock_count)) {
return;
Expand All @@ -525,6 +531,11 @@ void CheckBthreadScheSafety() {
<< std::endl << trace.ToString();
}
}
#else
#define ADD_TLS_PTHREAD_LOCK_COUNT ((void)0)
#define SUB_TLS_PTHREAD_LOCK_COUNT ((void)0)
void CheckBthreadScheSafety() {}
#endif // BRPC_DEBUG_BTHREAD_SCHE_SAFETY

// Speed up with TLS:
// Most pthread_mutex are locked and unlocked in the same thread. Putting
Expand Down Expand Up @@ -638,7 +649,7 @@ BUTIL_FORCE_INLINE int pthread_mutex_lock_internal(pthread_mutex_t* mutex,
sys_pthread_mutex_lock(mutex) :
sys_pthread_mutex_timedlock(mutex, abstime);
if (0 == rc) {
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}
return rc;
}
Expand All @@ -647,7 +658,7 @@ BUTIL_FORCE_INLINE int pthread_mutex_lock_internal(pthread_mutex_t* mutex,
const struct timespec*/* Not supported */) {
int rc = sys_pthread_mutex_lock(mutex);
if (0 == rc) {
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}
return rc;
}
Expand All @@ -656,13 +667,13 @@ BUTIL_FORCE_INLINE int pthread_mutex_lock_internal(pthread_mutex_t* mutex,
BUTIL_FORCE_INLINE int pthread_mutex_trylock_internal(pthread_mutex_t* mutex) {
int rc = sys_pthread_mutex_trylock(mutex);
if (0 == rc) {
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}
return rc;
}

BUTIL_FORCE_INLINE int pthread_mutex_unlock_internal(pthread_mutex_t* mutex) {
--tls_pthread_lock_count;
SUB_TLS_PTHREAD_LOCK_COUNT;
return sys_pthread_mutex_unlock(mutex);
}
#endif // NO_PTHREAD_MUTEX_HOOK
Expand Down Expand Up @@ -903,14 +914,14 @@ void FastPthreadMutex::lock() {
}

(void)lock_contended(NULL);
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}

bool FastPthreadMutex::try_lock() {
auto split = (bthread::MutexInternal*)&_futex;
bool lock = !split->locked.exchange(1, butil::memory_order_acquire);
if (lock) {
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}
return lock;
}
Expand All @@ -921,13 +932,13 @@ bool FastPthreadMutex::timed_lock(const struct timespec* abstime) {
}
int rc = lock_contended(abstime);
if (rc == 0) {
++tls_pthread_lock_count;
ADD_TLS_PTHREAD_LOCK_COUNT;
}
return rc == 0;
}

void FastPthreadMutex::unlock() {
--tls_pthread_lock_count;
SUB_TLS_PTHREAD_LOCK_COUNT;
auto whole = (butil::atomic<unsigned>*)&_futex;
const unsigned prev = whole->exchange(0, butil::memory_order_release);
// CAUTION: the mutex may be destroyed, check comments before butex_create
Expand Down

0 comments on commit f3b23c4

Please sign in to comment.