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.

Sign up for GitHub

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

Jump to bottom

lib: posix: dynamic stack support for pthread_create #44727

Merged
merged 4 commits into from
Jul 24, 2023
Merged

lib: posix: dynamic stack support for pthread_create #44727

merged 4 commits into from
Jul 24, 2023

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Apr 11, 2022 •
edited
Loading

Support automatic thread stack allocation for pthread_create(), which is required as part of IEEE Std. 1003.2017 (and all earlier versions).

For more info, see
https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_create.html

Fixes #25973

(Note: This depends on #44379 and #59773)

@github-actions github-actions bot added area: API Changes to public APIs area: Kernel area: POSIX POSIX API Library area: Tests Issues related to a particular existing or missing test labels Apr 11, 2022
@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from e952741 to d288c14 Compare April 11, 2022 02:45
@cfriedt cfriedt mentioned this pull request Apr 11, 2022
Open
16 tasks
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 11, 2022
@cfriedt cfriedt removed the Stale label Jun 11, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Aug 11, 2022
@cfriedt cfriedt removed the Stale label Aug 11, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Oct 11, 2022
@cfriedt cfriedt removed the Stale label Oct 12, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Dec 11, 2022
@cfriedt cfriedt removed the Stale label Dec 11, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Feb 10, 2023
@cfriedt cfriedt removed the Stale label Feb 10, 2023
@abrodkin abrodkin mentioned this pull request Mar 14, 2023
Closed
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Apr 12, 2023
@cfriedt cfriedt removed the Stale label Apr 13, 2023
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 13, 2023
@cfriedt cfriedt removed the Stale label Jun 13, 2023
@cfriedt cfriedt mentioned this pull request Jul 13, 2023
Open
@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from d288c14 to 78d154c Compare July 14, 2023 01:16
@cfriedt cfriedt mentioned this pull request Jul 18, 2023
Closed
@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from 52503b3 to e7daa08 Compare July 19, 2023 12:01
@cfriedt
Copy link
Member Author

cfriedt commented Jul 19, 2023

  • fixed merge conflict in tests/posix/common/testcase.yaml

@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch 2 times, most recently from cfa7228 to 60b5cbf Compare July 20, 2023 19:01
@cfriedt
Copy link
Member Author

cfriedt commented Jul 20, 2023

  • added Kconfig option CONFIG_PTHREAD_RECYCLER_DELAY_MS
  • fixed merge conflict with tests/posix/common/src/pthread.c

@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from 60b5cbf to b0fefbf Compare July 20, 2023 20:25
@cfriedt
Copy link
Member Author

cfriedt commented Jul 20, 2023

  • Bitten by VS Code spaces vs tabs - fixed!

keith-packard
keith-packard previously approved these changes Jul 20, 2023
View reviewed changes
Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

Looks ready to me.

ceolin
ceolin reviewed Jul 21, 2023
View reviewed changes
lib/posix/pthread.c Outdated Show resolved Hide resolved
ceolin
ceolin reviewed Jul 21, 2023
View reviewed changes
kernel/dynamic.c Outdated Show resolved Hide resolved
ceolin
ceolin reviewed Jul 21, 2023
View reviewed changes
kernel/dynamic.c Outdated Show resolved Hide resolved
@ceolin
Copy link
Member

ceolin commented Jul 21, 2023

minor comments, it is looking pretty good !

@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from b0fefbf to 4340ab4 Compare July 21, 2023 21:02
cfriedt and others added 4 commits July 21, 2023 18:02
@cfriedt
kernel: dynamic: remove unnecessary size assignment
43ffae2 
Previously, the kernel stack size was adjusted for no apparent
reason.

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
@cfriedt
kernel: dynamic: declare dynamic stubs when disabled
e5e7992 
With some of the recent work to disable unnecessary system
calls, there is a scenario where `z_impl_k_thread_stack_free()`
is not defined and an undefined symbol error occurs.

Safety was very concerned that dynamic thread stack code might
touch other code that does not malloc, so add a separate file
for the stack alloc and free stubs.

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
@cfriedt
pthread: facilitate dynamically allocated thread stacks
d0da8c3 
This change allows users to call pthread_create() with
the pthread_attr_t argument equal to NULL.

If Zephyr is configured with `CONFIG_DYNAMIC_THREAD`, then a
suitable thread stack will be allocated via
k_thread_stack_alloc(). The allocated thread stack is
automatically freed via k_thread_stack_free().

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

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
@cfriedt
pthread: test: facilitate dynamically allocated thread stacks
4a69113 
Tests for dynamically allocated POSIX thread stacks.

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
@cfriedt cfriedt force-pushed the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch from 4340ab4 to 4a69113 Compare July 21, 2023 22:07
@cfriedt
Copy link
Member Author

cfriedt commented Jul 21, 2023

  • rebased

@cfriedt
Copy link
Member Author

cfriedt commented Jul 22, 2023

Unrelated errors are unrelated. Need to wait until a hotfix or 2 rolls in

@cfriedt
Copy link
Member Author

cfriedt commented Jul 22, 2023

@keith-packard @ceolin - look ok?

keith-packard
keith-packard approved these changes Jul 22, 2023
View reviewed changes
Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

Looks ready; had a question about whether you want to require any attribute use to include a stack as that doesn't seem entirely compatible with POSIX? Happy to let that come in a later PR.

@cfriedt
Copy link
Member Author

cfriedt commented Jul 22, 2023

Looks ready; had a question about whether you want to require any attribute use to include a stack as that doesn't seem entirely compatible with POSIX? Happy to let that come in a later PR.

There are other use cases - another PR would be good and a kconfig of course.

@cfriedt cfriedt requested a review from ceolin July 23, 2023 17:58
@cfriedt cfriedt merged commit 5a28297 into zephyrproject-rtos:main Jul 24, 2023
@cfriedt cfriedt deleted the issue/25973/lib-posix-dynamic-stack-support-for-pthread-create branch November 14, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keith-packard keith-packard keith-packard approved these changes

@ceolin ceolin ceolin approved these changes

@peter-mitsis peter-mitsis Awaiting requested review from peter-mitsis

@nashif nashif Awaiting requested review from nashif

@dcpleung dcpleung Awaiting requested review from dcpleung

@andyross andyross Awaiting requested review from andyross

Assignees

@cfriedt cfriedt

Labels
area: API Changes to public APIs area: Kernel area: POSIX POSIX API Library area: Tests Issues related to a particular existing or missing test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lib: posix: dynamic stack support for pthread_create()
4 participants
@cfriedt @ceolin @andyross @keith-packard