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/07/02 16:55:49 UTC
[GitHub] [incubator-nuttx] no1wudi opened a new pull request #4048: libc: Move pthread_key_destructor to task_info_s
no1wudi opened a new pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048
## Summary
## Impact
## Testing
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048#discussion_r663357537
##########
File path: include/nuttx/tls.h
##########
@@ -63,6 +63,13 @@
struct task_info_s
{
+ sem_t tg_sem;
Review comment:
replace tg_ to ta_
##########
File path: libs/libc/tls/tls_alloc.c
##########
@@ -52,49 +50,54 @@
*
* Returned Value:
* A TLS index that is unique for use within this task group.
+ * If unsuccessful, an errno value will be returned and set to errno.
*
****************************************************************************/
-int tls_alloc(void)
+int tls_alloc(CODE void (*dtor)(FAR void *))
{
- FAR struct tcb_s *rtcb = this_task();
- FAR struct task_group_s *group = rtcb->group;
- irqstate_t flags;
+ FAR struct task_info_s *info = task_get_info();
int candidate;
int ret = -EAGAIN;
- DEBUGASSERT(group != NULL);
+ DEBUGASSERT(info);
- /* Search for an unused index. This is done in a critical section here to
- * avoid concurrent modification of the group TLS index set.
- */
+ ret = _SEM_WAIT(&info->tg_sem);
+
+ if (ERROR == ret)
+ {
+ ret = _SEM_ERRVAL(ret);
+ goto errout_with_errno;
Review comment:
direct return
##########
File path: libs/libc/tls/tls_setvalue.c
##########
@@ -57,15 +57,14 @@
int tls_set_value(int tlsindex, uintptr_t tlsvalue)
{
- FAR struct tls_info_s *info;
+ FAR struct tls_info_s *info = up_tls_info();
Review comment:
don't need change
##########
File path: libs/libc/pthread/pthread_keycreate.c
##########
@@ -76,20 +76,16 @@ int pthread_key_create(FAR pthread_key_t *key,
/* Allocate a TLS index */
- tlsindex = tls_alloc();
+ tlsindex = tls_alloc(destructor);
/* Check if found a TLS index. */
if (tlsindex >= 0)
{
- /* Yes.. Return the key value and success */
-
- *key = (pthread_key_t)tlsindex;
- tls_set_dtor(tlsindex, destructor);
return OK;
}
- return -tlsindex;
Review comment:
revert the change
##########
File path: sched/group/group_create.c
##########
@@ -180,6 +180,10 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
goto errout_with_stream;
}
+ /* Initial user space semaphore */
+
+ nxsem_init(&group->tg_info->tg_sem, 0, 1);
Review comment:
need call nxsem_destroy when release
##########
File path: libs/libc/tls/tls_alloc.c
##########
@@ -52,49 +50,54 @@
*
* Returned Value:
* A TLS index that is unique for use within this task group.
+ * If unsuccessful, an errno value will be returned and set to errno.
*
****************************************************************************/
-int tls_alloc(void)
+int tls_alloc(CODE void (*dtor)(FAR void *))
{
- FAR struct tcb_s *rtcb = this_task();
- FAR struct task_group_s *group = rtcb->group;
- irqstate_t flags;
+ FAR struct task_info_s *info = task_get_info();
int candidate;
int ret = -EAGAIN;
- DEBUGASSERT(group != NULL);
+ DEBUGASSERT(info);
- /* Search for an unused index. This is done in a critical section here to
Review comment:
why remove the comment?
##########
File path: libs/libc/tls/tls_alloc.c
##########
@@ -52,49 +50,54 @@
*
* Returned Value:
* A TLS index that is unique for use within this task group.
+ * If unsuccessful, an errno value will be returned and set to errno.
*
****************************************************************************/
-int tls_alloc(void)
+int tls_alloc(CODE void (*dtor)(FAR void *))
{
- FAR struct tcb_s *rtcb = this_task();
- FAR struct task_group_s *group = rtcb->group;
- irqstate_t flags;
+ FAR struct task_info_s *info = task_get_info();
int candidate;
int ret = -EAGAIN;
- DEBUGASSERT(group != NULL);
+ DEBUGASSERT(info);
- /* Search for an unused index. This is done in a critical section here to
- * avoid concurrent modification of the group TLS index set.
- */
+ ret = _SEM_WAIT(&info->tg_sem);
+
+ if (ERROR == ret)
+ {
+ ret = _SEM_ERRVAL(ret);
+ goto errout_with_errno;
+ }
- flags = spin_lock_irqsave(NULL);
Review comment:
move ret = -EAGAIN from line 61 to here
##########
File path: libs/libc/tls/tls_alloc.c
##########
@@ -52,49 +50,54 @@
*
* Returned Value:
* A TLS index that is unique for use within this task group.
+ * If unsuccessful, an errno value will be returned and set to errno.
*
****************************************************************************/
-int tls_alloc(void)
+int tls_alloc(CODE void (*dtor)(FAR void *))
{
- FAR struct tcb_s *rtcb = this_task();
- FAR struct task_group_s *group = rtcb->group;
- irqstate_t flags;
+ FAR struct task_info_s *info = task_get_info();
int candidate;
int ret = -EAGAIN;
- DEBUGASSERT(group != NULL);
+ DEBUGASSERT(info);
- /* Search for an unused index. This is done in a critical section here to
- * avoid concurrent modification of the group TLS index set.
- */
+ ret = _SEM_WAIT(&info->tg_sem);
+
+ if (ERROR == ret)
Review comment:
ret <0
##########
File path: libs/libc/tls/tls_alloc.c
##########
@@ -52,49 +50,54 @@
*
* Returned Value:
* A TLS index that is unique for use within this task group.
+ * If unsuccessful, an errno value will be returned and set to errno.
*
****************************************************************************/
-int tls_alloc(void)
+int tls_alloc(CODE void (*dtor)(FAR void *))
{
- FAR struct tcb_s *rtcb = this_task();
- FAR struct task_group_s *group = rtcb->group;
- irqstate_t flags;
+ FAR struct task_info_s *info = task_get_info();
int candidate;
int ret = -EAGAIN;
- DEBUGASSERT(group != NULL);
+ DEBUGASSERT(info);
- /* Search for an unused index. This is done in a critical section here to
- * avoid concurrent modification of the group TLS index set.
- */
+ ret = _SEM_WAIT(&info->tg_sem);
+
+ if (ERROR == ret)
+ {
+ ret = _SEM_ERRVAL(ret);
+ goto errout_with_errno;
+ }
- flags = spin_lock_irqsave(NULL);
for (candidate = 0; candidate < CONFIG_TLS_NELEM; candidate++)
{
/* Is this candidate index available? */
tls_ndxset_t mask = (1 << candidate);
- if ((group->tg_tlsset & mask) == 0)
+ if ((info->tg_tlsset & mask) == 0)
{
/* Yes.. allocate the index and break out of the loop */
- group->tg_tlsset |= mask;
+ info->tg_tlsset |= mask;
break;
}
}
- spin_unlock_irqrestore(NULL, flags);
+ _SEM_POST(&info->tg_sem);
/* Check if found a valid TLS data index. */
if (candidate < CONFIG_TLS_NELEM)
{
/* Yes.. Return the TLS index and success */
+ info->tg_tlsdtor[candidate] = dtor;
Review comment:
move after line 82
##########
File path: libs/libc/tls/tls_getvalue.c
##########
@@ -56,7 +56,7 @@
uintptr_t tls_get_value(int tlsindex)
{
- FAR struct tls_info_s *info;
+ FAR struct tls_info_s *info = up_tls_info();
Review comment:
don't need change
##########
File path: include/nuttx/tls.h
##########
@@ -247,7 +255,7 @@ FAR struct tls_info_s *tls_get_info(void);
****************************************************************************/
#if CONFIG_TLS_NELEM > 0
-int tls_set_dtor(int tlsindex, tls_dtor_t destr);
+int tls_set_dtor(int tlsindex, tls_dtor_t dtor);
Review comment:
remove and tls_get_dtor
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048#discussion_r663180800
##########
File path: sched/group/group_create.c
##########
@@ -180,6 +180,10 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
goto errout_with_stream;
}
+ /* Initial user space semaphore */
+
+ _SEM_INIT(&group->tg_info->tg_sem, 0, 1);
Review comment:
change to nxsem_init, _SEM_INIT is designed for the code need be compiled for both kernel and userspace
##########
File path: libs/libc/tls/Make.defs
##########
@@ -21,6 +21,7 @@
CSRCS += task_getinfo.c
ifneq ($(CONFIG_TLS_NELEM),0)
+CSRCS += tls_alloc.c tls_free.c tls_getdtor.c tls_getset.c tls_setdtor.c
Review comment:
Remove tls_getdtor.c and tls_getset.c, it's more simple to call task_get_info directly.
##########
File path: include/nuttx/tls.h
##########
@@ -63,6 +63,15 @@
struct task_info_s
{
+ sem_t tg_sem;
+
+#if CONFIG_TLS_NELEM > 0
+ 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 has tg_tlsdestr and tg_tlsdtor? should we remove the similar field from task_group?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] no1wudi commented on a change in pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048#discussion_r663355377
##########
File path: libs/libc/tls/Make.defs
##########
@@ -21,6 +21,7 @@
CSRCS += task_getinfo.c
ifneq ($(CONFIG_TLS_NELEM),0)
+CSRCS += tls_alloc.c tls_free.c tls_getdtor.c tls_getset.c tls_setdtor.c
Review comment:
Removed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] no1wudi commented on a change in pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048#discussion_r663294192
##########
File path: include/nuttx/tls.h
##########
@@ -63,6 +63,15 @@
struct task_info_s
{
+ sem_t tg_sem;
+
+#if CONFIG_TLS_NELEM > 0
+ tls_ndxset_t tg_tlsset; /* Set of TLS indexes allocated */
+
+ tls_dtor_t tg_tlsdestr[CONFIG_TLS_NELEM]; /* List of TLS destructors */
Review comment:
This mistake is made by cherry-pick, fixed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #4048: libc: Move pthread_key_destructor to task_info_s
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4048:
URL: https://github.com/apache/incubator-nuttx/pull/4048#issuecomment-873226187
@no1wudi Please, fill the required fields in the PR description.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org