-
Notifications
You must be signed in to change notification settings - Fork 7.8k
System workqueue: Prevent blocking API calls #87522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
bb1e9f9
eb67bb0
949b689
f0d9a3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -523,6 +523,10 @@ static inline void z_vrfy_k_thread_resume(k_tid_t thread) | |
|
||
static void unready_thread(struct k_thread *thread) | ||
{ | ||
if (IS_ENABLED(CONFIG_SYSTEM_WORKQUEUE_NO_BLOCK) && k_is_in_sys_work()) { | ||
k_oops(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks wrong to me. "Ready" and "running" aren't the same thing. A thread can be ready but lower priority than _current. Basically: my guess is that this code will oops if you try to You need to add a test for thread == _current at least, but it would probably be better to move this test to reschedule() instead. Also: probably want a panic here and not an oops. An oops in userspace will kill only the current thread, but a misuse of the system workqueue (which obviously is a kernel thread anyway) is a global failure. And finally: neither oops nor panic give any feedback to the poor user whose code blew up. Probably wants a printk() here (or to be expressed as an __ASSERT() when available). |
||
} | ||
|
||
if (z_is_thread_queued(thread)) { | ||
dequeue_thread(thread); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
default y if ASSERT
or something similar? This is a cheap check with clear value, probably wants to be on any time CONFIG_ASSERT=y