app: Add work queue for long blocking operations - #409
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a dedicated Zephyr work queue intended for long/blocking operations so the Serial Modem’s primary AT-command processing path (driven via the existing sm_work_q) is less likely to be stalled by lengthy tasks (e.g., nRF Cloud connect/location requests).
Changes:
- Adds a new optional
sm_blocking_work_qwork queue (with configurable stack size) alongside the existingsm_work_q. - Starts the new blocking work-queue thread from
main()when enabled. - Moves several nRF Cloud work items from
sm_work_qtosm_blocking_work_qand adds busy-guarding for connect/disconnect work.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/sm_util.h | Exposes sm_blocking_work_q (when enabled) as a shared extern. |
| app/src/sm_at_nrfcloud.c | Routes nRF Cloud connect/disconnect and location request work to the blocking queue; adds k_work_busy_get() checks. |
| app/src/main.c | Defines, initializes, and starts the blocking work queue thread. |
| app/Kconfig | Adds SM_BLOCKING_WORK_Q and stack-size config; enables it for SM_NRF_CLOUD. |
Suppressed comments (4)
app/src/sm_at_nrfcloud.c:257
- This work is submitted to sm_blocking_work_q without guarding CONFIG_SM_BLOCKING_WORK_Q. This breaks the at_nrfcloud unit test build (no CONFIG_SM_BLOCKING_WORK_Q and no sm_blocking_work_q stub) and makes the module harder to reuse in configurations that don’t enable the blocking queue. Add a compile-time fallback to sm_work_q.
k_work_submit_to_queue(&sm_blocking_work_q, &nrfcloud_conn_work);
app/src/sm_at_nrfcloud.c:456
- This submits the location request work to sm_blocking_work_q unconditionally. The at_nrfcloud unit test build compiles this file without CONFIG_SM_BLOCKING_WORK_Q (and only provides sm_work_q), so this will fail to compile there. Consider a compile-time fallback to sm_work_q when the blocking queue isn’t enabled.
k_work_submit_to_queue(&sm_blocking_work_q, &nrfcloud_loc_req_work);
app/src/sm_at_nrfcloud.c:589
- This submits nrfcloud_loc_req_work to sm_blocking_work_q unconditionally. In the at_nrfcloud unit test build CONFIG_SM_BLOCKING_WORK_Q isn’t defined, so sm_blocking_work_q isn’t declared/defined and this will fail to compile. Add a compile-time fallback to sm_work_q when the blocking queue is unavailable.
k_work_submit_to_queue(&sm_blocking_work_q, &nrfcloud_loc_req_work);
app/src/sm_at_nrfcloud.c:622
- This submits nrfcloud_loc_req_work to sm_blocking_work_q unconditionally. The at_nrfcloud unit test target doesn’t define CONFIG_SM_BLOCKING_WORK_Q and only stubs sm_work_q, so this breaks that build. Add a compile-time fallback to sm_work_q when the blocking queue isn’t enabled.
k_work_submit_to_queue(&sm_blocking_work_q, &nrfcloud_loc_req_work);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Cannot test locally. Running the integration tests in CI. |
71f8553 to
08a14f7
Compare
|
Memory usage changed for nRF91M1 configuration. See CI run text data bss dec hex flash% ram%
--- baseline
+++ pr
@@ -2 +2 @@
- 319456 74588 167246 561290 8908a 95.29% 82.07%
+ 319540 74616 169534 563690 899ea 95.31% 83.15% |
|
|
||
| config SM_BLOCKING_WORK_Q_STACK_SIZE | ||
| int "Stack size for the blocking operations work queue" | ||
| default 4096 |
There was a problem hiding this comment.
As discussed, we can decrease SM work queue stack size from 4kB to 2kB when nRF Cloud location requests are not there anymore. Those requests require ~3.3kB of stack due to nrf_cloud_coap library implementation.
|
|
||
| menu "Nordic Serial Modem" | ||
|
|
||
| config SM_BLOCKING_WORK_Q |
There was a problem hiding this comment.
If we are merging this PR, I'm not sure we should have this configurability for the blocking work queue.
There was a problem hiding this comment.
I want this off for Native TLS. The nrf cloud does not fit in there and RAM is hard to grab.
We don't want to block the AT-command processing for long periods of time when we do long blocking operations. - Add a separate work queue for those. - Drop the sm_work_q stack size to 2048 as the blocking operations are the ones with most stack usage. Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
08a14f7 to
33f917e
Compare
We don't want to block the AT-command processing for long periods of time when we do long blocking operations.