[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #7966: group/addrenv: Fix the removal of address environments when a process exits

[GitBox <gi...@apache.org>]

xiaoxiang781216 commented on code in PR #7966:
URL: https://github.com/apache/nuttx/pull/7966#discussion_r1059963499


##########
include/nuttx/sched.h:
##########
@@ -1054,6 +1058,30 @@ FAR struct task_tcb_s *nxtask_setup_vfork(start_t retaddr);
 pid_t nxtask_start_vfork(FAR struct task_tcb_s *child);
 void nxtask_abort_vfork(FAR struct task_tcb_s *child, int errcode);
 
+/****************************************************************************
+ * Name: group_addrenv_attach
+ *
+ * Description:
+ *   Attach address environment to a newly created group. Called by exec()
+ *   right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ *   tcb     - The tcb of the newly loaded task.
+ *   addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ *   This is a NuttX internal function so it follows the convention that
+ *   0 (OK) is returned on success and a negated errno is returned on
+ *   failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+struct group_addrenv_s;

Review Comment:
   don't need?



##########
include/nuttx/sched.h:
##########
@@ -525,6 +526,9 @@ struct tcb_s
 
   /* Task Group *************************************************************/
 
+#ifdef CONFIG_ARCH_ADDRENV
+  FAR struct task_group_s *mm_group;   /* Active memory mappings group      */

Review Comment:
   why need mm_group and group?



##########
sched/group/group_leave.c:
##########
@@ -408,3 +376,66 @@ void group_leave(FAR struct tcb_s *tcb)
 }
 
 #endif /* HAVE_GROUP_MEMBERS */
+
+/****************************************************************************
+ * Name: group_drop
+ *
+ * Description:
+ *   Release the group's memory. This function is called whenever a reference
+ *   to the group structure is released. It is not dependent on member count,
+ *   but rather external references, which include:
+ *   - Waiter list for waitpid()
+ *   - Users of the group's address environment, which can be the group's
+ *     members themselves, as well as kernel tasks whom do not have their own
+ *     address environment.
+ *
+ * Input Parameters:
+ *   group - The group that is to be dropped
+ *
+ * Returned Value:
+ *   None.
+ *
+ * Assumptions:
+ *   Called during task deletion or context switch in a safe context.  No
+ *   special precautions are required here.
+ *
+ ****************************************************************************/
+
+void group_drop(FAR struct task_group_s *group)
+{
+  /* Dummy initial condition */
+
+  if (0)

Review Comment:
   let's remove the dummy if:
   ```
   #if defined(CONFIG_SCHED_WAITPID) && !defined(CONFIG_SCHED_HAVE_PARENT)
     if (group->tg_nwaiters > 0)
       {
       }
     else
   #endif
   #if defined(CONFIG_ARCH_ADDRENV)
     /* If someone needs this address environment, cannot drop it yet */
     if (group->tg_addrenv_refs > 0)
       {
       }
     else
   #endif
     /* Finally, if no one needs the group and it has been deleted, remove it */
     if (group->tg_flags & GROUP_FLAG_DELETED)
       {
       }
   ```



##########
sched/group/group.h:
##########
@@ -80,6 +69,7 @@ int  group_bind(FAR struct pthread_tcb_s *tcb);
 int  group_join(FAR struct pthread_tcb_s *tcb);
 #endif
 void group_leave(FAR struct tcb_s *tcb);
+void group_drop(FAR struct task_group_s *group);

Review Comment:
   why we have both group_drop and group_addrenv_drop



-- 
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