You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/04/11 13:06:53 UTC

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #3517: arch: Allocate the space from the beginning in up_stack_frame

patacongo commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r611186107



##########
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.
   
   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.
   
   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.




-- 
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:
users@infra.apache.org