From 090b8af12b91c28e3a73b99067dfb893f9af999c Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 17 Nov 2022 02:22:44 +0000 Subject: [PATCH] thread: add spdk_thread_get_app_thread The "app thread" will always be the first thread created using spdk_thread_create(). There are many operations throughout SPDK that implicitly expect to happen in the context of this app thread, so by formalizing it we can start to make assertions on this to help clarify and simplify locking and synchronization through the code base. Signed-off-by: Jim Harris Change-Id: I7133b58c311710f1d132ee5f09500ffeb4168b15 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15497 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Tomasz Zawadzki --- CHANGELOG.md | 5 +++++ include/spdk/thread.h | 18 +++++++++++++++++- lib/thread/Makefile | 2 +- lib/thread/spdk_thread.map | 1 + lib/thread/thread.c | 38 +++++++++++++++++++++++++++++++------- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c7223ef0..ec835643e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,11 @@ caused a function callback in file descriptor group to execute. Added API `spdk_json_write_double` and `spdk_json_write_named_double` to allow for writing and decoding of the the double data type. +### thread + +Added `spdk_thread_get_app_thread` which returns the first thread that was created using +`spdk_thread_create`. + ## v22.09 ### accel diff --git a/include/spdk/thread.h b/include/spdk/thread.h index 121d93f84..168f46044 100644 --- a/include/spdk/thread.h +++ b/include/spdk/thread.h @@ -231,6 +231,10 @@ void spdk_thread_lib_fini(void); /** * Creates a new SPDK thread object. * + * Note that the first thread created via spdk_thread_create() will be designated as + * the app thread. Other SPDK libraries may place restrictions on certain APIs to + * only be called in the context of this app thread. + * * \param name Human-readable name for the thread; can be retrieved with spdk_thread_get_name(). * The string is copied, so the pointed-to data only needs to be valid during the * spdk_thread_create() call. May be NULL to specify no name. @@ -242,6 +246,15 @@ void spdk_thread_lib_fini(void); */ struct spdk_thread *spdk_thread_create(const char *name, const struct spdk_cpuset *cpumask); +/** + * Return the app thread. + * + * The app thread is the first thread created using spdk_thread_create(). + * + * \return a pointer to the app thread, or NULL if no thread has been created yet. + */ +struct spdk_thread *spdk_thread_get_app_thread(void); + /** * Force the current system thread to act as if executing the given SPDK thread. * @@ -260,7 +273,10 @@ void spdk_set_thread(struct spdk_thread *thread); * this function. This function will complete these processing. The completion can * be queried by spdk_thread_is_exited(). * - * \param thread The thread to destroy. + * Note that this function must not be called on the app thread until after it + * has been called for all other threads. + * + * \param thread The thread to exit. * * \return always 0. (return value was deprecated but keep it for ABI compatibility.) */ diff --git a/lib/thread/Makefile b/lib/thread/Makefile index 13b06c35f..359d5aa70 100644 --- a/lib/thread/Makefile +++ b/lib/thread/Makefile @@ -7,7 +7,7 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..) include $(SPDK_ROOT_DIR)/mk/spdk.common.mk SO_VER := 7 -SO_MINOR := 0 +SO_MINOR := 1 C_SRCS = thread.c LIBNAME = thread diff --git a/lib/thread/spdk_thread.map b/lib/thread/spdk_thread.map index 672f2183e..0e48a2c3c 100644 --- a/lib/thread/spdk_thread.map +++ b/lib/thread/spdk_thread.map @@ -6,6 +6,7 @@ spdk_thread_lib_init_ext; spdk_thread_lib_fini; spdk_thread_create; + spdk_thread_get_app_thread; spdk_set_thread; spdk_thread_exit; spdk_thread_is_exited; diff --git a/lib/thread/thread.c b/lib/thread/thread.c index 2071cf89e..58b410fce 100644 --- a/lib/thread/thread.c +++ b/lib/thread/thread.c @@ -32,6 +32,8 @@ #define SPDK_MAX_POLLER_NAME_LEN 256 #define SPDK_MAX_THREAD_NAME_LEN 256 +static struct spdk_thread *g_app_thread; + enum spdk_poller_state { /* The poller is registered with a thread but not currently executing its fn. */ SPDK_POLLER_STATE_WAITING, @@ -377,21 +379,25 @@ spdk_thread_lib_fini(void) SPDK_ERRLOG("io_device %s not unregistered\n", dev->name); } - if (g_spdk_msg_mempool) { - spdk_mempool_free(g_spdk_msg_mempool); - g_spdk_msg_mempool = NULL; - } - g_new_thread_fn = NULL; g_thread_op_fn = NULL; g_thread_op_supported_fn = NULL; g_ctx_sz = 0; + if (g_app_thread != NULL) { + _free_thread(g_app_thread); + g_app_thread = NULL; + } + + if (g_spdk_msg_mempool) { + spdk_mempool_free(g_spdk_msg_mempool); + g_spdk_msg_mempool = NULL; + } } struct spdk_thread * spdk_thread_create(const char *name, const struct spdk_cpuset *cpumask) { - struct spdk_thread *thread; + struct spdk_thread *thread, *null_thread; struct spdk_msg *msgs[SPDK_MSG_MEMPOOL_CACHE_SIZE]; int rc = 0, i; @@ -482,9 +488,23 @@ spdk_thread_create(const char *name, const struct spdk_cpuset *cpumask) thread->state = SPDK_THREAD_STATE_RUNNING; + /* If this is the first thread, save it as the app thread. Use an atomic + * compare + exchange to guard against crazy users who might try to + * call spdk_thread_create() simultaneously on multiple threads. + */ + null_thread = NULL; + __atomic_compare_exchange_n(&g_app_thread, &null_thread, thread, false, + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); + return thread; } +struct spdk_thread * +spdk_thread_get_app_thread(void) +{ + return g_app_thread; +} + void spdk_set_thread(struct spdk_thread *thread) { @@ -578,6 +598,7 @@ spdk_thread_is_exited(struct spdk_thread *thread) void spdk_thread_destroy(struct spdk_thread *thread) { + assert(thread != NULL); SPDK_DEBUGLOG(thread, "Destroy thread %s\n", thread->name); assert(thread->state == SPDK_THREAD_STATE_EXITED); @@ -586,7 +607,10 @@ spdk_thread_destroy(struct spdk_thread *thread) tls_thread = NULL; } - _free_thread(thread); + /* To be safe, do not free the app thread until spdk_thread_lib_fini(). */ + if (thread != g_app_thread) { + _free_thread(thread); + } } void *