sched/group: move task group into task_tcb_s to improve performance #11832
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| /* Task Group *************************************************************/ | ||
|
|
||
| struct task_group_s group; /* Pointer to shared task group data */ |
Contributor
There was a problem hiding this comment.
but, main thread may exist before other thread, the used-after-free will happen.
Contributor
Author
There was a problem hiding this comment.
Task will kill all pthreads in the group before exiting. Will the task exit early than pthread?
#0 nxsched_release_tcb (tcb=0x409f04 <nxtask_exithook+96>, ttype=255 '\377') at sched/sched_releasetcb.c:99
#1 0x000000000040a0e5 in nxtask_terminate (pid=5) at task/task_terminate.c:122
#2 0x00000000004087c7 in pthread_cancel (thread=5) at pthread/pthread_cancel.c:110
#3 0x000000000040867b in group_cancel_children_handler (pid=5, arg=0x4) at group/group_killchildren.c:113
#4 0x000000000040a342 in group_foreachchild (group=0x7ffff3db53e0, handler=0x408613 <group_cancel_children_handler>, arg=0x4) at group/group_foreachchild.c:70
#5 0x000000000040870e in group_kill_children (tcb=0x7ffff3db52a0) at group/group_killchildren.c:212
#6 0x0000000000407adb in _exit (status=0) at task/exit.c:67
#7 0x0000000000404222 in exit (status=0) at stdlib/lib_exit.c:126
#8 0x000000000040dffe in nxtask_startup (entrypt=0x444546 <hello_main>, argc=1, argv=0x7ffff3db58c0) at sched/task_startup.c:70
#9 0x00000000004076b4 in nxtask_start () at task/task_start.c:134
#10 0x0000000000413081 in pre_start () at sim/sim_initialstate.c:52
Contributor
There was a problem hiding this comment.
exit terminate the whole proccess, but pthread_exit terminate the current thread. You can reproduce the case by:
- Create a work thread and call printf in a loop
- Exit the main thread by pthread_exit
Contributor
Author
There was a problem hiding this comment.
Got, I have a update which delayed the release of the parent tcb, the parent tcb will be released after all child threads exit.
anchao
force-pushed
the
24030401
branch
5 times, most recently
from
b4e2da5 to
c64e763
Compare
acassis
approved these changes
Mar 4, 2024
Contributor
|
@anchao please rebase again, the last master fix the ci broken. |
anchao
force-pushed
the
24030401
branch
3 times, most recently
from
e8c56f0 to
79cdd63
Compare
anchao
force-pushed
the
24030401
branch
3 times, most recently
from
d112a77 to
4349caa
Compare
move task group into task_tcb_s to avoid access allocator to improve performance for Task Termination, the time consumption will be reduced ~2us (Tricore TC397 300MHZ): 15.97(us) -> 13.55(us) Signed-off-by: chao an <anchao@lixiang.com>
Contributor
|
I noticed the following crash with this PR. rv-virt:ksmp64 sabre-6quad:netnsh_smp esp32:wifi_smp (dev board) |
Contributor
|
@anchao |
Contributor
Author
@masayuki2009 emm ... sorry for the regression, thanks for your verification again. I have a fix that verifying internally. I will send a PR later. |
|
@anchao @xiaoxiang781216 #define TIMEOUT 5 /* Timeout value of 5 seconds. */
#define INTHREAD 0 /* Control going to or is already for Thread */
#define INMAIN 1 /* Control going to or is already for Main */
static int sem1; /* Manual semaphore */
static void *a_thread_func(void *arg)
{
printf("a_thread_func entry \n");
/* Indicate to main() that the thread was created. */
sem1 = INTHREAD;
/* Wait for main to detach change the attribute object and try and detach this thread.
* Wait for a timeout value of 10 seconds before timing out if the thread was not able
* to be detached. */
// sleep(TIMEOUT);
printf("a_thread_func entry 2222\n");
printf
("Test FAILED: Did not detach the thread, main still waiting for it to end execution.\n");
pthread_exit((void *)PTS_FAIL);
return NULL;
}
int main(void)
{
pthread_t new_th;
pthread_attr_t new_attr;
int ret_val;
/* Initializing */
sem1 = INMAIN;
if (pthread_attr_init(&new_attr) != 0) {
perror("Cannot initialize attribute object\n");
return PTS_UNRESOLVED;
}
/* Create a new thread passing it the new attribute object */
if (pthread_create(&new_th, &new_attr, a_thread_func, NULL) != 0) {
perror("Error creating thread\n");
return PTS_UNRESOLVED;
}
/* Wait for thread to indicate that the start routine for the thread has started. */
while (sem1 == INMAIN)
sleep(1);
/* If pthread_detach fails, that means that the test fails as well. */
ret_val = pthread_detach(new_th);
if (ret_val != 0) {
/* Thread is already detached. */
if (ret_val == EINVAL) {
printf("Test FAILED\n");
return PTS_FAIL;
}
/* pthread_detach() failed for another reason. */
else {
printf("Error in pthread_detach(), error: %d\n",
ret_val);
return PTS_UNRESOLVED;
}
}
printf("Test PASSED\n");
printf("Test PASSED 2222 \n");
return PTS_PASS;
}The specific logic is as follows: When the process starts to exit by calling exit(), the main thread will be removed from the group. The call chain is as follows: Then, when the child thread executes sleep(), it will also trigger the exit process: the child thread is removed from the group and its TCB is deleted. At this point, since the TCB and the group are allocated together in memory, the memory space of the group will be deleted along with the TCB. The call chain is as follows: Finally, the process starts to exit by calling up_exit(). The exit process still contains an operation to delete the TCB, resulting in a double free. The call chain is as follows: |
Closed
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
sched/group: move task group into task_tcb_s to improve performance
move task group into task_tcb_s to avoid access allocator to improve performance
for Task Termination, the time consumption will be reduced ~2us (Tricore TC397 300MHZ):
15.97(us) -> 13.55(us)
In interval B:
Signed-off-by: chao an anchao@lixiang.com
Impact
N/A
Testing
ci-check