This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new a50d87d sched/group: Simplify the allocation and deallocation logic
a50d87d is described below
commit a50d87d5b73c5440399aa4510e06523c57373f40
Author: Xiang Xiao <xiaoxi...@xiaomi.com>
AuthorDate: Wed Mar 2 02:24:05 2022 +0800
sched/group: Simplify the allocation and deallocation logic
1.Move tg_membe allocation to group_alloc
2.Merge group_deallocate to group_release
Signed-off-by: Xiang Xiao <xiaoxi...@xiaomi.com>
Signed-off-by: chao.an <anc...@xiaomi.com>
---
sched/group/group.h | 3 +-
sched/group/group_create.c | 130 ++++++++++++++-------------------------------
sched/group/group_leave.c | 38 ++++++++-----
sched/group/group_waiter.c | 2 +-
sched/init/nx_start.c | 2 +-
sched/task/task_init.c | 15 +-----
sched/task/task_vfork.c | 9 +---
7 files changed, 71 insertions(+), 128 deletions(-)
diff --git a/sched/group/group.h b/sched/group/group.h
index 9436b42..3b9fa98 100644
--- a/sched/group/group.h
+++ b/sched/group/group.h
@@ -73,8 +73,7 @@ void weak_function task_initialize(void);
/* Task group data structure management */
int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype);
-void group_deallocate(FAR struct task_group_s *group);
-int group_initialize(FAR struct task_tcb_s *tcb);
+void group_initialize(FAR struct task_tcb_s *tcb);
#ifndef CONFIG_DISABLE_PTHREAD
int group_bind(FAR struct pthread_tcb_s *tcb);
int group_join(FAR struct pthread_tcb_s *tcb);
diff --git a/sched/group/group_create.c b/sched/group/group_create.c
index f97eba3..acfc79b 100644
--- a/sched/group/group_create.c
+++ b/sched/group/group_create.c
@@ -152,7 +152,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t
ttype)
}
# if defined(CONFIG_FILE_STREAM)
-
/* In a flat, single-heap build. The stream list is allocated with the
* group structure. But in a kernel build with a kernel allocator, it
* must be separately allocated using a user-space allocator.
@@ -163,7 +162,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t
ttype)
group->tg_streamlist = (FAR struct streamlist *)
group_zalloc(group, sizeof(struct streamlist));
-
if (!group->tg_streamlist)
{
goto errout_with_group;
@@ -172,20 +170,29 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t
ttype)
# endif /* defined(CONFIG_FILE_STREAM) */
#endif /* defined(CONFIG_MM_KERNEL_HEAP) */
+#ifdef HAVE_GROUP_MEMBERS
+ /* Allocate space to hold GROUP_INITIAL_MEMBERS members of the group */
+
+ group->tg_members = kmm_malloc(GROUP_INITIAL_MEMBERS * sizeof(pid_t));
+ if (!group->tg_members)
+ {
+ goto errout_with_stream;
+ }
+
+ /* Number of members in allocation */
+
+ group->tg_mxmembers = GROUP_INITIAL_MEMBERS;
+#endif
+
/* Alloc task info for group */
group->tg_info = (FAR struct task_info_s *)
group_zalloc(group, sizeof(struct task_info_s));
-
if (!group->tg_info)
{
- goto errout_with_stream;
+ goto errout_with_member;
}
- /* Initial user space semaphore */
-
- nxsem_init(&group->tg_info->ta_sem, 0, 1);
-
/* Attach the group to the TCB */
tcb->cmn.group = group;
@@ -200,9 +207,13 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t
ttype)
if (ret < 0)
{
tcb->cmn.group = NULL;
- goto errout_with_group;
+ goto errout_with_taskinfo;
}
+ /* Initial user space semaphore */
+
+ nxsem_init(&group->tg_info->ta_sem, 0, 1);
+
/* Initialize file descriptors for the TCB */
files_initlist(&group->tg_filelist);
@@ -232,64 +243,19 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t
ttype)
return OK;
+errout_with_taskinfo:
+ group_free(group, group->tg_info);
+errout_with_member:
+#ifdef HAVE_GROUP_MEMBERS
+ kmm_free(group->tg_members);
errout_with_stream:
+#endif
#if defined(CONFIG_FILE_STREAM) && defined(CONFIG_MM_KERNEL_HEAP)
group_free(group, group->tg_streamlist);
-#endif
errout_with_group:
- group_deallocate(group);
- return ret;
-}
-
-/****************************************************************************
- * Name: group_deallocate
- *
- * Description:
- * Free an existing task group structure.
- *
- * Input Parameters:
- * group = The group structure
- *
- ****************************************************************************/
-
-void group_deallocate(FAR struct task_group_s *group)
-{
- if (group)
- {
-#ifdef CONFIG_ARCH_ADDRENV
- save_addrenv_t oldenv;
-
- /* NOTE: switch the addrenv before accessing group->tg_info
- * located in the userland, also save the current addrenv
- */
-
- up_addrenv_select(&group->tg_addrenv, &oldenv);
-#endif
-
- if (group->tg_info)
- {
- nxsem_destroy(&group->tg_info->ta_sem);
- group_free(group, group->tg_info);
- }
-
-#ifdef CONFIG_ARCH_ADDRENV
- /* Destroy the group address environment */
-
- up_addrenv_destroy(&group->tg_addrenv);
-
- /* Mark no address environment */
-
- g_pid_current = INVALID_PROCESS_ID;
#endif
-
- kmm_free(group);
-
-#ifdef CONFIG_ARCH_ADDRENV
- /* Restore the previous addrenv */
-
- up_addrenv_restore(&oldenv);
-#endif
- }
+ kmm_free(group);
+ return ret;
}
/****************************************************************************
@@ -305,7 +271,7 @@ void group_deallocate(FAR struct task_group_s *group)
* tcb - The tcb in need of the task group.
*
* Returned Value:
- * 0 (OK) on success; a negated errno value on failure.
+ * None.
*
* Assumptions:
* Called during task creation in a safe context. No special precautions
@@ -313,7 +279,7 @@ void group_deallocate(FAR struct task_group_s *group)
*
****************************************************************************/
-int group_initialize(FAR struct task_tcb_s *tcb)
+void group_initialize(FAR struct task_tcb_s *tcb)
{
FAR struct task_group_s *group;
#if defined(HAVE_GROUP_MEMBERS) || defined(CONFIG_ARCH_ADDRENV)
@@ -324,35 +290,9 @@ int group_initialize(FAR struct task_tcb_s *tcb)
group = tcb->cmn.group;
#ifdef HAVE_GROUP_MEMBERS
- /* Allocate space to hold GROUP_INITIAL_MEMBERS members of the group */
-
- group->tg_members = kmm_malloc(GROUP_INITIAL_MEMBERS * sizeof(pid_t));
- if (!group->tg_members)
- {
- group_deallocate(group);
- tcb->cmn.group = NULL;
- return -ENOMEM;
- }
-
/* Assign the PID of this new task as a member of the group. */
group->tg_members[0] = tcb->cmn.pid;
-
- /* Initialize the non-zero elements of group structure and assign it to
- * the tcb.
- */
-
- group->tg_mxmembers = GROUP_INITIAL_MEMBERS; /* Number of members in
allocation */
-
-#endif
-
-#if defined(HAVE_GROUP_MEMBERS) || defined(CONFIG_ARCH_ADDRENV)
- /* Add the initialized entry to the list of groups */
-
- flags = enter_critical_section();
- group->flink = g_grouphead;
- g_grouphead = group;
- leave_critical_section(flags);
#endif
/* Save the ID of the main task within the group of threads. This needed
@@ -366,5 +306,13 @@ int group_initialize(FAR struct task_tcb_s *tcb)
/* Mark that there is one member in the group, the main task */
group->tg_nmembers = 1;
- return OK;
+
+#if defined(HAVE_GROUP_MEMBERS) || defined(CONFIG_ARCH_ADDRENV)
+ /* Add the initialized entry to the list of groups */
+
+ flags = enter_critical_section();
+ group->flink = g_grouphead;
+ g_grouphead = group;
+ leave_critical_section(flags);
+#endif
}
diff --git a/sched/group/group_leave.c b/sched/group/group_leave.c
index d1b52f5..9cfe4f7 100644
--- a/sched/group/group_leave.c
+++ b/sched/group/group_leave.c
@@ -128,10 +128,17 @@ static void group_remove(FAR struct task_group_s *group)
static inline void group_release(FAR struct task_group_s *group)
{
+#ifdef CONFIG_ARCH_ADDRENV
+ save_addrenv_t oldenv;
+#endif
+
#if CONFIG_TLS_TASK_NELEM > 0
task_tls_destruct();
#endif
+ nxsem_destroy(&group->tg_info->ta_sem);
+ group_free(group, group->tg_info);
+
#if defined(CONFIG_SCHED_HAVE_PARENT) && defined(CONFIG_SCHED_CHILD_STATUS)
/* Free all un-reaped child exit status */
@@ -174,17 +181,6 @@ static inline void group_release(FAR struct task_group_s
*group)
shm_group_release(group);
#endif
-#ifdef CONFIG_ARCH_ADDRENV
- /* NOTE:
- * We do not destroy the group address environment here.
- * It will be done in the group_deallocate().
- * However, we mark no address environment here,
- * so that group_addrenv() can work correctly
- */
-
- g_pid_current = INVALID_PROCESS_ID;
-#endif
-
#if defined(HAVE_GROUP_MEMBERS) || defined(CONFIG_ARCH_ADDRENV)
/* Remove the group from the list of groups */
@@ -241,6 +237,24 @@ static inline void group_release(FAR struct task_group_s
*group)
}
#endif
+#ifdef CONFIG_ARCH_ADDRENV
+ /* Switch the addrenv and also save the current addrenv */
+
+ up_addrenv_select(&group->tg_addrenv, &oldenv);
+
+ /* Destroy the group address environment */
+
+ up_addrenv_destroy(&group->tg_addrenv);
+
+ /* Mark no address environment */
+
+ g_pid_current = INVALID_PROCESS_ID;
+
+ /* Restore the previous addrenv */
+
+ up_addrenv_restore(&oldenv);
+#endif
+
#if defined(CONFIG_SCHED_WAITPID) && !defined(CONFIG_SCHED_HAVE_PARENT)
/* If there are threads waiting for this group to be freed, then we cannot
* yet free the memory resources. Instead just mark the group deleted
@@ -256,7 +270,7 @@ static inline void group_release(FAR struct task_group_s
*group)
{
/* Release the group container itself */
- group_deallocate(group);
+ kmm_free(group);
}
}
diff --git a/sched/group/group_waiter.c b/sched/group/group_waiter.c
index 2eda24d..1c4889f 100644
--- a/sched/group/group_waiter.c
+++ b/sched/group/group_waiter.c
@@ -76,7 +76,7 @@ void group_del_waiter(FAR struct task_group_s *group)
* freed).
*/
- group_deallocate(group);
+ kmm_free(group);
}
}
diff --git a/sched/init/nx_start.c b/sched/init/nx_start.c
index a17b97b..60d580f 100644
--- a/sched/init/nx_start.c
+++ b/sched/init/nx_start.c
@@ -588,7 +588,7 @@ void nx_start(void)
* of child status in the IDLE group.
*/
- DEBUGVERIFY(group_initialize(&g_idletcb[i]));
+ group_initialize(&g_idletcb[i]);
g_idletcb[i].cmn.group->tg_flags = GROUP_FLAG_NOCLDWAIT;
}
diff --git a/sched/task/task_init.c b/sched/task/task_init.c
index a777c44..126484b 100644
--- a/sched/task/task_init.c
+++ b/sched/task/task_init.c
@@ -161,20 +161,10 @@ int nxtask_init(FAR struct task_tcb_s *tcb, const char
*name, int priority,
/* Now we have enough in place that we can join the group */
- ret = group_initialize(tcb);
- if (ret == OK)
- {
- return ret;
- }
-
- /* The TCB was added to the inactive task list by
- * nxtask_setup_scheduler().
- */
-
- dq_rem((FAR dq_entry_t *)tcb, (FAR dq_queue_t *)&g_inactivetasks);
+ group_initialize(tcb);
+ return ret;
errout_with_group:
-
if (!stack && tcb->cmn.stack_alloc_ptr)
{
#ifdef CONFIG_BUILD_KERNEL
@@ -196,7 +186,6 @@ errout_with_group:
}
group_leave(&tcb->cmn);
-
return ret;
}
diff --git a/sched/task/task_vfork.c b/sched/task/task_vfork.c
index e49eab1..1642ca4 100644
--- a/sched/task/task_vfork.c
+++ b/sched/task/task_vfork.c
@@ -210,17 +210,10 @@ FAR struct task_tcb_s *nxtask_setup_vfork(start_t retaddr)
/* Now we have enough in place that we can join the group */
- ret = group_initialize(child);
- if (ret < OK)
- {
- goto errout_with_list;
- }
-
+ group_initialize(child);
sinfo("parent=%p, returning child=%p\n", parent, child);
return child;
-errout_with_list:
- dq_rem((FAR dq_entry_t *)child, (FAR dq_queue_t *)&g_inactivetasks);
errout_with_tcb:
nxsched_release_tcb((FAR struct tcb_s *)child, ttype);
errout:
