-
Notifications
You must be signed in to change notification settings - Fork 1.6k
sched/wqueue: add configuring the stack of an hpwork/lpwork as static #18072
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -308,6 +308,7 @@ static int work_thread_create(FAR const char *name, int priority, | |||||||||||||
| char arg1[32]; | ||||||||||||||
| int wndx; | ||||||||||||||
| int pid; | ||||||||||||||
| FAR void *stack = NULL; | ||||||||||||||
|
|
||||||||||||||
| /* Don't permit any of the threads to run until we have fully initialized | ||||||||||||||
| * all of them. | ||||||||||||||
|
|
@@ -325,8 +326,15 @@ static int work_thread_create(FAR const char *name, int priority, | |||||||||||||
| argv[1] = arg1; | ||||||||||||||
| argv[2] = NULL; | ||||||||||||||
|
|
||||||||||||||
| pid = kthread_create_with_stack(name, priority, stack_addr, stack_size, | ||||||||||||||
| work_thread, argv); | ||||||||||||||
| /* In case of the stack_addr is NULL */ | ||||||||||||||
|
|
||||||||||||||
| if (stack_addr) | ||||||||||||||
| { | ||||||||||||||
| stack = (FAR void *)((uintptr_t)stack_addr + wndx * stack_size); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pid = kthread_create_with_stack(name, priority, stack, | ||||||||||||||
| stack_size, work_thread, argv); | ||||||||||||||
|
|
||||||||||||||
| DEBUGASSERT(pid > 0); | ||||||||||||||
| if (pid < 0) | ||||||||||||||
|
|
@@ -550,9 +558,21 @@ int work_start_highpri(void) | |||||||||||||
|
|
||||||||||||||
| sinfo("Starting high-priority kernel worker thread(s)\n"); | ||||||||||||||
|
|
||||||||||||||
| #ifdef SCHED_HPWORKSTACKSECTION | ||||||||||||||
| static uint8_t hp_work_stack[CONFIG_SCHED_HPNTHREADS] | ||||||||||||||
| [CONFIG_SCHED_HPWORKSTACKSIZE] | ||||||||||||||
| locate_data(CONFIG_SCHED_HPWORKSTACKSECTION); | ||||||||||||||
|
|
||||||||||||||
| return work_thread_create(HPWORKNAME, | ||||||||||||||
| CONFIG_SCHED_HPWORKPRIORITY, | ||||||||||||||
| hp_work_stack, | ||||||||||||||
| CONFIG_SCHED_HPWORKSTACKSIZE, | ||||||||||||||
| (FAR struct kwork_wqueue_s *)&g_hpwork); | ||||||||||||||
| #else | ||||||||||||||
| return work_thread_create(HPWORKNAME, CONFIG_SCHED_HPWORKPRIORITY, NULL, | ||||||||||||||
| CONFIG_SCHED_HPWORKSTACKSIZE, | ||||||||||||||
| (FAR struct kwork_wqueue_s *)&g_hpwork); | ||||||||||||||
| #endif | ||||||||||||||
| } | ||||||||||||||
| #endif /* CONFIG_SCHED_HPWORK */ | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -578,9 +598,22 @@ int work_start_lowpri(void) | |||||||||||||
|
|
||||||||||||||
| sinfo("Starting low-priority kernel worker thread(s)\n"); | ||||||||||||||
|
|
||||||||||||||
| return work_thread_create(LPWORKNAME, CONFIG_SCHED_LPWORKPRIORITY, NULL, | ||||||||||||||
| #ifdef SCHED_LPWORKSTACKSECTION | ||||||||||||||
| static uint8_t lp_work_stack[CONFIG_SCHED_LPNTHREADS] | ||||||||||||||
| [CONFIG_SCHED_LPWORKSTACKSIZE] | ||||||||||||||
| locate_data(CONFIG_SCHED_LPWORKSTACKSECTION); | ||||||||||||||
|
Comment on lines
+602
to
+604
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
emm... stack align should set the data type to uint32_t or other arch requirement ...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks for the correction. I think it can be revised as follows:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aligned_data just aligned the first stack, we should consider CONFIG_SCHED_LPWORKSTACKSIZE also must align with arch requirement
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Alright, the modifications below should be appropriate. Please check them. Thank you. #define LP_WORK_STACK_SIZE STACK_ALIGN_UP(CONFIG_SCHED_LPWORKSTACKSIZE) static aligned_data(STACK_ALIGNMENT) uint8_t return work_thread_create(LPWORKNAME, |
||||||||||||||
|
|
||||||||||||||
| return work_thread_create(LPWORKNAME, | ||||||||||||||
| CONFIG_SCHED_LPWORKPRIORITY, | ||||||||||||||
| lp_work_stack, | ||||||||||||||
| CONFIG_SCHED_LPWORKSTACKSIZE, | ||||||||||||||
| (FAR struct kwork_wqueue_s *)&g_lpwork); | ||||||||||||||
| #else | ||||||||||||||
| return work_thread_create(LPWORKNAME, | ||||||||||||||
| CONFIG_SCHED_LPWORKPRIORITY, NULL, | ||||||||||||||
| CONFIG_SCHED_LPWORKSTACKSIZE, | ||||||||||||||
| (FAR struct kwork_wqueue_s *)&g_lpwork); | ||||||||||||||
| #endif | ||||||||||||||
| } | ||||||||||||||
| #endif /* CONFIG_SCHED_LPWORK */ | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is causing a failing build for RPi4B.
Usage of this definition is guarded by
In kwork_thread.c l561, but
ifdefdoes not evaluate to false with the default value""(empty string). There needs to be a boolean condition guarding this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi:
I think I've identified the root cause of the issue. It's a bug in the makefile:
You might have SCHED_HPWORK disabled. Could you please try enabling SCHED_HPWORK and test again? Alternatively, could you let me know which configuration you used for compilation? I'll try to reproduce it locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick response! I was testing the patch in #18333 using the
raspberrypi-4b:sdconfiguration. I faced this error after rebasing the patch onto main, but I tested again on mainline afterwards and did not have this issue. It may not be relevant and instead due to some build artifacts being out of sync.Don't worry about this issue. If I encounter it on mainline again I'll let you know but I think it was just due to some out of sync build stuff. Thank you again!