Skip to content

userspace: proxy: fix multicore work queue handling#11024

Open
serhiy-katsyuba-intel wants to merge 3 commits into
thesofproject:mainfrom
serhiy-katsyuba-intel:pin_only_wa
Open

userspace: proxy: fix multicore work queue handling#11024
serhiy-katsyuba-intel wants to merge 3 commits into
thesofproject:mainfrom
serhiy-katsyuba-intel:pin_only_wa

Conversation

@serhiy-katsyuba-intel

Copy link
Copy Markdown
Contributor

On Xtensa, with CONFIG_SCHED_CPU_MASK_PIN_ONLY=y, k_thread_cpu_pin() cannot safely change a thread's core after it has been active on another core. Enabling CONFIG_SCHED_CPU_MASK_PIN_ONLY also enables an optimization in the thread-switch code: because Zephyr assumes that a thread will resume on the same core, it does not write back the thread's cached stack.

If k_thread_cpu_pin() subsequently moves the thread to another core, the cached stack on the new core contains garbage. The cached stack was not written back on the previous core and is not invalidated on the new core. With CONFIG_SCHED_CPU_MASK_PIN_ONLY disabled, Zephyr writes back the stack when the thread becomes inactive and invalidates it when the thread becomes active on another core.

See arch_cohere_stacks() impl here zephyrproject-rtos/zephyr@3606493

k_work_user_queue_create() starts its worker thread on core 0, leaving no safe way to re-pin it when CONFIG_SCHED_CPU_MASK_PIN_ONLY=y. Because we do not want to disable CONFIG_SCHED_CPU_MASK_PIN_ONLY or use an uncached stack, we implement a copy of k_work_user_queue_create() that does not start the worker thread. The caller configures the thread, for example by pinning it to the required core, and then calls k_thread_start().

To avoid re-pinning, rework the userspace proxy to use a separate worker per core. Each worker is pinned to its core once before it starts.

On Xtensa, with CONFIG_SCHED_CPU_MASK_PIN_ONLY=y, k_thread_cpu_pin()
cannot safely change a thread's core after it has been active on another
core. Enabling CONFIG_SCHED_CPU_MASK_PIN_ONLY also enables an optimization
in the thread-switch code: because Zephyr assumes that a thread will resume
on the same core, it does not write back the thread's cached stack.

If k_thread_cpu_pin() subsequently moves the thread to another core, the
cached stack on the new core contains garbage. The cached stack was not
written back on the previous core and is not invalidated on the new core.
With CONFIG_SCHED_CPU_MASK_PIN_ONLY disabled, Zephyr writes back the stack
when the thread becomes inactive and invalidates it when the thread becomes
active on another core.

k_work_user_queue_create() starts its worker thread on core 0, leaving no
safe way to re-pin it when CONFIG_SCHED_CPU_MASK_PIN_ONLY=y. Because we do
not want to disable CONFIG_SCHED_CPU_MASK_PIN_ONLY or use an uncached stack,
this file implements a copy of k_work_user_queue_create() that does not
start the worker thread. The caller configures the thread, for example by
pinning it to the required core, and then calls k_thread_start().

Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an Xtensa multicore issue when CONFIG_SCHED_CPU_MASK_PIN_ONLY=y by avoiding post-start CPU re-pinning of Zephyr userspace workqueue threads and restructuring the SOF userspace proxy to use a dedicated (pinned) worker per core.

Changes:

  • Add a SOF-local variant of Zephyr k_work_user_queue_create() that creates (but does not start) the worker thread, allowing the caller to pin it before k_thread_start().
  • Rework the userspace proxy to maintain one userspace IPC worker/workqueue per core instead of a single shared worker.
  • Wire the new helper into the Zephyr library build when DP-thread IPC handling is not enabled.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
zephyr/lib/user_work.c Adds sof_work_user_queue_create() and a local copy of the userspace workqueue thread entry to allow “create then pin then start”.
zephyr/include/rtos/user_work.h Exposes sof_work_user_queue_create() to SOF code.
zephyr/CMakeLists.txt Conditionally includes lib/user_work.c in the Zephyr library build.
src/audio/module_adapter/library/userspace_proxy.c Switches from a single shared userspace IPC worker to per-core workers pinned before start, and removes runtime re-pinning.

Comment thread src/audio/module_adapter/library/userspace_proxy.c
Comment thread src/audio/module_adapter/library/userspace_proxy.c Outdated
When CONFIG_SCHED_CPU_MASK_PIN_ONLY=y, re-pinning a thread to a new core
works only if the thread has never been active on another core. To avoid
re-pinning, rework the userspace proxy to use a separate worker per core.
Each worker is pinned to its core once before it starts.

Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
The proxy now uses a separate worker per core. Each work item is
allocated, submitted and processed on the same core, so cross-core
coherency is no longer required. The same applies when per-core DP
threads process work items.

Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but is there a use case or UT where it is tested ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants