Skip to content
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

kernel: fix runtime initialization of k_pipe object #24526

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Apr 20, 2020

Runtime initialization failed to reset the lock field, causing
problems when the pipe object is located on a stack and passed by
reference to other code. Lacking an API for initializing a spinlock
by itself use the idiom from _K_PIPE_INITIALIZER().

To simplify maintainability the initialization order is changed
slightly to match the structure field declaration order.

Signed-off-by: Peter Bigot peter.bigot@nordicsemi.no

Runtime initialization failed to reset the lock field, causing
problems when the pipe object is located on a stack and passed by
reference to other code.  Lacking an API for initializing a spinlock
by itself use the idiom from _K_PIPE_INITIALIZER().

To simplify maintainability the initialization order is changed
slightly to match the structure field declaration order.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

Where's the spot where there's a struct k_pipe on the stack? Is it in-tree? I'm trying to track spots like this down where synchronized objects are located in stack memory.

On cache-incoherent architectures, there's a desire that we place all the synchronized data in an uncached region. User stacks would be cached by default, but that breaks paradigms like this.

@pabigot
Copy link
Collaborator Author

pabigot commented Apr 20, 2020

@andyross I'm unaware of in-tree uses. There is a use in the test provided by #24486, where I encountered it during review posted in the original issue; also the author of that PR asked about the fault on slack which reminded me of the failure and its solution (in that situation, use the existing statically allocated pipe).

I'm not in favor of allocating pipes on a stack, but in restricted situations it might work and be useful, so it seems worthwhile avoiding an uninitialized-memory-reference if it happens.

@cfriedt
Copy link
Member

cfriedt commented Apr 21, 2020

This fixes some issues I've observed. E.g. if a k_pipe is k_malloc'ed or if it's on the stack as well.

@ioannisg ioannisg merged commit aea9d35 into zephyrproject-rtos:master Apr 21, 2020
@pabigot pabigot deleted the nordic/20200420c branch April 27, 2020 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants