From a30cf39975b049d4630b86c986e2974d67da1d1b Mon Sep 17 00:00:00 2001 From: Peter Mitsis Date: Mon, 11 Apr 2022 19:54:23 -0400 Subject: [PATCH] kernel: update k_thread_state_str() API When threads are in more than one state at a time, k_thread_state_str() returns a string that lists each of its states delimited by a '+'. This in turn necessitates a change to the API that includes both a pointer to the buffer to use for the string and the size of the buffer. Signed-off-by: Peter Mitsis --- include/zephyr/kernel.h | 10 ++- kernel/thread.c | 71 +++++++++++++------ subsys/shell/modules/kernel_service.c | 4 +- tests/kernel/sched/preempt/src/main.c | 7 +- .../thread_apis/src/test_kthread_for_each.c | 38 +++++++--- .../src/test_threads_suspend_resume.c | 16 +++-- 6 files changed, 103 insertions(+), 43 deletions(-) diff --git a/include/zephyr/kernel.h b/include/zephyr/kernel.h index 397571cc7df..2bfa9af5a09 100644 --- a/include/zephyr/kernel.h +++ b/include/zephyr/kernel.h @@ -1074,12 +1074,16 @@ __syscall int k_thread_name_copy(k_tid_t thread, char *buf, /** * @brief Get thread state string * - * Get the human friendly thread state string + * This routine generates a human friendly string containing the thread's + * state, and copies as much of it as possible into @a buf. * * @param thread_id Thread ID - * @retval Thread state string, empty if no state flag is set + * @param buf Buffer into which to copy state strings + * @param buf_size Size of the buffer + * + * @retval Pointer to @a buf if data was copied, else a pointer to "". */ -const char *k_thread_state_str(k_tid_t thread_id); +const char *k_thread_state_str(k_tid_t thread_id, char *buf, size_t buf_size); /** * @} diff --git a/kernel/thread.c b/kernel/thread.c index 16047c952f7..927204a6566 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -258,32 +258,57 @@ int z_impl_k_thread_name_copy(k_tid_t thread, char *buf, size_t size) #endif /* CONFIG_THREAD_NAME */ } -const char *k_thread_state_str(k_tid_t thread_id) +static size_t copy_bytes(char *dest, size_t dest_size, const char *src, size_t src_size) { - switch (thread_id->base.thread_state) { - case 0: + size_t bytes_to_copy; + + bytes_to_copy = MIN(dest_size, src_size); + memcpy(dest, src, bytes_to_copy); + + return bytes_to_copy; +} + +const char *k_thread_state_str(k_tid_t thread_id, char *buf, size_t buf_size) +{ + size_t off = 0; + uint8_t bit; + uint8_t thread_state = thread_id->base.thread_state; + static const char *states_str[8] = {"dummy", "pending", "prestart", + "dead", "suspended", "aborting", + "", "queued"}; + static const size_t states_sz[8] = {5, 7, 8, 4, 9, 8, 0, 6}; + + if ((buf == NULL) || (buf_size == 0)) { return ""; - case _THREAD_DUMMY: - return "dummy"; - case _THREAD_PENDING: - return "pending"; - case _THREAD_PRESTART: - return "prestart"; - case _THREAD_DEAD: - return "dead"; - case _THREAD_SUSPENDED: - return "suspended"; - case _THREAD_ABORTING: - return "aborting"; - case _THREAD_QUEUED: - return "queued"; - default: - /* Add a break, some day when another case gets added at the end, - * this bit of defensive programming will be useful - */ - break; } - return "unknown"; + + buf_size--; /* Reserve 1 byte for end-of-string character */ + + /* + * Loop through each bit in the thread_state. Stop once all have + * been processed. If more than one thread_state bit is set, then + * separate the descriptive strings with a '+'. + */ + + for (uint8_t index = 0; thread_state != 0; index++) { + bit = BIT(index); + if ((thread_state & bit) == 0) { + continue; + } + + off += copy_bytes(buf + off, buf_size - off, + states_str[index], states_sz[index]); + + thread_state &= ~bit; + + if (thread_state != 0) { + off += copy_bytes(buf + off, buf_size - off, "+", 1); + } + } + + buf[off] = '\0'; + + return (const char *)buf; } #ifdef CONFIG_USERSPACE diff --git a/subsys/shell/modules/kernel_service.c b/subsys/shell/modules/kernel_service.c index 2862cb6f2f7..f14c6002426 100644 --- a/subsys/shell/modules/kernel_service.c +++ b/subsys/shell/modules/kernel_service.c @@ -62,6 +62,7 @@ static void shell_tdata_dump(const struct k_thread *cthread, void *user_data) size_t size = thread->stack_info.size; const char *tname; int ret; + char state_str[32]; #ifdef CONFIG_THREAD_RUNTIME_STATS k_thread_runtime_stats_t rt_stats_thread; @@ -79,7 +80,8 @@ static void shell_tdata_dump(const struct k_thread *cthread, void *user_data) thread->base.user_options, thread->base.prio, (int64_t)thread->base.timeout.dticks); - shell_print(shell, "\tstate: %s, entry: %p", k_thread_state_str(thread), + shell_print(shell, "\tstate: %s, entry: %p", + k_thread_state_str(thread, state_str, sizeof(state_str)), thread->entry.pEntry); #ifdef CONFIG_THREAD_RUNTIME_STATS diff --git a/tests/kernel/sched/preempt/src/main.c b/tests/kernel/sched/preempt/src/main.c index 3d05b03f085..37f4c3c2fc6 100644 --- a/tests/kernel/sched/preempt/src/main.c +++ b/tests/kernel/sched/preempt/src/main.c @@ -103,9 +103,12 @@ void wakeup_src_thread(int id) */ for (int i = 0; i < NUM_THREADS; i++) { k_tid_t th = &worker_threads[i]; + char buffer[16]; + const char *str; - zassert_equal(strcmp(k_thread_state_str(th), "pending"), - 0, "worker thread %d not pending?", i); + str = k_thread_state_str(th, buffer, sizeof(buffer)); + zassert_not_null(strstr(str, "pending"), + "worker thread %d not pending?", i); } /* Wake the src worker up */ diff --git a/tests/kernel/threads/thread_apis/src/test_kthread_for_each.c b/tests/kernel/threads/thread_apis/src/test_kthread_for_each.c index 04a92891586..4e89751faa5 100644 --- a/tests/kernel/threads/thread_apis/src/test_kthread_for_each.c +++ b/tests/kernel/threads/thread_apis/src/test_kthread_for_each.c @@ -219,32 +219,50 @@ void test_k_thread_foreach_unlocked_null_cb(void) */ void test_k_thread_state_str(void) { + char state_str[32]; + const char *str; k_tid_t tid = &tdata1; tid->base.thread_state = 0; - zassert_true(strcmp(k_thread_state_str(tid), "") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "") == 0, NULL); tid->base.thread_state = _THREAD_DUMMY; - zassert_true(strcmp(k_thread_state_str(tid), "dummy") == 0, NULL); + + str = k_thread_state_str(tid, NULL, sizeof(state_str)); + zassert_true(strcmp(str, "") == 0, NULL); + + str = k_thread_state_str(tid, state_str, 0); + zassert_true(strcmp(str, "") == 0, NULL); + + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "dummy") == 0, NULL); tid->base.thread_state = _THREAD_PENDING; - zassert_true(strcmp(k_thread_state_str(tid), "pending") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "pending") == 0, NULL); tid->base.thread_state = _THREAD_PRESTART; - zassert_true(strcmp(k_thread_state_str(tid), "prestart") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "prestart") == 0, NULL); tid->base.thread_state = _THREAD_DEAD; - zassert_true(strcmp(k_thread_state_str(tid), "dead") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "dead") == 0, NULL); tid->base.thread_state = _THREAD_SUSPENDED; - zassert_true(strcmp(k_thread_state_str(tid), "suspended") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "suspended") == 0, NULL); tid->base.thread_state = _THREAD_ABORTING; - zassert_true(strcmp(k_thread_state_str(tid), "aborting") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "aborting") == 0, NULL); tid->base.thread_state = _THREAD_QUEUED; - zassert_true(strcmp(k_thread_state_str(tid), "queued") == 0, NULL); + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "queued") == 0, NULL); - tid->base.thread_state = 0xFF; - zassert_true(strcmp(k_thread_state_str(tid), "unknown") == 0, NULL); + tid->base.thread_state = _THREAD_PENDING | _THREAD_SUSPENDED; + str = k_thread_state_str(tid, state_str, sizeof(state_str)); + zassert_true(strcmp(str, "pending+suspended") == 0, NULL); } diff --git a/tests/kernel/threads/thread_apis/src/test_threads_suspend_resume.c b/tests/kernel/threads/thread_apis/src/test_threads_suspend_resume.c index 2e01c33af32..54e891e8458 100644 --- a/tests/kernel/threads/thread_apis/src/test_threads_suspend_resume.c +++ b/tests/kernel/threads/thread_apis/src/test_threads_suspend_resume.c @@ -156,20 +156,28 @@ void test_threads_suspend_timeout(void) */ void test_resume_unsuspend_thread(void) { + char buffer[32]; + const char *str; k_tid_t tid = k_thread_create(&tdata, tstack, STACK_SIZE, thread_entry, NULL, NULL, NULL, 0, K_USER, K_NO_WAIT); + /* Resume an unsuspend thread will not change the thread state. */ - zassert_true(strcmp(k_thread_state_str(tid), "queued") == 0, NULL); + str = k_thread_state_str(tid, buffer, sizeof(buffer)); + zassert_true(strcmp(str, "queued") == 0, NULL); k_thread_resume(tid); - zassert_true(strcmp(k_thread_state_str(tid), "queued") == 0, NULL); + str = k_thread_state_str(tid, buffer, sizeof(buffer)); + zassert_true(strcmp(str, "queued") == 0, NULL); /* suspend created thread */ k_thread_suspend(tid); - zassert_true(strcmp(k_thread_state_str(tid), "suspended") == 0, NULL); + str = k_thread_state_str(tid, buffer, sizeof(buffer)); + zassert_true(strcmp(str, "suspended") == 0, NULL); + /* Resume an suspend thread will make it to be next eligible.*/ k_thread_resume(tid); - zassert_true(strcmp(k_thread_state_str(tid), "queued") == 0, NULL); + str = k_thread_state_str(tid, buffer, sizeof(buffer)); + zassert_true(strcmp(str, "queued") == 0, NULL); k_thread_abort(tid); }