xiaoxiang781216 commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r611661458
##########
File path: include/nuttx/tls.h
##########
@@ -78,10 +78,17 @@ struct tls_info_s
#if CONFIG_TLS_NELEM > 0
uintptr_t tl_elem[CONFIG_TLS_NELEM]; /* TLS elements */
#endif
- FAR struct libvars_s *tl_libvars; /* Task-specific C library data */
int tl_errno; /* Per-thread error number */
};
+struct task_info_s
+{
+ struct tls_info_s ta_tls; /* Must be first field */
+#ifndef CONFIG_BUILD_KERNEL
+ struct getopt_s ta_getopt; /* Globals used by getopt() */
+#endif
Review comment:
> This is incorrect. This mean that there will be a copy of the getopt()
variables in every thread right? But only the copy in the main thread will
used. This seems wrong.
>
No, main thread allocate ```struct task_info_s```:
https://github.com/apache/incubator-nuttx/pull/3517/files#diff-06984fa681053d61fe6d98716085fb7d1e9099deecbb332fa5da35d05b28ee1fR177-R185
https://github.com/apache/incubator-nuttx/pull/3517/files#diff-3e8be9133fa805536122dc95c4303786822b38382e44e053b399e5a43824ae9fR133-R140
pthread allocate ``` struct tls_info_s```:
https://github.com/apache/incubator-nuttx/pull/3517/files#diff-936dfc6e164f9162faee0b703c30f31bea8d6f5a60a6a92b0965f179f7c763a4R308-R315
Since tls_info_s is the first field of task_info_s, all tls specific
function and data work with the main thread. But the main thread could add more
state for task wide state.
> Before your questionable change, there was a single copy of the getopt()
variables in the main thread created by up_stack_frame. Each thread pointed to
that single copied. But it looks like you have ruined that. Your solution is
incorrect.
>
> It is VERY important to restore this to the way it was. The number of
per-task variables will grow a lot (see #3168). It is critical that there be
only a single copy of the per-task data. Please revert this change.
Here is the diagram:
```
Main thread Pthread
/+-------------+ +-------------+
Thread specific< | ta_tls | | tl_elem |
\| | | tl_errno |
/+-------------+ +-------------+
Task specific< | ta_getopt | | |
\+-------------+ | |
| argv | | |
+-------------+ | |
| | | |
| Available | | Available |
| Stack | | Stack |
| | | |
| | | |
| | ^ | | ^
| | | | | |
| | | |
+-------------+ +-------------+
| Task Data* | | Task Data* |
+-------------+ +-------------+
```
--
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
