xiaoxiang781216 commented on a change in pull request #3626:
URL: https://github.com/apache/incubator-nuttx/pull/3626#discussion_r638418101
##########
File path: include/nuttx/sched.h
##########
@@ -504,33 +509,35 @@ struct task_group_s
#endif /* CONFIG_SCHED_HAVE_PARENT */
#if defined(CONFIG_SCHED_WAITPID) && !defined(CONFIG_SCHED_HAVE_PARENT)
- /* waitpid support
************************************************************/
+ /* waitpid support ********************************************************/
- /* Simple mechanism used only when there is no support for SIGCHLD
*/
+ /* Simple mechanism used only when there is no support for SIGCHLD */
- uint8_t tg_nwaiters; /* Number of waiters
*/
- uint8_t tg_waitflags; /* User flags for waitpid behavior
*/
- sem_t tg_exitsem; /* Support for waitpid
*/
- FAR int *tg_statloc; /* Location to return exit status
*/
+ uint8_t tg_nwaiters; /* Number of waiters */
+ uint8_t tg_waitflags; /* User flags for waitpid behavior */
+ sem_t tg_exitsem; /* Support for waitpid */
+ FAR int *tg_statloc; /* Location to return exit status */
#endif
#ifndef CONFIG_DISABLE_PTHREAD
- /* Pthreads
*******************************************************************/
+ /* Pthreads ***************************************************************/
- /* Pthread join Info:
*/
+ /* Pthread join Info: */
- sem_t tg_joinsem; /* Mutually exclusive access to join
data */
- FAR struct join_s *tg_joinhead; /* Head of a list of join data
*/
- FAR struct join_s *tg_jointail; /* Tail of a list of join data
*/
+ sem_t tg_joinsem; /* Mutually exclusive access to join data */
+ FAR struct join_s *tg_joinhead; /* Head of a list of join data */
+ FAR struct join_s *tg_jointail; /* Tail of a list of join data */
#endif
- /* Thread local storage
*******************************************************/
+ /* Thread local storage ***************************************************/
#if CONFIG_TLS_NELEM > 0
- tls_ndxset_t tg_tlsset; /* Set of TLS data indexes allocated
*/
+ tls_ndxset_t tg_tlsset; /* Set of TLS indexes allocated */
+
+ tls_dtor_t tg_tlsdestr[CONFIG_TLS_NELEM]; /* List of TLS destructors */
Review comment:
> > why not save to task_info_s?
>
> Because the destructor is common for all pthreads. It is saved with the
pthread data key is created and used on all threads. So it is a group thing. It
could go in task_info_s, however (But the all of the group data could go into
task_info_s for that matter). I would only move it if it can save system call
and if the job can be done with no critical section.
Yes, this is a good guide.
> Critical sections can only be used within the OS in supervisor mode.
Yes, that's why I just sugest to move tg_tlsdestr to task_info_s, but leave
tg_tlsset in task_group_s, because:
1. tg_tlsset is protected by critical section
2. tg_tlsdestr isn't needed to protect and can remove
tls_get_dtor/tls_set_dtor from syscall
We can even change the protection from critical section to semaphere and
then move tg_tlsset to task_info_s too.
--
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
