Repository navigation
audio: kpb: make KPB service usable from user space - #11425
Open
serhiy-katsyuba-intel wants to merge 3 commits into
Open
serhiy-katsyuba-intel wants to merge 3 commits into
serhiy-katsyuba-intel wants to merge 3 commits into
Conversation
Extend the KPB service to support, in addition to the kernel-only notifier interface, the client queue (CONFIG_KPB_CLI_Q), which can also be used from user space. Replace the k_queue with a statically defined k_msgq that carries struct kpb_client by value, and replace kpb_notifier_init() and kpb_notifier_schedule() with a single kpb_notifier_schedule(id, drain_req) syscall taking only scalar arguments. No pointers are passed from user space to the kernel. As before, only a single KPB instance is supported. With the client queue, draining setup runs in the client queue thread instead of the LL thread, so lock the scheduler around kpb_init_draining() to keep kpb_copy() from preempting it. The thread is pinned to the KPB core, so this is sufficient. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
serhiy-katsyuba-intel
requested review from
abonislawski,
dbaluta,
fkwasowi,
iganakov,
iuliana-prodan,
kv2019i,
lbetlej,
lgirdwood,
lyakh,
mmaka1,
plbossart and
ranj063
as code owners
November 9, 2026 16:43
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Queued requests can dereference missing sinks and fault the kernel thread across lifecycle changes, while zero-length requests can leave KPB stuck draining with the detector paused.
2 open issues
What changed in this PR
This PR makes the KPB service callable from user space through the client queue while preserving the kernel-only notifier interface and single-instance restriction.
Changes:
- Replaces the pointer-based queue interface with a static message queue and scalar-only scheduling syscall.
- Adds client-queue support and error propagation to the module service bridge.
- Locks scheduling during draining setup to prevent interference from the audio copy thread.
| File | Description |
|---|---|
zephyr/syscall/kpb.c |
Adds the scheduling syscall verification wrapper. |
zephyr/CMakeLists.txt |
Builds the wrapper for user-space client-queue configurations. |
src/include/sof/audio/kpb.h |
Declares the scalar syscall and removes client pointers. |
src/audio/module_adapter/library/kpb_service_bridge.c |
Routes draining requests through the queue and maps errors. |
src/audio/module_adapter/Kconfig |
Avoids selecting the notifier when the client queue is enabled. |
src/audio/kpb.c |
Implements value-based queueing, instance lifecycle handling, and scheduler locking. |
Review effort: Balanced
PR 11425: test resultsRun date: 2026-11-10 14:05 UTC Tested commit: ef71b65abd91f1d5e72e9b68ca7814d0f03b5407
|
kpb_init_draining() reads the host sink state before checking the KPB state, but host_sink and sel_sink are NULL before bind and after reset/unbind. As draining can potentially be requested at any time while the KPB instance exists, such a request would dereference a NULL sink. Treat the sink as not ready if either sink is missing. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
A draining request of 0 ms, i.e. no history, only the real time stream, is accepted by kpb_init_draining(), which pauses the selector and moves KPB to KPB_STATE_DRAINING. However, kpb_draining_task() only picks up data buffered while draining and switches to KPB_STATE_HOST_COPY inside the drain_req > 0 block, so with a zero request it completes without ever leaving KPB_STATE_DRAINING, leaving KPB stuck with the detector paused and rejecting further requests. Move the drain completion handling out of that block so it also runs for a zero-length request. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
kv2019i
approved these changes
Nov 10, 2026
| kpb_init_draining(cli->dev, cli); | ||
| k_msgq_get(&kpb_cli_msgq, &cli, K_FOREVER); | ||
| /* keep the LL thread (kpb_copy()) from preempting draining setup */ | ||
| k_sched_lock(); |
Collaborator
There was a problem hiding this comment.
We haven't used this k_sched_lock() interface much. Seems to be ok here...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the KPB service to support, in addition to the kernel-only notifier interface, the client queue (CONFIG_KPB_CLI_Q), which can also be used from user space.
Replace the k_queue with a statically defined k_msgq that carries struct kpb_client by value, and replace kpb_notifier_init() and kpb_notifier_schedule() with a single kpb_notifier_schedule(id, drain_req) syscall taking only scalar arguments. No pointers are passed from user space to the kernel.
As before, only a single KPB instance is supported.
With the client queue, draining setup runs in the client queue thread instead of the LL thread, so lock the scheduler around kpb_init_draining() to keep kpb_copy() from preempting it. The thread is pinned to the KPB core, so this is sufficient.