kernel: userspace: dynamic thread stack support #59773
Conversation
|
@ceolin - I've applied your fixups and pushed to |
ceolin
force-pushed
the
stack/dynamic
branch
7 times, most recently
from
564bbca
to
6b1d662
Compare
Test that automatic thread stack allocation works for both user and kernel threads. Signed-off-by: Flavio Ceolin <[email protected]>
Add a new API to dynamically allocate kernel objects that allow passing an arbitrary size. This new API allows to allocate dynamic thread stack. Signed-off-by: Flavio Ceolin <[email protected]>
Allocate kernel object for userspace thread stack. Signed-off-by: Flavio Ceolin <[email protected]>
ceolin
changed the title
zephyrbot
added
area: Kernel
area: Base OS
| /* | ||
| * Linked list of allocated kernel objects, for iteration over all allocated | ||
| * objects (and potentially deleting them during iteration). | ||
| */ | ||
| static sys_dlist_t obj_list = SYS_DLIST_STATIC_INIT(&obj_list); | ||
|
|
||
| /* | ||
| * TODO: Write some hash table code that will replace both obj_rb_tree | ||
| * and obj_list. | ||
| * TODO: Write some hash table code that will replace obj_list. |
There was a problem hiding this comment.
There is hash_map_api.h if you are looking for one
kernel/userspace.c
Outdated
| struct dyn_obj_base base; | ||
|
|
||
| /* The object itself */ | ||
| uint8_t data[] __aligned(DYN_OBJ_DATA_ALIGN_K_THREAD_STACK); |
There was a problem hiding this comment.
This looks suspicious? Aligning the data[] array means that on MMU systems it's going to waste 4096 - sizeof(struct dyn_obj_base) bytes of padding! Tracking the stack as a kobj makes sense, but putting the whole stack inline seems like it's not going to work well.
There was a problem hiding this comment.
yep ... there is this waste of memory indeed, that is probably no longer needed now that I am searching the object linearly. I can now do two allocations and avoid it. Let met try it out.
There was a problem hiding this comment.
I removed this alignment requirement for the object and make a new allocation for the stack data itself (with the proper alignment), this can potentially avoid some memory waste.
| @@ -193,6 +227,9 @@ static size_t obj_align_get(enum k_objects otype) | |||
| ret = __alignof(struct dyn_obj); | |||
| #endif | |||
| break; | |||
| case K_OBJ_THREAD_STACK_ELEMENT: | |||
| ret = __alignof(struct dyn_obj_stack); | |||
There was a problem hiding this comment.
Similar issue, but a nitpick: That struct itself isn't aligned, just a submember. Does this give the right answer on all toolchains? Seems like it might be clearer to return DYN_OBJ_DATA_ALIGN_K_THREAD_STACK specifically.
There was a problem hiding this comment.
that function is asking for the object alignment and not the data itself.
Add support for dynamic thread stack objects. A new container for this kernel object was added to avoid its alignment constraint to all dynamic objects. Signed-off-by: Flavio Ceolin <[email protected]>
Fix the preference allocation logic. If pool is preferred but POOL_SIZE is 0 or pool allocation fails, it fallbacks to heap allocation if it is enabled. Signed-off-by: Flavio Ceolin <[email protected]>
Since the rbtree is using as list because we no longer can assume that the object pointer is the address of the data field in the dynamic object struct, lets just use the already existent dlist for tracking dynamic kernel objects. Signed-off-by: Flavio Ceolin <[email protected]>
There was a problem hiding this comment.
One more nitpick about the confusingly aliased "data" fields. FWIW I'm seeing now where that alignment padding in the struct came from, it wasn't originally for page padding, it was to get the struct k_thread aligned correctly on x86, which (IIRC) needs that for the x87/SSE context switch field.
No reason not to approve, but if the two uses of data became a proper union with an assert or something to track misuse, I wouldn't cry.
| }; | ||
|
|
||
| struct dyn_obj { | ||
| struct dyn_obj_base base; | ||
|
|
||
| /* The object itself */ | ||
| uint8_t data[] __aligned(DYN_OBJ_DATA_ALIGN_K_THREAD); |
There was a problem hiding this comment.
Shouldn't this item be removed now? The address where it would be is now (in a dyn_obj_stack) the "data" pointer (identically named, but a pointer and not an array!). Trying to use this now would be an aliasing bug.
There was a problem hiding this comment.
I think I can make everything work with only one object type now. This was needed to avoid imposing the stack alignment requirement for all objects. I can try it out later.
Support for dynamic thread stack on userspace.