kernel: work: add k_work_queue_run() #90746
Conversation
github-actions
bot
requested review from
andyross,
ceolin,
cfriedt,
dcpleung,
nashif,
npitre,
peter-mitsis and
TaiJuWu
include/zephyr/kernel.h
Outdated
| /** @brief Run work queue using calling thread | ||
| * | ||
| * This will run the work queue forever, the function never returns | ||
| * | ||
| * @param queue the queue to run |
There was a problem hiding this comment.
Missing doxygen for the configuration parameter.
Wouldn't it be possible to call k_work_queue_stop to make this function return?
There was a problem hiding this comment.
Yeah, probably. Let me test that and adjust as needed.
There was a problem hiding this comment.
@pdgendt Needed one small fix I missed for using the thread_id, along with a test for it.
petejohanson
force-pushed
the
kernel-add-workqueu-run
branch
2 times, most recently
from
66bed1b to
809e4fa
Compare
|
|
||
| k_work_queue_run(queue, &cfg); | ||
| } | ||
|
|
||
| static void test_queue_start(void) |
There was a problem hiding this comment.
this is now not testing k_work_queue_start as the test name says? Maybe you should be adding a new test for k_work_queue_run instead?
There was a problem hiding this comment.
Fair. I'll add a separate dedicated test for k_work_queue_run instead of mixing them together.
There was a problem hiding this comment.
So... that function is actually doing the setup for the other tests that then use the various queues for their tests. I have added a dedicated run/stop test (along with a small fix) to satisfy the request from @pdgendt to support k_work_queue_stop() properly. I'm happy to adjust this somehow as well, if you really want.
petejohanson
force-pushed
the
kernel-add-workqueu-run
branch
from
809e4fa to
a286a2d
Compare
include/zephyr/kernel.h
Outdated
| @@ -3625,6 +3625,18 @@ void k_work_queue_start(struct k_work_q *queue, | |||
| k_thread_stack_t *stack, size_t stack_size, | |||
| int prio, const struct k_work_queue_config *cfg); | |||
|
|
|||
| /** @brief Run work queue using calling thread | |||
| * | |||
| * This will run the work queue forever, the function never returns | |||
There was a problem hiding this comment.
I noticed that this signature was missing the no-return attribute and then looked at it a bit more. It will return when stopped.
| * This will run the work queue forever, the function never returns | |
| * This will run the work queue forever unless stopped by @ref k_work_queue_stop. |
The other thing that could be nice to have in a work queue config is a k_timeout_t timeout but that's probably a separate discussion.
There was a problem hiding this comment.
Thanks, yeah, forgot to update the docs after properly implementing support for the queue being stopped. Rebased the branch and updated the docs.
Regarding timeout: I could perhaps see the utility in that, but I'm not 100% convinced of the use case. I'd prefer not to mix that in with this PR.
Add the ability to block and process a work queue by invoking `k_work_queue_run` from an existing thread. This can be particularly useful for using the main thread to process work items, in the same vein as the system work queue, but from a lower priority/preemptible thread. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no> Co-authored-by: Peter Johanson <peter@peterjohanson.com>
petejohanson
force-pushed
the
kernel-add-workqueu-run
branch
from
a286a2d to
3bc0056
Compare
|
I wanted to put forward this additional kernel API for review/discussion. The initial work was completed by @bjarki-andreasen and then I've done some minor tweaks to the code & tests to ensure this path gets tested. (I've left @bjarki-andreasen as the
Signed-off-byand added myself withCo-authored-by, but if that's the not process please let me know)Add the ability to block and process a work queue by invoking
k_work_queue_runfrom an existing thread. This can be particularly useful for using themainthread to process work items, in the same vein as the system work queue, but from a lower priority/preemptible thread.Technical Details
k_tid_tthat is processing the queue, so we can use that ID for the couple areas in the work queue code that need to know if calls into the API are coming from the same thread currently processing a work item. Previously, the internal thread was used to make this comparison.struct k_thread. Doing so would likely need a restructuring of the work queue API for the existing APIs to pass in an externally allocated thread for all "entrypoints". I'm open to ideas here to avoid this, but I don't see any easy way.TIA.