You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2024/03/07 04:39:36 UTC
(nuttx) 02/02: sched/group: change type of task group member to single queue
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit ec08031e4bf01f0abc9e590484382c166d90915e
Author: chao an <an...@lixiang.com>
AuthorDate: Wed Mar 6 10:13:47 2024 +0800
sched/group: change type of task group member to single queue
Change the type of task group member to single list chain to
avoid accessing the memory allocator to improve the performance
Signed-off-by: chao an <an...@lixiang.com>
---
binfmt/binfmt_execmodule.c | 2 +-
fs/procfs/fs_procfsproc.c | 16 +++--
include/nuttx/sched.h | 10 ++-
sched/clock/clock_gettime.c | 30 +++++----
sched/group/group_create.c | 33 ++--------
sched/group/group_foreachchild.c | 18 ++++--
sched/group/group_join.c | 127 ++-------------------------------------
sched/group/group_killchildren.c | 2 +-
sched/group/group_leave.c | 121 +++----------------------------------
sched/task/task_exithook.c | 6 +-
10 files changed, 72 insertions(+), 293 deletions(-)
diff --git a/binfmt/binfmt_execmodule.c b/binfmt/binfmt_execmodule.c
index 85978b411b..0f5fffc0a6 100644
--- a/binfmt/binfmt_execmodule.c
+++ b/binfmt/binfmt_execmodule.c
@@ -121,7 +121,7 @@ static void exec_swap(FAR struct tcb_s *ptcb, FAR struct tcb_s *chtcb)
pid_t pid;
irqstate_t flags;
#ifdef HAVE_GROUP_MEMBERS
- FAR pid_t *tg_members;
+ sq_queue_t tg_members;
#endif
#ifdef CONFIG_SCHED_HAVE_PARENT
# ifdef CONFIG_SCHED_CHILD_STATUS
diff --git a/fs/procfs/fs_procfsproc.c b/fs/procfs/fs_procfsproc.c
index c6b7145c0f..4f5911e130 100644
--- a/fs/procfs/fs_procfsproc.c
+++ b/fs/procfs/fs_procfsproc.c
@@ -43,6 +43,7 @@
# include <time.h>
#endif
+#include <nuttx/nuttx.h>
#include <nuttx/irq.h>
#include <nuttx/tls.h>
#include <nuttx/sched.h>
@@ -53,6 +54,7 @@
#include <nuttx/fs/procfs.h>
#include <nuttx/fs/ioctl.h>
#include <nuttx/mm/mm.h>
+#include <nuttx/queue.h>
#if !defined(CONFIG_SCHED_CPULOAD_NONE) || defined(CONFIG_SCHED_CRITMONITOR)
# include <nuttx/clock.h>
@@ -1122,7 +1124,8 @@ static ssize_t proc_groupstatus(FAR struct proc_file_s *procfile,
size_t copysize;
size_t totalsize;
#ifdef HAVE_GROUP_MEMBERS
- int i;
+ FAR sq_entry_t *curr;
+ FAR sq_entry_t *next;
#endif
DEBUGASSERT(group != NULL);
@@ -1168,13 +1171,14 @@ static ssize_t proc_groupstatus(FAR struct proc_file_s *procfile,
buffer += copysize;
remaining -= copysize;
+#ifdef HAVE_GROUP_MEMBERS
if (totalsize >= buflen)
{
return totalsize;
}
- linesize = procfs_snprintf(procfile->line, STATUS_LINELEN, "%-12s%d\n",
- "Members:", group->tg_nmembers);
+ linesize = procfs_snprintf(procfile->line, STATUS_LINELEN, "%-12s%zu\n",
+ "Members:", sq_count(&group->tg_members));
copysize = procfs_memcpy(procfile->line, linesize, buffer,
remaining, &offset);
@@ -1182,7 +1186,6 @@ static ssize_t proc_groupstatus(FAR struct proc_file_s *procfile,
buffer += copysize;
remaining -= copysize;
-#ifdef HAVE_GROUP_MEMBERS
if (totalsize >= buflen)
{
return totalsize;
@@ -1202,10 +1205,11 @@ static ssize_t proc_groupstatus(FAR struct proc_file_s *procfile,
return totalsize;
}
- for (i = 0; i < group->tg_nmembers; i++)
+ sq_for_every_safe(&group->tg_members, curr, next)
{
+ tcb = container_of(curr, struct tcb_s, member);
linesize = procfs_snprintf(procfile->line, STATUS_LINELEN, " %d",
- group->tg_members[i]);
+ tcb->pid);
copysize = procfs_memcpy(procfile->line, linesize, buffer,
remaining, &offset);
diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h
index cfc0eb9c8b..879619874d 100644
--- a/include/nuttx/sched.h
+++ b/include/nuttx/sched.h
@@ -429,10 +429,8 @@ struct task_group_s
/* Group membership *******************************************************/
- uint8_t tg_nmembers; /* Number of members in the group */
#ifdef HAVE_GROUP_MEMBERS
- uint8_t tg_mxmembers; /* Number of members in allocation */
- FAR pid_t *tg_members; /* Members of the group */
+ sq_queue_t tg_members; /* List of members for task */
#endif
#ifdef CONFIG_BINFMT_LOADABLE
@@ -538,6 +536,12 @@ struct tcb_s
FAR struct task_group_s *group; /* Pointer to shared task group data */
+ /* Group membership *******************************************************/
+
+#ifdef HAVE_GROUP_MEMBERS
+ sq_entry_t member; /* List entry of task member */
+#endif
+
/* Address Environment ****************************************************/
#ifdef CONFIG_ARCH_ADDRENV
diff --git a/sched/clock/clock_gettime.c b/sched/clock/clock_gettime.c
index 0dffa8ae13..808ca7aefa 100644
--- a/sched/clock/clock_gettime.c
+++ b/sched/clock/clock_gettime.c
@@ -30,9 +30,11 @@
#include <errno.h>
#include <debug.h>
+#include <nuttx/nuttx.h>
#include <nuttx/arch.h>
#include <nuttx/sched.h>
#include <nuttx/spinlock.h>
+#include <nuttx/queue.h>
#include "clock/clock.h"
#ifdef CONFIG_CLOCK_TIMEKEEPING
@@ -164,11 +166,14 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp)
}
else if (clock_type == CLOCK_PROCESS_CPUTIME_ID)
{
- FAR struct task_group_s *group;
unsigned long runtime;
- irqstate_t flags;
- int i;
FAR struct tcb_s *tcb;
+# ifdef HAVE_GROUP_MEMBERS
+ FAR struct task_group_s *group;
+ FAR sq_entry_t *curr;
+ FAR sq_entry_t *next;
+ irqstate_t flags;
+# endif
if (pid == 0)
{
@@ -183,20 +188,23 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp)
if (tcb != NULL)
{
+# ifdef HAVE_GROUP_MEMBERS
group = tcb->group;
runtime = 0;
- flags = enter_critical_section();
- for (i = group->tg_nmembers - 1; i >= 0; i--)
+ flags = spin_lock_irqsave(NULL);
+ sq_for_every_safe(&group->tg_members, curr, next)
{
- tcb = nxsched_get_tcb(group->tg_members[i]);
- if (tcb != NULL)
- {
- runtime += tcb->run_time;
- }
+ tcb = container_of(curr, struct tcb_s, member);
+
+ runtime += tcb->run_time;
}
- leave_critical_section(flags);
+ spin_unlock_irqrestore(NULL, flags);
+# else /* HAVE_GROUP_MEMBERS */
+ runtime = tcb->run_time;
+# endif /* HAVE_GROUP_MEMBERS */
+
perf_convert(runtime, tp);
}
else
diff --git a/sched/group/group_create.c b/sched/group/group_create.c
index c14660f6c7..01262d64bf 100644
--- a/sched/group/group_create.c
+++ b/sched/group/group_create.c
@@ -39,14 +39,6 @@
#include "group/group.h"
#include "tls/tls.h"
-/****************************************************************************
- * Pre-processor Definitions
- ****************************************************************************/
-
-/* Is this worth making a configuration option? */
-
-#define GROUP_INITIAL_MEMBERS 4
-
/****************************************************************************
* Public Data
****************************************************************************/
@@ -147,18 +139,9 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
#endif /* defined(CONFIG_MM_KERNEL_HEAP) */
#ifdef HAVE_GROUP_MEMBERS
- /* Allocate space to hold GROUP_INITIAL_MEMBERS members of the group */
+ /* Initialize member list of the group */
- group->tg_members = kmm_malloc(GROUP_INITIAL_MEMBERS * sizeof(pid_t));
- if (!group->tg_members)
- {
- ret = -ENOMEM;
- goto errout_with_group;
- }
-
- /* Number of members in allocation */
-
- group->tg_mxmembers = GROUP_INITIAL_MEMBERS;
+ sq_init(&group->tg_members);
#endif
/* Attach the group to the TCB */
@@ -178,7 +161,7 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
ret = task_init_info(group);
if (ret < 0)
{
- goto errout_with_member;
+ goto errout_with_group;
}
#ifndef CONFIG_DISABLE_PTHREAD
@@ -195,11 +178,7 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype)
return OK;
-errout_with_member:
-#ifdef HAVE_GROUP_MEMBERS
- kmm_free(group->tg_members);
errout_with_group:
-#endif
kmm_free(group);
return ret;
}
@@ -240,7 +219,7 @@ void group_initialize(FAR struct task_tcb_s *tcb)
#ifdef HAVE_GROUP_MEMBERS
/* Assign the PID of this new task as a member of the group. */
- group->tg_members[0] = tcb->cmn.pid;
+ sq_addlast(&tcb->cmn.member, &group->tg_members);
#endif
/* Save the ID of the main task within the group of threads. This needed
@@ -250,8 +229,4 @@ void group_initialize(FAR struct task_tcb_s *tcb)
*/
group->tg_pid = tcb->cmn.pid;
-
- /* Mark that there is one member in the group, the main task */
-
- group->tg_nmembers = 1;
}
diff --git a/sched/group/group_foreachchild.c b/sched/group/group_foreachchild.c
index 81761ba6e8..827b53a651 100644
--- a/sched/group/group_foreachchild.c
+++ b/sched/group/group_foreachchild.c
@@ -25,7 +25,9 @@
#include <nuttx/config.h>
#include <assert.h>
+#include <nuttx/nuttx.h>
#include <nuttx/sched.h>
+#include <nuttx/queue.h>
#include "group/group.h"
@@ -58,23 +60,27 @@
int group_foreachchild(FAR struct task_group_s *group,
foreachchild_t handler, FAR void *arg)
{
+ FAR sq_entry_t *curr;
+ FAR sq_entry_t *next;
int ret;
- int i;
DEBUGASSERT(group);
/* Visit the main thread last (if present) */
- for (i = group->tg_nmembers - 1; i >= 0; i--)
+ sq_for_every_safe(&group->tg_members, curr, next)
{
- ret = handler(group->tg_members[i], arg);
- if (ret != 0)
+ FAR struct tcb_s *mtcb =
+ container_of(curr, struct tcb_s, member);
+
+ ret = handler(mtcb->pid, arg);
+ if (ret != OK)
{
- return ret;
+ break;
}
}
- return 0;
+ return ret;
}
#endif /* HAVE_GROUP_MEMBERS */
diff --git a/sched/group/group_join.c b/sched/group/group_join.c
index 42d82310f1..48befc1d3b 100644
--- a/sched/group/group_join.c
+++ b/sched/group/group_join.c
@@ -38,102 +38,6 @@
#ifndef CONFIG_DISABLE_PTHREAD
-/****************************************************************************
- * Pre-processor Definitions
- ****************************************************************************/
-
-/* Is this worth making a configuration option? */
-
-#define GROUP_REALLOC_MEMBERS 4
-
-/****************************************************************************
- * Private Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: group_addmember
- *
- * Description:
- * Add a new member to a group.
- *
- * Input Parameters:
- * group - The task group to add the new member
- * pid - The new member
- *
- * Returned Value:
- * 0 (OK) on success; a negated errno value on failure.
- *
- * Assumptions:
- * Called during thread creation and during reparenting in a safe context.
- * No special precautions are required here.
- *
- ****************************************************************************/
-
-#ifdef HAVE_GROUP_MEMBERS
-static inline int group_addmember(FAR struct task_group_s *group, pid_t pid)
-{
- FAR pid_t *oldmembers = NULL;
- irqstate_t flags;
-
- DEBUGASSERT(group && group->tg_nmembers < UINT8_MAX);
-
- /* Will we need to extend the size of the array of groups? */
-
- if (group->tg_nmembers >= group->tg_mxmembers)
- {
- FAR pid_t *newmembers;
- unsigned int newmax;
-
- /* Yes... reallocate the array of members */
-
- newmax = group->tg_mxmembers + GROUP_REALLOC_MEMBERS;
- if (newmax > UINT8_MAX)
- {
- newmax = UINT8_MAX;
- }
-
- newmembers = kmm_malloc(sizeof(pid_t) * newmax);
-
- if (!newmembers)
- {
- serr("ERROR: Failed to reallocate tg_members\n");
- return -ENOMEM;
- }
-
- /* Save the new number of members in the reallocated members array.
- * We need to make the following atomic because the member list
- * may be traversed from an interrupt handler (read-only).
- */
-
- flags = spin_lock_irqsave(NULL);
- memcpy(newmembers, group->tg_members,
- sizeof(pid_t) * group->tg_mxmembers);
- oldmembers = group->tg_members;
- group->tg_members = newmembers;
- group->tg_mxmembers = newmax;
- }
- else
- {
- flags = spin_lock_irqsave(NULL);
- }
-
- /* Assign this new pid to the group; group->tg_nmembers will be incremented
- * by the caller.
- */
-
- group->tg_members[group->tg_nmembers] = pid;
- group->tg_nmembers++;
- spin_unlock_irqrestore(NULL, flags);
-
- if (oldmembers != NULL)
- {
- kmm_free(oldmembers);
- }
-
- return OK;
-}
-#endif /* HAVE_GROUP_MEMBERS */
-
/****************************************************************************
* Public Functions
****************************************************************************/
@@ -142,12 +46,7 @@ static inline int group_addmember(FAR struct task_group_s *group, pid_t pid)
* Name: group_bind
*
* Description:
- * A thread joins the group when it is created. This is a two step process,
- * first, the group must bound to the new threads TCB. group_bind() does
- * this (at the return from group_join, things are a little unstable: The
- * group has been bound, but tg_nmembers has not yet been incremented).
- * Then, after the new thread is initialized and has a PID assigned to it,
- * group_join() is called, incrementing the tg_nmembers count on the group.
+ * A thread joins the group when it is created.
*
* Input Parameters:
* tcb - The TCB of the new "child" task that need to join the group.
@@ -179,12 +78,7 @@ int group_bind(FAR struct pthread_tcb_s *tcb)
* Name: group_join
*
* Description:
- * A thread joins the group when it is created. This is a two step process,
- * first, the group must bound to the new threads TCB. group_bind() does
- * this (at the return from group_join, things are a little unstable: The
- * group has been bound, but tg_nmembers has not yet been incremented).
- * Then, after the new thread is initialized and has a PID assigned to it,
- * group_join() is called, incrementing the tg_nmembers count on the group.
+ * A thread joins the group when it is created.
*
* Input Parameters:
* tcb - The TCB of the new "child" task that need to join the group.
@@ -203,32 +97,19 @@ int group_bind(FAR struct pthread_tcb_s *tcb)
int group_join(FAR struct pthread_tcb_s *tcb)
{
FAR struct task_group_s *group;
-#ifdef HAVE_GROUP_MEMBERS
- int ret;
-#else
irqstate_t flags;
-#endif
- DEBUGASSERT(tcb && tcb->cmn.group &&
- tcb->cmn.group->tg_nmembers < UINT8_MAX);
+ DEBUGASSERT(tcb && tcb->cmn.group);
/* Get the group from the TCB */
group = tcb->cmn.group;
-#ifdef HAVE_GROUP_MEMBERS
/* Add the member to the group */
- ret = group_addmember(group, tcb->cmn.pid);
- if (ret < 0)
- {
- return ret;
- }
-#else
flags = spin_lock_irqsave(NULL);
- group->tg_nmembers++;
+ sq_addfirst(&tcb->cmn.member, &group->tg_members);
spin_unlock_irqrestore(NULL, flags);
-#endif
return OK;
}
diff --git a/sched/group/group_killchildren.c b/sched/group/group_killchildren.c
index bafe734071..7807af43ec 100644
--- a/sched/group/group_killchildren.c
+++ b/sched/group/group_killchildren.c
@@ -191,7 +191,7 @@ int group_kill_children(FAR struct tcb_s *tcb)
ret = CONFIG_GROUP_KILL_CHILDREN_TIMEOUT_MS;
while (1)
{
- if (tcb->group->tg_nmembers <= 1)
+ if (sq_empty(&tcb->group->tg_members))
{
break;
}
diff --git a/sched/group/group_leave.c b/sched/group/group_leave.c
index 89f8e6ee68..04d4134aac 100644
--- a/sched/group/group_leave.c
+++ b/sched/group/group_leave.c
@@ -105,16 +105,6 @@ static inline void group_release(FAR struct task_group_s *group)
mm_map_destroy(&group->tg_mm_map);
-#ifdef HAVE_GROUP_MEMBERS
- /* Release the members array */
-
- if (group->tg_members)
- {
- kmm_free(group->tg_members);
- group->tg_members = NULL;
- }
-#endif
-
#ifdef CONFIG_BINFMT_LOADABLE
/* If the exiting task was loaded into RAM from a file, then we need to
* lease all of the memory resource when the last thread exits the task
@@ -137,58 +127,6 @@ static inline void group_release(FAR struct task_group_s *group)
group_drop(group);
}
-/****************************************************************************
- * Name: group_removemember
- *
- * Description:
- * Remove a member from a group.
- *
- * Input Parameters:
- * group - The group from which to remove the member.
- * pid - The member to be removed.
- *
- * Returned Value:
- * On success, returns the number of members remaining in the group (>=0).
- * Can fail only if the member is not found in the group. On failure,
- * returns -ENOENT
- *
- * Assumptions:
- * Called during task deletion and also from the reparenting logic, both
- * in a safe context. No special precautions are required here.
- *
- ****************************************************************************/
-
-#ifdef HAVE_GROUP_MEMBERS
-static inline void group_removemember(FAR struct task_group_s *group,
- pid_t pid)
-{
- irqstate_t flags;
- int i;
-
- DEBUGASSERT(group);
-
- /* Find the member in the array of members and remove it */
-
- for (i = 0; i < group->tg_nmembers; i++)
- {
- /* Does this member have the matching pid */
-
- if (group->tg_members[i] == pid)
- {
- /* Remove the member from the array of members. This must be an
- * atomic operation because the member array may be accessed from
- * interrupt handlers (read-only).
- */
-
- flags = enter_critical_section();
- group->tg_members[i] = group->tg_members[group->tg_nmembers - 1];
- group->tg_nmembers--;
- leave_critical_section(flags);
- }
- }
-}
-#endif /* HAVE_GROUP_MEMBERS */
-
/****************************************************************************
* Public Functions
****************************************************************************/
@@ -214,10 +152,12 @@ static inline void group_removemember(FAR struct task_group_s *group,
*
****************************************************************************/
-#ifdef HAVE_GROUP_MEMBERS
void group_leave(FAR struct tcb_s *tcb)
{
FAR struct task_group_s *group;
+#ifdef HAVE_GROUP_MEMBERS
+ irqstate_t flags;
+#endif
DEBUGASSERT(tcb);
@@ -226,17 +166,17 @@ void group_leave(FAR struct tcb_s *tcb)
group = tcb->group;
if (group)
{
- /* Remove the member from group. This function may be called
- * during certain error handling before the PID has been
- * added to the group. In this case tcb->pid will be uninitialized
- * group_removemember() will fail.
- */
+ /* Remove the member from group. */
- group_removemember(group, tcb->pid);
+#ifdef HAVE_GROUP_MEMBERS
+ flags = spin_lock_irqsave(NULL);
+ sq_rem(&tcb->member, &group->tg_members);
+ spin_unlock_irqrestore(NULL, flags);
/* Have all of the members left the group? */
- if (group->tg_nmembers == 0)
+ if (sq_empty(&group->tg_members))
+#endif
{
/* Yes.. Release all of the resource held by the task group */
@@ -251,47 +191,6 @@ void group_leave(FAR struct tcb_s *tcb)
}
}
-#else /* HAVE_GROUP_MEMBERS */
-
-void group_leave(FAR struct tcb_s *tcb)
-{
- FAR struct task_group_s *group;
-
- DEBUGASSERT(tcb);
-
- /* Make sure that we have a group */
-
- group = tcb->group;
- if (group)
- {
- /* Yes, we have a group.. Is this the last member of the group? */
-
- if (group->tg_nmembers > 1)
- {
- /* No.. just decrement the number of members in the group */
-
- group->tg_nmembers--;
- }
-
- /* Yes.. that was the last member remaining in the group */
-
- else
- {
- /* Release all of the resource held by the task group */
-
- group_release(group);
- }
-
- /* In any event, we can detach the group from the TCB so we won't do
- * this again.
- */
-
- tcb->group = NULL;
- }
-}
-
-#endif /* HAVE_GROUP_MEMBERS */
-
/****************************************************************************
* Name: group_drop
*
diff --git a/sched/task/task_exithook.c b/sched/task/task_exithook.c
index b8afa5e977..0a586f8908 100644
--- a/sched/task/task_exithook.c
+++ b/sched/task/task_exithook.c
@@ -174,7 +174,7 @@ static inline void nxtask_sigchild(pid_t ppid, FAR struct tcb_s *ctcb,
* should generate SIGCHLD.
*/
- if (chgrp->tg_nmembers == 1)
+ if (sq_is_singular(&chgrp->tg_members))
{
/* Mark that all of the threads in the task group have exited */
@@ -360,7 +360,9 @@ static inline void nxtask_exitwakeup(FAR struct tcb_s *tcb, int status)
/* Is this the last thread in the group? */
- if (group->tg_nmembers == 1)
+#ifndef CONFIG_DISABLE_PTHREAD
+ if (sq_is_singular(&group->tg_members))
+#endif
{
/* Yes.. Wakeup any tasks waiting for this task to exit */