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/userspace: Fix dynamic thread stack allocation at userspace #69540

Conversation

edersondisouza
Copy link
Collaborator

It wasn't saving adjusted stack size at either the private stack or the k_object, thus failing subsequent checks.

Test added to check for this case and prevent regressions.

peter-mitsis
peter-mitsis previously approved these changes Feb 28, 2024
nashif
nashif previously approved these changes Feb 28, 2024
dcpleung
dcpleung previously approved these changes Feb 28, 2024
@cfriedt
Copy link
Member

cfriedt commented Feb 29, 2024

LGTM - just one minor suggestion

@kartben
Copy link
Collaborator

kartben commented Feb 29, 2024

LGTM - just one minor suggestion

@cfriedt looks like you may have forgotten to add it to your review

- CONFIG_DYNAMIC_THREAD_POOL_SIZE=0
- CONFIG_DYNAMIC_THREAD_ALLOC=y
- CONFIG_USERSPACE=y
- CONFIG_DYNAMIC_OBJECTS=y
Copy link
Member

@cfriedt cfriedt Feb 29, 2024

Choose a reason for hiding this comment

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

CONFIG_DYNAMIC_OBJECTS might be better off in prj.conf, in which case this 011 case should be the same as the other.

It should be activated whenever userspace is active in these tests in any case.

teburd
teburd previously approved these changes Feb 29, 2024
It wasn't saving adjusted stack size at either the private stack or the
k_object, thus failing subsequent checks.

Test added to check for this case and prevent regressions.

Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
@edersondisouza
Copy link
Collaborator Author

v2:

  • Simplify testcase.yaml - no need for another entry, if we enable CONFIG_DYNAMIC_OBJECTS.

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @edersondisouza 👍

@edersondisouza
Copy link
Collaborator Author

Any comments, @andyross?

@fabiobaltieri fabiobaltieri assigned dcpleung and unassigned andyross Mar 6, 2024
@henrikbrixandersen henrikbrixandersen merged commit 4440d6a into zephyrproject-rtos:main Mar 6, 2024
34 checks passed
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.

10 participants