From 68d7ec1db8a979d1939d6a78d13067efc555f586 Mon Sep 17 00:00:00 2001 From: altman08 <936396749@qq.com> Date: Fri, 10 Jul 2026 17:27:52 +0800 Subject: [PATCH] bthread: use macros for safer thread-local access on ARM Route accesses to tls_task_group, tls_task_group_nosignal, tls_bls and several mutex-profiling thread-locals (tls_inside_lock, tls_warn_up, tls_pthread_lock_count, tls_csites, tls_ever_created_keytable) through the BAIDU_(GET|SET|GET_PTR)_VOLATILE_THREAD_LOCAL macros instead of touching the raw __thread variables directly. On aarch64/clang the compiler can incorrectly cache the address of a thread_local variable across a suspend point (e.g. when a bthread is rescheduled onto another worker), so raw accesses can silently read a stale TaskGroup*/LocalStorage. Going through the accessor macros everywhere forces a fresh, non-cached load/store on those platforms while staying a plain variable access elsewhere. Add bthread::tls_bls_ptr() as the single helper for obtaining the current LocalStorage*, and update all call sites (bthread.cpp, butex.cpp, fd.cpp, key.cpp, mutex.cpp, task_control.cpp, brpc/controller.cpp, test/bthread_unittest.cpp) to use it instead of declaring their own stale/mistyped extern references to tls_task_group or tls_bls. Also hardens borrow_keytable()/return_keytable() in key.cpp: the KeyTableList pointer obtained under the read lock is no longer reused after re-acquiring the write lock, since pool->list/pool->destroyed may change concurrently via bthread_keytable_pool_destroy(); the thread-local list is drained first before falling back to the pool's global free list. --- src/brpc/controller.cpp | 4 +- src/brpc/span.h | 4 -- src/bthread/bthread.cpp | 17 +++---- src/bthread/butex.cpp | 20 ++++---- src/bthread/fd.cpp | 8 ++-- src/bthread/key.cpp | 90 +++++++++++++++++++++--------------- src/bthread/mutex.cpp | 39 +++++++++------- src/bthread/task_control.cpp | 8 ++-- src/bthread/task_group.cpp | 16 +++---- src/bthread/task_meta.h | 4 ++ test/bthread_unittest.cpp | 9 ++-- 11 files changed, 124 insertions(+), 95 deletions(-) diff --git a/src/brpc/controller.cpp b/src/brpc/controller.cpp index bf1fecf3be..a68972a3da 100644 --- a/src/brpc/controller.cpp +++ b/src/brpc/controller.cpp @@ -51,7 +51,7 @@ #include "bthread/task_group.h" namespace bthread { -extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group; +EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); } // This is the only place that both client/server must link, so we put @@ -714,7 +714,7 @@ void Controller::OnVersionedRPCReturned(const CompletionInfo& info, response_attachment().clear(); // Retry backoff. - bthread::TaskGroup* g = bthread::tls_task_group; + bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); int64_t backoff_time_us = retry_policy->GetBackoffTimeMs(this) * 1000L; if (backoff_time_us > 0 && backoff_time_us < _deadline_us - butil::gettimeofday_us()) { diff --git a/src/brpc/span.h b/src/brpc/span.h index 70fdbf4d60..efa394b548 100644 --- a/src/brpc/span.h +++ b/src/brpc/span.h @@ -36,10 +36,6 @@ #include "brpc/options.pb.h" // ProtocolType #include "brpc/span.pb.h" -namespace bthread { -extern __thread bthread::LocalStorage tls_bls; -} - namespace brpc { class Span; diff --git a/src/bthread/bthread.cpp b/src/bthread/bthread.cpp index 5bcda94dcd..57c77890b1 100644 --- a/src/bthread/bthread.cpp +++ b/src/bthread/bthread.cpp @@ -86,7 +86,6 @@ pthread_mutex_t g_task_control_mutex = PTHREAD_MUTEX_INITIALIZER; // are not constructed before main(). TaskControl* g_task_control = NULL; -extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group; EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); extern void (*g_worker_startfn)(); extern void (*g_tagged_worker_startfn)(bthread_tag_t); @@ -263,7 +262,7 @@ static bool validate_bthread_concurrency_by_tag(const char*, int32_t val) { return bthread_setconcurrency_by_tag(val, FLAGS_bthread_current_tag) == 0; } -__thread TaskGroup* tls_task_group_nosignal = NULL; +BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group_nosignal, NULL); BUTIL_FORCE_INLINE int start_from_non_worker(bthread_t* __restrict tid, @@ -283,13 +282,14 @@ start_from_non_worker(bthread_t* __restrict tid, // 1. NOSIGNAL is often for creating many bthreads in batch, // inserting into the same TaskGroup maximizes the batch. // 2. bthread_flush() needs to know which TaskGroup to flush. - TaskGroup* g = tls_task_group_nosignal; - if (NULL == g) { + auto g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal); + if (NULL == g || g->tag() != tag) { g = c->choose_one_group(tag); - tls_task_group_nosignal = g; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, g); } return g->start_background(tid, attr, fn, arg); } + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, NULL); return c->choose_one_group(tag)->start_background(tid, attr, fn, arg); } @@ -298,7 +298,8 @@ start_from_non_worker(bthread_t* __restrict tid, // tag equal to thread local // tag equal to BTHREAD_TAG_INVALID BUTIL_FORCE_INLINE bool can_run_thread_local(const bthread_attr_t* __restrict attr) { - return attr == nullptr || attr->tag == tls_task_group->tag() || + return attr == nullptr || + attr->tag == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)->tag() || attr->tag == BTHREAD_TAG_INVALID; } @@ -360,10 +361,10 @@ void bthread_flush() { if (g) { return g->flush_nosignal_tasks(); } - g = bthread::tls_task_group_nosignal; + g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal); if (g) { // NOSIGNAL tasks were created in this non-worker. - bthread::tls_task_group_nosignal = NULL; + bthread::BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, NULL); return g->flush_nosignal_tasks_remote(); } } diff --git a/src/bthread/butex.cpp b/src/bthread/butex.cpp index 92de900cf0..d833465c57 100644 --- a/src/bthread/butex.cpp +++ b/src/bthread/butex.cpp @@ -187,7 +187,7 @@ int wait_pthread(ButexPthreadWaiter& pw, const timespec* abstime) { } } -extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group; +EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); // Returns 0 when no need to unschedule or successfully unscheduled, // -1 otherwise. @@ -280,7 +280,8 @@ void butex_destroy(void* butex) { // if TaskGroup tls_task_group is belong to tag inline bool is_same_tag(bthread_tag_t tag) { - return tls_task_group && tls_task_group->tag() == tag; + auto g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); + return g && g->tag() == tag; } // nosignal is true & tag is same can return true @@ -290,7 +291,8 @@ inline bool check_nosignal(bool nosignal, bthread_tag_t tag) { // if tag is same return tls_task_group else choose one group with tag inline TaskGroup* get_task_group(TaskControl* c, bthread_tag_t tag) { - return is_same_tag(tag) ? tls_task_group : c->choose_one_group(tag); + return is_same_tag(tag) ? BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group) + : c->choose_one_group(tag); } inline void run_in_local_task_group(TaskGroup* g, TaskMeta* next_meta, bool nosignal) { @@ -320,7 +322,7 @@ int butex_wake(void* arg, bool nosignal) { ButexBthreadWaiter* bbw = static_cast(front); unsleep_if_necessary(bbw, get_global_timer_thread()); TaskGroup* g = get_task_group(bbw->control, bbw->task_meta->attr.tag); - if (g == tls_task_group) { + if (g == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)) { run_in_local_task_group(g, bbw->task_meta, nosignal); } else { g->ready_to_run_remote(bbw->task_meta, check_nosignal(nosignal, g->tag())); @@ -384,7 +386,7 @@ int butex_wake_n(void* arg, size_t n, bool nosignal) { } } auto g = get_task_group(next->control, next->task_meta->attr.tag); - if (g == tls_task_group) { + if (g == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)) { run_in_local_task_group(g, next->task_meta, nosignal); } else { g->ready_to_run_remote(next->task_meta, check_nosignal(nosignal, g->tag())); @@ -488,7 +490,9 @@ int butex_requeue(void* arg, void* arg2) { } ButexBthreadWaiter* bbw = static_cast(front); unsleep_if_necessary(bbw, get_global_timer_thread()); - auto g = is_same_tag(bbw->task_meta->attr.tag) ? tls_task_group : NULL; + auto g = is_same_tag(bbw->task_meta->attr.tag) + ? BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group) + : NULL; if (g) { TaskGroup::exchange(&g, bbw->task_meta); } else { @@ -597,7 +601,7 @@ void wait_for_butex(void* arg) { // the two functions. The on-stack ButexBthreadWaiter is safe to use and // bw->waiter_state will not change again. // unsleep_if_necessary(bw, get_global_timer_thread()); - tls_task_group->ready_to_run(bw->task_meta); + BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)->ready_to_run(bw->task_meta); // FIXME: jump back to original thread is buggy. // // Value unmatched or waiter is already woken up by TimerThread, jump @@ -677,7 +681,7 @@ int butex_wait(void* arg, int expected_value, const timespec* abstime, bool prep butil::atomic_thread_fence(butil::memory_order_acquire); return -1; } - TaskGroup* g = tls_task_group; + TaskGroup* g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); if (NULL == g || g->is_current_pthread_task()) { return butex_wait_from_pthread(g, b, expected_value, abstime, prepend); } diff --git a/src/bthread/fd.cpp b/src/bthread/fd.cpp index 17ca63dcfe..36e9b313e4 100644 --- a/src/bthread/fd.cpp +++ b/src/bthread/fd.cpp @@ -44,7 +44,7 @@ extern int pthread_fd_wait(int fd, unsigned events, const timespec* abstime); namespace bthread { -extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group; +EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); template class LazyArray { @@ -446,7 +446,7 @@ int bthread_fd_wait(int fd, unsigned events) { errno = EINVAL; return -1; } - bthread::TaskGroup* g = bthread::tls_task_group; + bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); if (NULL != g && !g->is_current_pthread_task()) { return bthread::get_epoll_thread(fd).fd_wait( fd, events, NULL); @@ -463,7 +463,7 @@ int bthread_fd_timedwait(int fd, unsigned events, errno = EINVAL; return -1; } - bthread::TaskGroup* g = bthread::tls_task_group; + bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); if (NULL != g && !g->is_current_pthread_task()) { return bthread::get_epoll_thread(fd).fd_wait( fd, events, abstime); @@ -473,7 +473,7 @@ int bthread_fd_timedwait(int fd, unsigned events, int bthread_connect(int sockfd, const sockaddr* serv_addr, socklen_t addrlen) { - bthread::TaskGroup* g = bthread::tls_task_group; + bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); if (NULL == g || g->is_current_pthread_task()) { return ::connect(sockfd, serv_addr, addrlen); } diff --git a/src/bthread/key.cpp b/src/bthread/key.cpp index 74945833d9..dd62ce47ab 100644 --- a/src/bthread/key.cpp +++ b/src/bthread/key.cpp @@ -49,9 +49,7 @@ EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); class KeyTable; -// defined in task_group.cpp -extern __thread LocalStorage tls_bls; -static __thread bool tls_ever_created_keytable = false; +BAIDU_VOLATILE_THREAD_LOCAL(bool, tls_ever_created_keytable, false); // We keep thread specific data in a two-level array. The top-level array // contains at most KEY_1STLEVEL_SIZE pointers to dynamically allocated @@ -227,12 +225,12 @@ class BAIDU_CACHELINE_ALIGNMENT KeyTableList { ~KeyTableList() { TaskGroup* g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); - KeyTable* old_kt = tls_bls.keytable; + KeyTable* old_kt = tls_bls_ptr()->keytable; KeyTable* keytable = _head; while (keytable) { KeyTable* kt = keytable; keytable = kt->next; - tls_bls.keytable = kt; + tls_bls_ptr()->keytable = kt; if (g) { g->current_task()->local_storage.keytable = kt; } @@ -242,7 +240,7 @@ class BAIDU_CACHELINE_ALIGNMENT KeyTableList { } g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); } - tls_bls.keytable = old_kt; + tls_bls_ptr()->keytable = old_kt; if (g) { g->current_task()->local_storage.keytable = old_kt; } @@ -329,20 +327,33 @@ class BAIDU_CACHELINE_ALIGNMENT KeyTableList { KeyTable* borrow_keytable(bthread_keytable_pool_t* pool) { if (pool != NULL && (pool->list || pool->free_keytables)) { KeyTable* p; - pthread_rwlock_rdlock(&pool->rwlock); - auto list = (butil::ThreadLocal*)pool->list; - if (list) { - p = list->get()->remove_front(); - if (p) { - pthread_rwlock_unlock(&pool->rwlock); - return p; + { + pthread_rwlock_rdlock(&pool->rwlock); + auto list = (butil::ThreadLocal*)pool->list; + if (list) { + p = list->get()->remove_front(); + if (p) { + pthread_rwlock_unlock(&pool->rwlock); + return p; + } } + pthread_rwlock_unlock(&pool->rwlock); } - pthread_rwlock_unlock(&pool->rwlock); if (pool->free_keytables) { pthread_rwlock_wrlock(&pool->rwlock); + if (pool->destroyed) { + pthread_rwlock_unlock(&pool->rwlock); + return NULL; + } + auto list = (butil::ThreadLocal*)pool->list; p = (KeyTable*)pool->free_keytables; if (list) { + p = list->get()->remove_front(); + if (p) { + pthread_rwlock_unlock(&pool->rwlock); + return p; + } + p = (KeyTable*)pool->free_keytables; for (uint32_t i = 0; i < FLAGS_borrow_from_globle_size; ++i) { if (p) { pool->free_keytables = p->next; @@ -357,6 +368,7 @@ KeyTable* borrow_keytable(bthread_keytable_pool_t* pool) { pthread_rwlock_unlock(&pool->rwlock); return result; } else { + p = (KeyTable*)pool->free_keytables; if (p) { pool->free_keytables = p->next; pthread_rwlock_unlock(&pool->rwlock); @@ -379,25 +391,31 @@ void return_keytable(bthread_keytable_pool_t* pool, KeyTable* kt) { delete kt; return; } - pthread_rwlock_rdlock(&pool->rwlock); - if (pool->destroyed) { + bool need_move = false; + { + pthread_rwlock_rdlock(&pool->rwlock); + if (pool->destroyed) { + pthread_rwlock_unlock(&pool->rwlock); + delete kt; + return; + } + auto list = (butil::ThreadLocal*)pool->list; + list->get()->append(kt); + need_move = list->get()->get_length() > FLAGS_key_table_list_size; pthread_rwlock_unlock(&pool->rwlock); - delete kt; - return; } - auto list = (butil::ThreadLocal*)pool->list; - list->get()->append(kt); - if (list->get()->get_length() > FLAGS_key_table_list_size) { - pthread_rwlock_unlock(&pool->rwlock); + if (need_move) { pthread_rwlock_wrlock(&pool->rwlock); - if (!pool->destroyed) { + auto list = (butil::ThreadLocal*)pool->list; + if (!pool->destroyed && list != NULL && + list->get()->get_length() > FLAGS_key_table_list_size) { int out = list->get()->move_first_n_to_target( (KeyTable**)(&pool->free_keytables), FLAGS_key_table_list_size / 2); pool->size += out; } + pthread_rwlock_unlock(&pool->rwlock); } - pthread_rwlock_unlock(&pool->rwlock); } static void cleanup_pthread(void* arg) { @@ -405,7 +423,7 @@ static void cleanup_pthread(void* arg) { if (kt) { delete kt; // After deletion: tls may be set during deletion. - tls_bls.keytable = NULL; + tls_bls_ptr()->keytable = NULL; } } @@ -469,18 +487,18 @@ int bthread_keytable_pool_destroy(bthread_keytable_pool_t* pool) { // Cheat get/setspecific and destroy the keytables. bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); - bthread::KeyTable* old_kt = bthread::tls_bls.keytable; + bthread::KeyTable* old_kt = bthread::tls_bls_ptr()->keytable; while (saved_free_keytables) { bthread::KeyTable* kt = saved_free_keytables; saved_free_keytables = kt->next; - bthread::tls_bls.keytable = kt; + bthread::tls_bls_ptr()->keytable = kt; if (g) { g->current_task()->local_storage.keytable = kt; } delete kt; g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); } - bthread::tls_bls.keytable = old_kt; + bthread::tls_bls_ptr()->keytable = old_kt; if (g) { g->current_task()->local_storage.keytable = old_kt; } @@ -624,13 +642,13 @@ int bthread_key_delete(bthread_key_t key) { // -> bthread_setspecific succeeds to borrow_keytable and overwrites old data // at the position with newly created data, the old data is leaked. int bthread_setspecific(bthread_key_t key, void* data) { - bthread::KeyTable* kt = bthread::tls_bls.keytable; + bthread::KeyTable* kt = bthread::tls_bls_ptr()->keytable; if (NULL == kt) { kt = new (std::nothrow) bthread::KeyTable; if (NULL == kt) { return ENOMEM; } - bthread::tls_bls.keytable = kt; + bthread::tls_bls_ptr()->keytable = kt; bthread::TaskGroup* const g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); if (g) { g->current_task()->local_storage.keytable = kt; @@ -638,8 +656,8 @@ int bthread_setspecific(bthread_key_t key, void* data) { // Only cleanup keytable created by pthread. // keytable created by bthread will be deleted // in `return_keytable' or `bthread_keytable_pool_destroy'. - if (!bthread::tls_ever_created_keytable) { - bthread::tls_ever_created_keytable = true; + if (!bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_ever_created_keytable)) { + bthread::BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_ever_created_keytable, true); CHECK_EQ(0, butil::thread_atexit(bthread::cleanup_pthread, kt)); } } @@ -648,7 +666,7 @@ int bthread_setspecific(bthread_key_t key, void* data) { } void* bthread_getspecific(bthread_key_t key) { - bthread::KeyTable* kt = bthread::tls_bls.keytable; + bthread::KeyTable* kt = bthread::tls_bls_ptr()->keytable; if (kt) { return kt->get_data(key); } @@ -658,7 +676,7 @@ void* bthread_getspecific(bthread_key_t key) { kt = bthread::borrow_keytable(task->attr.keytable_pool); if (kt) { g->current_task()->local_storage.keytable = kt; - bthread::tls_bls.keytable = kt; + bthread::tls_bls_ptr()->keytable = kt; return kt->get_data(key); } } @@ -666,11 +684,11 @@ void* bthread_getspecific(bthread_key_t key) { } void bthread_assign_data(void* data) { - bthread::tls_bls.assigned_data = data; + bthread::tls_bls_ptr()->assigned_data = data; } void* bthread_get_assigned_data() { - return bthread::tls_bls.assigned_data; + return bthread::tls_bls_ptr()->assigned_data; } } // extern "C" diff --git a/src/bthread/mutex.cpp b/src/bthread/mutex.cpp index dff2da3ff4..a5d2cbd1c2 100644 --- a/src/bthread/mutex.cpp +++ b/src/bthread/mutex.cpp @@ -524,11 +524,11 @@ inline uint64_t hash_mutex_ptr(const Mutex* m) { // Mark being inside locking so that pthread_mutex calls inside collecting // code are never sampled, otherwise deadlock may occur. -static __thread bool tls_inside_lock = false; +BAIDU_VOLATILE_THREAD_LOCAL(bool, tls_inside_lock, false); // Warn up some singleton objects used in contention profiler // to avoid deadlock in malloc call stack. -static __thread bool tls_warn_up = false; +BAIDU_VOLATILE_THREAD_LOCAL(bool, tls_warn_up, false); #if BRPC_DEBUG_BTHREAD_SCHE_SAFETY // ++tls_pthread_lock_count when pthread locking, @@ -536,19 +536,22 @@ static __thread bool tls_warn_up = false; // 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; +BAIDU_VOLATILE_THREAD_LOCAL(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 +#define ADD_TLS_PTHREAD_LOCK_COUNT \ + ++(*BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_pthread_lock_count)) +#define SUB_TLS_PTHREAD_LOCK_COUNT \ + --(*BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_pthread_lock_count)) void CheckBthreadScheSafety() { - if (BAIDU_LIKELY(0 == tls_pthread_lock_count)) { + if (BAIDU_LIKELY(0 == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_pthread_lock_count))) { return; } // It can only be checked once because the counter is messed up. LOG_BACKTRACE_ONCE(ERROR) << "bthread is suspended while holding " - << tls_pthread_lock_count << " pthread locks."; + << BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_pthread_lock_count) + << " pthread locks."; } #else #define ADD_TLS_PTHREAD_LOCK_COUNT ((void)0) @@ -576,7 +579,8 @@ struct TLSPthreadContentionSites { uint64_t cp_version; MutexAndContentionSite list[TLS_MAX_COUNT]; }; -static __thread TLSPthreadContentionSites tls_csites = {0,0,{}}; +BAIDU_VOLATILE_THREAD_LOCAL(TLSPthreadContentionSites, tls_csites, + TLSPthreadContentionSites()); #endif // DONT_SPEEDUP_PTHREAD_CONTENTION_PROFILER_WITH_TLS // Guaranteed in linux/win. @@ -625,9 +629,9 @@ inline bool remove_pthread_contention_site(const Mutex* mutex, // Submit the contention along with the callsite('s stacktrace) void submit_contention(const bthread_contention_site_t& csite, int64_t now_ns) { - tls_inside_lock = true; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_inside_lock, true); BRPC_SCOPE_EXIT { - tls_inside_lock = false; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_inside_lock, false); }; butil::debug::StackTrace stack(true); // May lock. @@ -639,7 +643,8 @@ void submit_contention(const bthread_contention_site_t& csite, int64_t now_ns) { // 1. Warn up some singleton objects used in `submit_contention' // to avoid deadlock in malloc call stack. // 2. LocalPool is empty, GlobalPool may allocate memory by malloc. - if (!tls_warn_up || butil::local_pool_free_empty()) { + if (!BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_warn_up) || + butil::local_pool_free_empty()) { // In malloc call stack, can not submit contention. if (stack.FindSymbol((void*)malloc)) { return; @@ -656,7 +661,7 @@ void submit_contention(const bthread_contention_site_t& csite, int64_t now_ns) { sc->nframes = stack.CopyAddressTo(sc->stack, arraysize(sc->stack)); sc->submit(now_ns / 1000); // may lock // Once submit a contention, complete warn up. - tls_warn_up = true; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_warn_up, true); } #if BRPC_DEBUG_LOCK @@ -872,7 +877,7 @@ BUTIL_FORCE_INLINE int pthread_mutex_lock_impl(Mutex* mutex, const struct timesp if (!g_cp || // collecting code including backtrace() and submit() may call // pthread_mutex_lock and cause deadlock. Don't sample. - tls_inside_lock) { + BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_inside_lock)) { return pthread_mutex_lock_internal(mutex, abstime); } // Don't slow down non-contended locks. @@ -885,7 +890,8 @@ BUTIL_FORCE_INLINE int pthread_mutex_lock_impl(Mutex* mutex, const struct timesp bthread_contention_site_t* csite = NULL; #ifndef DONT_SPEEDUP_PTHREAD_CONTENTION_PROFILER_WITH_TLS - TLSPthreadContentionSites& fast_alt = tls_csites; + TLSPthreadContentionSites& fast_alt = + *BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_csites); if (fast_alt.cp_version != g_cp_version) { fast_alt.cp_version = g_cp_version; fast_alt.count = 0; @@ -927,7 +933,7 @@ BUTIL_FORCE_INLINE int pthread_mutex_trylock_impl(Mutex* mutex) { template BUTIL_FORCE_INLINE int pthread_mutex_unlock_impl(Mutex* mutex) { // Don't change behavior of unlock when profiler is off. - if (!g_cp || tls_inside_lock) { + if (!g_cp || BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_inside_lock)) { // This branch brings an issue that an entry created by // add_pthread_contention_site may not be cleared. Thus we add a // 16-bit rolling version in the entry to find out such entry. @@ -937,7 +943,8 @@ BUTIL_FORCE_INLINE int pthread_mutex_unlock_impl(Mutex* mutex) { bool miss_in_tls = true; bthread_contention_site_t saved_csite = {0,0}; #ifndef DONT_SPEEDUP_PTHREAD_CONTENTION_PROFILER_WITH_TLS - TLSPthreadContentionSites& fast_alt = tls_csites; + TLSPthreadContentionSites& fast_alt = + *BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_csites); for (int i = fast_alt.count - 1; i >= 0; --i) { if (fast_alt.list[i].mutex == mutex) { if (is_contention_site_valid(fast_alt.list[i].csite)) { diff --git a/src/bthread/task_control.cpp b/src/bthread/task_control.cpp index beb413060d..3ad3aa508d 100644 --- a/src/bthread/task_control.cpp +++ b/src/bthread/task_control.cpp @@ -70,7 +70,7 @@ DECLARE_int32(bthread_min_concurrency); DECLARE_int32(bthread_parking_lot_of_each_tag); extern pthread_mutex_t g_task_control_mutex; -extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group; +EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group); void (*g_worker_startfn)() = NULL; void (*g_tagged_worker_startfn)(bthread_tag_t) = NULL; @@ -128,7 +128,7 @@ void* TaskControl::worker_thread(void* arg) { } BT_VLOG << "Created worker=" << pthread_self() << " tid=" << g->_tid << " bthread=" << g->main_tid() << " tag=" << g->tag(); - tls_task_group = g; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group, g); c->_nworkers << 1; c->tag_nworkers(g->tag()) << 1; @@ -138,7 +138,7 @@ void* TaskControl::worker_thread(void* arg) { BT_VLOG << "Destroying worker=" << pthread_self() << " bthread=" << g->main_tid() << " idle=" << stat.cputime_ns / 1000000.0 << "ms uptime=" << g->current_uptime_ns() / 1000000.0 << "ms"; - tls_task_group = NULL; + BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group, NULL); g->destroy_self(); c->_nworkers << -1; c->tag_nworkers(g->tag()) << -1; @@ -619,7 +619,7 @@ int TaskControl::_destroy_group(TaskGroup* g) { } bool TaskControl::steal_task(bthread_t* tid, size_t* seed, size_t offset) { - auto tag = tls_task_group->tag(); + auto tag = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)->tag(); if (_enable_priority_queue) { for (int i = 0; diff --git a/src/bthread/task_group.cpp b/src/bthread/task_group.cpp index 46f9a13377..5bb95134af 100644 --- a/src/bthread/task_group.cpp +++ b/src/bthread/task_group.cpp @@ -484,23 +484,23 @@ void TaskGroup::task_runner(intptr_t skip_remained) { // which allocates bthread-local stream arrays via bthread_setspecific). // If span cleanup ran after keytable cleanup, such allocations would // re-populate the keytable and never be reclaimed, causing memory leak. - LocalStorage* tls_bls_ptr = BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_bls); - if (tls_bls_ptr->rpcz_parent_span && g_rpcz_parent_span_dtor) { - g_rpcz_parent_span_dtor(tls_bls_ptr->rpcz_parent_span); - tls_bls_ptr = BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_bls); - tls_bls_ptr->rpcz_parent_span = NULL; + LocalStorage* bls_ptr = tls_bls_ptr(); + if (bls_ptr->rpcz_parent_span && g_rpcz_parent_span_dtor) { + g_rpcz_parent_span_dtor(bls_ptr->rpcz_parent_span); + bls_ptr = tls_bls_ptr(); + bls_ptr->rpcz_parent_span = NULL; m->local_storage.rpcz_parent_span = NULL; } // Clean tls variables, must be done before changing version_butex // otherwise another thread just joined this thread may not see side // effects of destructing tls variables. - KeyTable* kt = tls_bls_ptr->keytable; + KeyTable* kt = bls_ptr->keytable; if (kt != NULL) { return_keytable(m->attr.keytable_pool, kt); // After deletion: tls may be set during deletion. - tls_bls_ptr = BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_bls); - tls_bls_ptr->keytable = NULL; + bls_ptr = tls_bls_ptr(); + bls_ptr->keytable = NULL; m->local_storage.keytable = NULL; // optional } diff --git a/src/bthread/task_meta.h b/src/bthread/task_meta.h index 074af430df..7c9e63790e 100644 --- a/src/bthread/task_meta.h +++ b/src/bthread/task_meta.h @@ -53,6 +53,10 @@ const static LocalStorage LOCAL_STORAGE_INIT = BTHREAD_LOCAL_STORAGE_INITIALIZER EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(LocalStorage, tls_bls); +inline LocalStorage* tls_bls_ptr() { + return BAIDU_GET_PTR_VOLATILE_THREAD_LOCAL(tls_bls); +} + enum TaskStatus { TASK_STATUS_UNKNOWN, TASK_STATUS_CREATED, diff --git a/test/bthread_unittest.cpp b/test/bthread_unittest.cpp index bd31a3c430..b6d5ca7f6e 100644 --- a/test/bthread_unittest.cpp +++ b/test/bthread_unittest.cpp @@ -34,7 +34,6 @@ int main(int argc, char* argv[]) { } namespace bthread { -extern __thread bthread::LocalStorage tls_bls; #ifdef BRPC_BTHREAD_TRACER extern std::string stack_trace(bthread_t tid); #endif // BRPC_BTHREAD_TRACER @@ -536,21 +535,21 @@ static const bthread_attr_t BTHREAD_ATTR_NORMAL_WITH_SPAN = void* test_parent_span(void* p) { uint64_t *q = (uint64_t *)p; - *q = (uint64_t)(bthread::tls_bls.rpcz_parent_span); + *q = (uint64_t)(bthread::tls_bls_ptr()->rpcz_parent_span); LOG(INFO) << "span id in thread is " << *q; return NULL; } void* test_grandson_parent_span(void* p) { uint64_t* q = (uint64_t*)p; - *q = (uint64_t)(bthread::tls_bls.rpcz_parent_span); + *q = (uint64_t)(bthread::tls_bls_ptr()->rpcz_parent_span); LOG(INFO) << "parent span id in thread is " << *q; return NULL; } void* test_son_parent_span(void* p) { uint64_t* q = (uint64_t*)p; - *q = (uint64_t)(bthread::tls_bls.rpcz_parent_span); + *q = (uint64_t)(bthread::tls_bls_ptr()->rpcz_parent_span); LOG(INFO) << "parent span id in thread is " << *q; bthread_t th; uint64_t multi_p; @@ -581,7 +580,7 @@ TEST_F(BthreadTest, test_span) { uint64_t target = 0xBADBEAFUL; LOG(INFO) << "target span id is " << target; - bthread::tls_bls.rpcz_parent_span = (void*)target; + bthread::tls_bls_ptr()->rpcz_parent_span = (void*)target; bthread_t th1; ASSERT_EQ(0, bthread_start_urgent(&th1, &BTHREAD_ATTR_NORMAL_WITH_SPAN, test_parent_span, &p1)); ASSERT_EQ(0, bthread_join(th1, NULL));