From ec820daba30807df5b21e6cce6cb0606ced3faf6 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 25 Apr 2019 00:41:59 -0700 Subject: [PATCH] rocksdb: use C++ constructor/destructor for thread_ctx There's some more work needed to clean this up, but this at least gets RocksDB working for now after the following patch which enables dynamically specifying the RocksDB commit ID. Note that for now, we've modified the RocksDB BGThreadWrapper to call SpdkInitializeThread() explicitly. We'll be able to move that in the future, but for now we check whether the channel has already been allocated so that this extra SpdkInitializeThread() effectively becomes a nop. Note that for the main thread, g_fs hasn't been set yet, so we can't allocate the thread_ctx. So we still need an explicit SpdkInitializeThread() call in the main thread. This has the nice side effect of removing the need for SpdkEnv to override Env::StartThread - so remove all of the code associate with that. Signed-off-by: Jim Harris Change-Id: I1e8d12b74e688953e15d5d6df58b93e3f5b74c3d Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452112 Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu Tested-by: SPDK CI Jenkins --- lib/rocksdb/env_spdk.cc | 52 +++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/lib/rocksdb/env_spdk.cc b/lib/rocksdb/env_spdk.cc index aebe3efe3..d5b02339b 100644 --- a/lib/rocksdb/env_spdk.cc +++ b/lib/rocksdb/env_spdk.cc @@ -58,11 +58,31 @@ uint32_t g_lcore = 0; std::string g_bdev_name; volatile bool g_spdk_ready = false; volatile bool g_spdk_start_failure = false; -struct sync_args { + +void SpdkInitializeThread(void); +void SpdkFinalizeThread(void); + +class SpdkThreadCtx +{ +public: struct spdk_fs_thread_ctx *channel; + + SpdkThreadCtx(void) : channel(NULL) + { + SpdkInitializeThread(); + } + + ~SpdkThreadCtx(void) + { + SpdkFinalizeThread(); + } + +private: + SpdkThreadCtx(const SpdkThreadCtx &); + SpdkThreadCtx &operator=(const SpdkThreadCtx &); }; -__thread struct sync_args g_sync_args; +thread_local SpdkThreadCtx g_sync_args; static void __call_fn(void *arg1, void *arg2) @@ -510,7 +530,6 @@ public: } return Status::OK(); } - virtual void StartThread(void (*function)(void *arg), void *arg) override; virtual Status LockFile(const std::string &fname, FileLock **lock) override { std::string name = sanitize_path(fname, mDirectory); @@ -583,7 +602,7 @@ void SpdkInitializeThread(void) { struct spdk_thread *thread; - if (g_fs != NULL) { + if (g_fs != NULL && g_sync_args.channel == NULL) { thread = spdk_thread_create("spdk_rocksdb", NULL); spdk_set_thread(thread); g_sync_args.channel = spdk_fs_alloc_thread_ctx(g_fs); @@ -594,32 +613,10 @@ void SpdkFinalizeThread(void) { if (g_sync_args.channel) { spdk_fs_free_thread_ctx(g_sync_args.channel); + g_sync_args.channel = NULL; } } -struct SpdkThreadState { - void (*user_function)(void *); - void *arg; -}; - -static void SpdkStartThreadWrapper(void *arg) -{ - SpdkThreadState *state = reinterpret_cast(arg); - - SpdkInitializeThread(); - state->user_function(state->arg); - SpdkFinalizeThread(); - delete state; -} - -void SpdkEnv::StartThread(void (*function)(void *arg), void *arg) -{ - SpdkThreadState *state = new SpdkThreadState; - state->user_function = function; - state->arg = arg; - EnvWrapper::StartThread(SpdkStartThreadWrapper, state); -} - static void fs_load_cb(__attribute__((unused)) void *ctx, struct spdk_filesystem *fs, int fserrno) @@ -739,7 +736,6 @@ SpdkEnv::~SpdkEnv() } } - SpdkFinalizeThread(); spdk_app_start_shutdown(); pthread_join(mSpdkTid, NULL); }