Improve thread private data destructor to be more robust

These changes make the at-exit processing of destructors
less sensitive to the order in which destructors are
called.

Without this change, if the destructor for the
list of thread private data pointers is called first,
then the thread private data destructor will segfault.

Change-Id: I4767de682eacf32bba874555532d043415bbea61
Signed-off-by: davidoli <david.olien@intel.com>
This commit is contained in:
davidoli
2020-10-19 10:20:07 -07:00
committed by sys_ocldev
parent ac3b65ba82
commit 2fe425dc22
3 changed files with 90 additions and 109 deletions

View File

@@ -38,6 +38,17 @@ ze_result_t createAPITracer(zet_context_handle_t hContext, const zet_tracer_exp_
return ZE_RESULT_SUCCESS;
}
// This destructor will be called only during at-exit processing,
// Hence, this function is executing in a single threaded environment,
// and requires no mutex.
APITracerContextImp::~APITracerContextImp() {
std::list<ThreadPrivateTracerData *>::iterator itr = threadTracerDataList.begin();
while (itr != threadTracerDataList.end()) {
(*itr)->clearThreadTracerDataOnList();
itr = threadTracerDataList.erase(itr);
}
}
ze_result_t APITracerImp::destroyTracer(zet_tracer_exp_handle_t phTracer) {
APITracerImp *tracer = static_cast<APITracerImp *>(phTracer);
@@ -75,61 +86,50 @@ ze_result_t APITracerImp::enableTracer(ze_bool_t enable) {
return pGlobalAPITracerContextImp->enableTracingImp(this, enable);
}
static std::mutex perThreadTracerDataMutex;
static std::list<per_thread_tracer_data_t *> perThreadTracerDataList;
void ThreadPrivateTracerData::allocatePerThreadPublicTracerData() {
if (myThreadPublicTracerData == nullptr) {
myThreadPublicTracerData = new per_thread_tracer_data_t;
myThreadPublicTracerData->tracerArrayPointer.store(NULL, std::memory_order_relaxed);
myThreadPublicTracerData->thread_id = std::this_thread::get_id();
std::lock_guard<std::mutex> lock(perThreadTracerDataMutex);
perThreadTracerDataList.push_back(myThreadPublicTracerData);
}
void APITracerContextImp::addThreadTracerDataToList(ThreadPrivateTracerData *threadDataP) {
std::lock_guard<std::mutex> lock(threadTracerDataListMutex);
threadTracerDataList.push_back(threadDataP);
}
void ThreadPrivateTracerData::freePerThreadPublicTracerData() {
//
// There is no need to hold a mutex when testing
// my_thread_tracer_data is a thread_local object.
// my_threadd_tracer_data for nullptr since it can only be done
// within the current thread's context.
// So there can be no other racing threads.
//
if (myThreadPublicTracerData != nullptr) {
std::lock_guard<std::mutex> lock(perThreadTracerDataMutex);
perThreadTracerDataList.remove(myThreadPublicTracerData);
delete myThreadPublicTracerData;
myThreadPublicTracerData = nullptr;
}
}
ThreadPrivateTracerData::ThreadPrivateTracerData() {
myThreadPublicTracerData = nullptr;
}
ThreadPrivateTracerData::~ThreadPrivateTracerData() {
freePerThreadPublicTracerData();
void APITracerContextImp::removeThreadTracerDataFromList(ThreadPrivateTracerData *threadDataP) {
std::lock_guard<std::mutex> lock(threadTracerDataListMutex);
if (threadTracerDataList.empty())
return;
threadTracerDataList.remove(threadDataP);
}
thread_local ThreadPrivateTracerData myThreadPrivateTracerData;
//
// This thread_local allows for an optimisation of the test_for_tracer_array_references()
// function. The optimization adds a test and branch, but it allows the common code path
// to avoid TWO out of line function calls.
//
// One function call is to call the constructor for the thread_private_tracer_data class.
// Note that this call is probably pretty heavy-weight, because it needs to be thread safe.
// It MUST include a mutex.
//
// The second function call we avoid is the call to the thread_private_tracer_data class's
// allocate memory member. It appears that at least with the Linux g++ compiler,
// the "inline" annotation on a member function is accepted at compile time, but does not
// change the code that is generated.
//
static thread_local bool myThreadPrivateTracerDataIsInitialized = false;
ThreadPrivateTracerData::ThreadPrivateTracerData() {
isInitialized = false;
onList = false;
tracerArrayPointer.store(nullptr, std::memory_order_relaxed);
}
ThreadPrivateTracerData::~ThreadPrivateTracerData() {
if (onList) {
globalAPITracerContextImp.removeThreadTracerDataFromList(this);
onList = false;
}
tracerArrayPointer.store(nullptr, std::memory_order_relaxed);
}
void ThreadPrivateTracerData::removeThreadTracerDataFromList(void) {
if (onList) {
globalAPITracerContextImp.removeThreadTracerDataFromList(this);
onList = false;
}
tracerArrayPointer.store(nullptr, std::memory_order_relaxed);
}
bool ThreadPrivateTracerData::testAndSetThreadTracerDataInitializedAndOnList(void) {
if (!isInitialized) {
isInitialized = true;
onList = true;
globalAPITracerContextImp.addThreadTracerDataToList(&myThreadPrivateTracerData);
}
return onList;
}
bool APITracerContextImp::isTracingEnabled() { return driver_ddiTable.enableTracing; }
@@ -140,10 +140,10 @@ bool APITracerContextImp::isTracingEnabled() { return driver_ddiTable.enableTrac
// Return 1 if a reference is found. Otherwise return 0.
//
ze_bool_t APITracerContextImp::testForTracerArrayReferences(tracer_array_t *tracerArray) {
std::lock_guard<std::mutex> lock(perThreadTracerDataMutex);
std::list<per_thread_tracer_data_t *>::iterator itr;
for (itr = perThreadTracerDataList.begin();
itr != perThreadTracerDataList.end();
std::lock_guard<std::mutex> lock(threadTracerDataListMutex);
std::list<ThreadPrivateTracerData *>::iterator itr;
for (itr = threadTracerDataList.begin();
itr != threadTracerDataList.end();
itr++) {
if ((*itr)->tracerArrayPointer.load(std::memory_order_relaxed) == tracerArray)
return 1;
@@ -293,50 +293,24 @@ ze_result_t APITracerContextImp::finalizeDisableImpTracingWait(struct APITracerI
return result;
}
//
// For an explanation of this function and the reason for its while loop,
// see the comments at the top of this file.
//
void *APITracerContextImp::getActiveTracersList() {
tracer_array_t *stableTracerArray = NULL;
//
// This test and branch allows us to avoid TWO function calls. One call is for the
// constructor for my_thread_private_tracer_data. The other is to avoid the function
// call to allocate_per_thread_tracer_data().
//
// Since my_thread_private_tracer_data_is_initialized and my_thread_private_tracer_data are
// thread_local, there is no thread safety issue here. Each thread will find
// my_thread_private_tracer_data_is_initialized to be "false" at most once.
//
if (!myThreadPrivateTracerDataIsInitialized) {
myThreadPrivateTracerData.allocatePerThreadPublicTracerData();
myThreadPrivateTracerDataIsInitialized = true;
}
if (myThreadPrivateTracerData.myThreadPublicTracerData == nullptr) {
tracer_array_t *stableTracerArray = nullptr;
if (!myThreadPrivateTracerData.testAndSetThreadTracerDataInitializedAndOnList()) {
return nullptr;
}
do {
//
// This read of active_tracer_array DOES logically signal a transfer
// of tracer structure information from the threader enable/disable/destroy
// thread to this tracing thread. So it must use memory_order_acquire
//
stableTracerArray = pGlobalAPITracerContextImp->activeTracerArray.load(std::memory_order_acquire);
myThreadPrivateTracerData.myThreadPublicTracerData->tracerArrayPointer.store(stableTracerArray, std::memory_order_relaxed);
//
// This read of active_tracer_array does NOT transfer any information
// that was not already transferred by the previous read within this loop.
// So it can use memory_order_relaxed.
//
myThreadPrivateTracerData.tracerArrayPointer.store(stableTracerArray, std::memory_order_relaxed);
} while (stableTracerArray !=
pGlobalAPITracerContextImp->activeTracerArray.load(std::memory_order_relaxed));
return (void *)stableTracerArray;
}
void APITracerContextImp::releaseActivetracersList() {
if (myThreadPrivateTracerData.myThreadPublicTracerData) {
myThreadPrivateTracerData.myThreadPublicTracerData->tracerArrayPointer.store(NULL, std::memory_order_relaxed);
}
if (myThreadPrivateTracerData.testAndSetThreadTracerDataInitializedAndOnList())
myThreadPrivateTracerData.tracerArrayPointer.store(nullptr, std::memory_order_relaxed);
}
} // namespace L0

View File

@@ -53,26 +53,6 @@ typedef struct tracerArray {
tracer_array_entry_t *tracerArrayEntries;
} tracer_array_t;
typedef struct per_thread_public_tracer_data {
std::atomic<tracer_array_t *> tracerArrayPointer;
std::thread::id thread_id;
} per_thread_tracer_data_t;
class ThreadPrivateTracerData {
public:
per_thread_tracer_data_t *myThreadPublicTracerData;
void allocatePerThreadPublicTracerData();
void freePerThreadPublicTracerData();
ThreadPrivateTracerData();
~ThreadPrivateTracerData();
private:
ThreadPrivateTracerData(const ThreadPrivateTracerData &);
ThreadPrivateTracerData &operator=(const ThreadPrivateTracerData &);
};
extern thread_local ThreadPrivateTracerData myThreadPrivateTracerData;
typedef enum tracingState {
disabledState, // tracing has never been enabled
enabledState, // tracing is enabled.
@@ -91,13 +71,30 @@ struct APITracerImp : APITracer {
private:
};
class ThreadPrivateTracerData {
public:
void clearThreadTracerDataOnList(void) { onList = false; }
void removeThreadTracerDataFromList(void);
bool testAndSetThreadTracerDataInitializedAndOnList(void);
bool onList;
bool isInitialized;
ThreadPrivateTracerData();
~ThreadPrivateTracerData();
std::atomic<tracer_array_t *> tracerArrayPointer;
private:
ThreadPrivateTracerData(const ThreadPrivateTracerData &);
ThreadPrivateTracerData &operator=(const ThreadPrivateTracerData &);
};
struct APITracerContextImp : APITracerContext {
public:
APITracerContextImp() {
activeTracerArray.store(&emptyTracerArray, std::memory_order_relaxed);
};
~APITracerContextImp() override {}
~APITracerContextImp() override;
static void apiTracingEnable(ze_init_flag_t flag);
@@ -109,6 +106,9 @@ struct APITracerContextImp : APITracerContext {
bool isTracingEnabled();
void addThreadTracerDataToList(ThreadPrivateTracerData *threadDataP);
void removeThreadTracerDataFromList(ThreadPrivateTracerData *threadDataP);
private:
std::mutex traceTableMutex;
tracer_array_t emptyTracerArray = {0, NULL};
@@ -128,8 +128,13 @@ struct APITracerContextImp : APITracerContext {
ze_bool_t testForTracerArrayReferences(tracer_array_t *tracerArray);
size_t testAndFreeRetiredTracers();
int updateTracerArrays();
std::list<ThreadPrivateTracerData *> threadTracerDataList;
std::mutex threadTracerDataListMutex;
};
extern thread_local ThreadPrivateTracerData myThreadPrivateTracerData;
template <class T>
class APITracerCallbackStateImp {
public: