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

lib: posix: dynamic stack support for pthread_create() #25973

Closed
Tracked by #25569 ...
cfriedt opened this issue Jun 4, 2020 · 5 comments · Fixed by #44727
Closed
Tracked by #25569 ...

lib: posix: dynamic stack support for pthread_create() #25973

cfriedt opened this issue Jun 4, 2020 · 5 comments · Fixed by #44727
Labels
area: POSIX POSIX API Library Enhancement Changes/Updates/Additions to existing features

Comments

@cfriedt
Copy link
Member

cfriedt commented Jun 4, 2020

Is your enhancement proposal related to a problem? Please describe.
Currently, Zephyr's pthread_create() implementation requires that a pthread_attr_t be passed in whereas the POSIX specification explicitly allows NULL, in which case a suitably-sized stack is automatically allocated.

For the majority of use cases, static stack allocation should continue to be the preferred approach. However, there are some specific uses cases where this might not be ideal. For example, supporting std::thread with our existing toolchain and libstdc++'s default gthr-posix configuration.

Describe the solution you'd like

IIRC, we should be able to use the K_THREAD_STACK_LEN() macro for calculating the amount of space to allocate for a thread stack, and then the Z_THREAD_STACK_BUFFER() macro to get the offset into the allocated buffer that takes alignment and padding into account.

It doesn't even really make sense to limit this to pthread stacks, but it should be possible to use it for zephyr stacks in general. I will need to take some caution and ensure that permissions are correct for the kernel object.

There will be some additional Kconfig values. E.g

CONFIG_PTHREAD_DYNAMIC_STACK=y
CONFIG_PTHREAD_DYNAMIC_STACK_SIZE=1024

Describe alternatives you've considered

Originally my solution involved heap-allocating stacks, and then I flip-flopped and went with statically allocating a pool of thread stacks, and now I'm back to heap-allocated!

However, it's likely that we will just use heap-allocated stacks.

Additional context
I'm hoping to use this for #25572.

While ultimately, it would probably be desireable to use native zephyr threads for std::thread in order to keep the APIs separable, pthreads could be used as a stop-gap implementation as @pabigot pointed out in #25572.

@cfriedt cfriedt added the Enhancement Changes/Updates/Additions to existing features label Jun 4, 2020
@pfalcon
Copy link
Contributor

pfalcon commented Jun 4, 2020

@cfriedt: Are you aware of #16435 ?

@cfriedt
Copy link
Member Author

cfriedt commented Jun 4, 2020

@cfriedt: Are you aware of #16435 ?

Aha! I was not aware of it. Seems to be nearly complete. Looks like purely a heap implementation.

@andrewboie
Copy link
Contributor

andrewboie commented Jun 4, 2020

Initially, I had considered heap-allocation for stacks, but on devices with only 10s of kB's of SRAM, it might be a slippery-slope.

Not sure I understand the downsides of using a heap? You won't need to know how many stacks to reserve in the array in advance, nor would you have to come up with a CONFIG_PTHREAD_DYNAMIC_STACK_SIZE that works for all dynamic threads. It all depends on the app but threads tend to vary greatly in how much stack space they need. Ultimately it's up to you though.

Obviously, it would also be possible to use entirely heap-allocated dynamic stacks as well.

This is almost possible, but not quite. If you wanted to go the heap route I think there are two things standing in the way of this:

  • Stacks have specific alignment constraints. We need an allocator that can specify alignment. The old k_mem_pool couldn't do it. We have a new k_heap implementation, I remember @andyross told me it wouldn't be too hard to extend it to support aligned allocation.
  • Need to know the alignment characteristics for stack objects of a given in a generic way. I'm working on a stack overhaul PR overhaul thread stack specification #24714 which adds generic macros to know how to align the base address and round up the size properly, so these should be available soon.

I'll probably open a ticket for a general k_thread_stack_alloc() API which uses the above, we get questions about heap allocated stacks from time to time, and it could be useful for other compat layers besides POSIX (CMSIS comes to mind)

@cfriedt
Copy link
Member Author

cfriedt commented Jun 4, 2020

Initially, I had considered heap-allocation for stacks, but on devices with only 10s of kB's of SRAM, it might be a slippery-slope.

Not sure I understand the downsides of using a heap? You won't need to know how many stacks to reserve in the array in advance, nor would you have to come up with a CONFIG_PTHREAD_DYNAMIC_STACK_SIZE that works for all dynamic threads. It all depends on the app but threads tend to vary greatly in how much stack space they need. Ultimately it's up to you though.

@andrewboie I would prefer to use the heap as well. The only downside that I can see is that if you do know the (maximum) number of "dynamic" stacks in advance, and say that threads were created and destroyed somewhat regularly, then there is a chance that an application could malloc a large amount and starve one of the threads. If the application is written in C++ with std::thread, then that could be a distinct possibility.

I could wait for #24714 if you want to exclusively go the heap route.

There will need to be some toolchain work as well to use the gthr-posix.h configuration anyway.

@cfriedt
Copy link
Member Author

cfriedt commented Oct 9, 2020

Really rough draft atm @andrewboie, using the kernel thread macros mentioned in your comment here.

The PR that's linked to this has my changes.

Booting from ROM..*** Booting Zephyr OS build zephyr-v2.4.0-278-g4709ac9f23d2  ***
Running test suite posix_apis
===================================================================
START - test_posix_pthread_execution
allocated 00001400@0x000000000011cb60
stack     00000400@0x000000000011d000
allocated 00001400@0x000000000011df78
stack     00000400@0x000000000011e000
allocated 00001400@0x000000000011f390
stack     00000400@0x0000000000120000
Thread 0 starting with scheduling policy 1 & priority 1
Thread 1 starting with scheduling policy 1 & priority 1
Thread 2 starting with scheduling policy 1 & priority 1
Bounce test OK
free   >= 00000400@0x000000000011f390
ASSERTION FAIL [z_spin_lock_valid(l)] @ ../include/spinlock.h:92
	Recursive spinlock 0x00000000001078cf
E: RAX: 0x0000000000000004 RBX: 0x0000000000000000 RCX: 0x000000000011c2c0 RDX: 0x000000000011c2c0
E: RSI: 0x000000000000005c RDI: 0x0000000000114d5f RBP: 0x0000000000163db0 RSP: 0x0000000000163db0
E:  R8: 0x0000000000000001  R9: 0x0000000000000030 R10: 0x0000000000000000 R11: 0x0000000000000000
E: R12: 0x0000000000000000 R13: 0x0000000000000000 R14: 0x0000000000000000 R15: 0x0000000000000000
E: RSP: 0x0000000000163db0 RFLAGS: 0x0000000000000046 CS: 0x0018 CR3: 0x000000000016b000
E: call trace:
E: RIP: 0x0000000000103c86
E:      0x0000000000110f53
E:      0x00000000001110c7
E:      0x000000000011096a
E:      0x0000000000107f44
E:      0x000000000010c9e7
E:      0x0000000000110b03
E:      0x0000000000102aba
E: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
E: Current thread: 0x000000000011c2c0 (idle 00)
E: Halting system

cfriedt added a commit to cfriedt/zephyr that referenced this issue Feb 10, 2021
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Feb 10, 2021
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Feb 25, 2021
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Feb 25, 2021
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Apr 11, 2022
With `CONFIG_DYNAMIC_THREAD_STACKS`, Zephyr is able to
dynamically allocate thread stacks. If the stack is `NULL`,
then a stack will be dynamically allocated. When the
stack size is 0, the stack size is defaulted to at least
`CONFIG_DYNAMIC_THREAD_STACKS_DEFAULT_SIZE` bytes.

Without `CONFIG_DYNAMIC_THREAD_STACKS`, `pthread_create()`
behaves as it did previously.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
abrodkin pushed a commit to foss-for-synopsys-dwc-arc-processors/coremark-pro that referenced this issue Jun 26, 2023
While zephyrproject-rtos/zephyr#25973
is not fixed, let's use a dirty hack in the benchmark sources.
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 14, 2023
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 14, 2023
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 14, 2023
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 14, 2023
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 17, 2023
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 17, 2023
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 17, 2023
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 18, 2023
This change allows users to call pthread_create(3) with
the pthread_attr_t argument equal to NULL, or with the
pthread_attr_t argument specifying a NULL stack but a
custom stack size.

If either of the above to requirements are met, then
a stack will be heap-allocated internally and
freed again after the thread has terminated.

This makes the Zephyr implementation of pthread_create(3)
more compliant with the normative spec.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Jul 18, 2023
Tests for dynamically allocated POSIX thread stacks.

Fixes zephyrproject-rtos#25973

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: POSIX POSIX API Library Enhancement Changes/Updates/Additions to existing features
Projects
None yet
3 participants