xiaoxiang781216 commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r611188584
##########
File path: sched/task/task_init.c
##########
@@ -130,21 +127,15 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char
*name, int priority,
goto errout_with_group;
}
-#ifndef CONFIG_BUILD_KERNEL
- /* Allocate a stack frame to hold task-specific data */
-
- group = tcb->cmn.group;
- group->tg_libvars = up_stack_frame(&tcb->cmn, sizeof(struct libvars_s));
- DEBUGASSERT(group->tg_libvars != NULL);
-
- /* Initialize the task-specific data */
-
- memset(group->tg_libvars, 0, sizeof(struct libvars_s));
-
- /* Save the allocated task data in TLS */
+ /* Initialize thread local storage */
- tls_set_taskdata(&tcb->cmn);
-#endif
+ tcb->cmn.tls_info_ptr = up_stack_frame(&tcb->cmn,
+ sizeof(struct task_info_s));
+ if (tcb->cmn.tls_info_ptr == NULL)
+ {
+ ret = -ENOMEM;
+ goto errout_with_group;
+ }
Review comment:
> This is difficult to understand. But it looks like you are moving TLS
data from its correct position at the far end of the stack. That is very wrong.
In ALL systems, including NuttX in KERNEL MODE, the TLS data must lie at the
lowest address of the stack.
>
No, TLS is still at the lowest address of the stack, because I modify
up_frame_stack to allocate the memory from the beginning of stack:
https://github.com/apache/incubator-nuttx/pull/3517/files#diff-5c9cf944a960b9718953b2cb399f5ba99fafdc22445ddd6fb9ab27ac1b808ef6R96-R122
> Generally, TLS is obtained very efficiently like:
>
> ```
> tls = (FAR struct tls_info_s *)((builtin_sp()) & TLS_MASK);
> ```
>
> No system call is involved. This standard behavior must be preserved.
>
Yes, the trick still work as before if CONFIG_TLS_ALIGNED turn on.
> In FLAT mode, we can also use aligned stacks, but it is not a good use of
memory. In the FLAT mode, the overhead of a system call is preferable.
>
> But I would reject this change as I understand it because it destroys the
possibility of high perfromance, standard TLS.
I think my reply could resolve your concern now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
