forked from nuttx/nuttx-update
sched: Fix nx_waitid(), nx_waitpid(), nxtask_exithook() for SMP
Summary: - I noticed waitpid_test stops with lc823450-xgevk:rndis - The condition was CONFIG_DEBUG_ASSERTION=y - Actually, the child task sent SIGCHILD but the parent couldn't catch the signal - Then, I found that nx_waitid(), nx_waitpid() use sched_lock() - However, a parent task and a child task are running on different CPUs - So, sched_lock() is not enough and need to use a critical section - Also, signal handling in nxtask_exithook() must be done in a critical section Impact: - SMP only Testing: - Tested with ostest with the following configurations - lc823450-xgevk:rndis (CONFIG_DEBUG_ASSERTION=y and n) - spresense:smp - spresense:wifi_smp (NCPUS=2 and 4) - sabre-6quad:smp (QEMU) - esp32-core:smp (QEMU) - maix-bit:smp (QEMU) - sim:smp Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
This commit is contained in:
parent
0cf66149ed
commit
4cc38caba9
3 changed files with 53 additions and 4 deletions
|
@ -143,6 +143,14 @@ int nx_waitid(int idtype, id_t id, FAR siginfo_t *info, int options)
|
|||
sigemptyset(&set);
|
||||
nxsig_addset(&set, SIGCHLD);
|
||||
|
||||
/* NOTE: sched_lock() is not enough for SMP
|
||||
* because the child task is running on another CPU
|
||||
*/
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
irqstate_t flags = enter_critical_section();
|
||||
#endif
|
||||
|
||||
/* Disable pre-emption so that nothing changes while the loop executes */
|
||||
|
||||
sched_lock();
|
||||
|
@ -379,6 +387,11 @@ int nx_waitid(int idtype, id_t id, FAR siginfo_t *info, int options)
|
|||
|
||||
errout:
|
||||
sched_unlock();
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
leave_critical_section(flags);
|
||||
#endif
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
@ -72,6 +72,14 @@ pid_t nx_waitpid(pid_t pid, int *stat_loc, int options)
|
|||
|
||||
DEBUGASSERT(stat_loc);
|
||||
|
||||
/* NOTE: sched_lock() is not enough for SMP
|
||||
* because the child task is running on another CPU
|
||||
*/
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
irqstate_t flags = enter_critical_section();
|
||||
#endif
|
||||
|
||||
/* Disable pre-emption so that nothing changes in the following tests */
|
||||
|
||||
sched_lock();
|
||||
|
@ -158,11 +166,15 @@ pid_t nx_waitpid(pid_t pid, int *stat_loc, int options)
|
|||
|
||||
/* On success, return the PID */
|
||||
|
||||
sched_unlock();
|
||||
return pid;
|
||||
ret = pid;
|
||||
|
||||
errout:
|
||||
sched_unlock();
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
leave_critical_section(flags);
|
||||
#endif
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
@ -199,6 +211,14 @@ pid_t nx_waitpid(pid_t pid, int *stat_loc, int options)
|
|||
sigemptyset(&set);
|
||||
nxsig_addset(&set, SIGCHLD);
|
||||
|
||||
/* NOTE: sched_lock() is not enough for SMP
|
||||
* because the child task is running on another CPU
|
||||
*/
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
irqstate_t flags = enter_critical_section();
|
||||
#endif
|
||||
|
||||
/* Disable pre-emption so that nothing changes while the loop executes */
|
||||
|
||||
sched_lock();
|
||||
|
@ -438,11 +458,17 @@ pid_t nx_waitpid(pid_t pid, int *stat_loc, int options)
|
|||
}
|
||||
}
|
||||
|
||||
sched_unlock();
|
||||
return pid;
|
||||
/* On success, return the PID */
|
||||
|
||||
ret = pid;
|
||||
|
||||
errout:
|
||||
sched_unlock();
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
leave_critical_section(flags);
|
||||
#endif
|
||||
|
||||
return ret;
|
||||
}
|
||||
#endif /* CONFIG_SCHED_HAVE_PARENT */
|
||||
|
|
|
@ -634,6 +634,12 @@ void nxtask_exithook(FAR struct tcb_s *tcb, int status, bool nonblocking)
|
|||
|
||||
nxtask_recover(tcb);
|
||||
|
||||
/* NOTE: signal handling needs to be done in a criticl section */
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
irqstate_t flags = enter_critical_section();
|
||||
#endif
|
||||
|
||||
/* Send the SIGCHLD signal to the parent task group */
|
||||
|
||||
nxtask_signalparent(tcb, status);
|
||||
|
@ -668,6 +674,10 @@ void nxtask_exithook(FAR struct tcb_s *tcb, int status, bool nonblocking)
|
|||
|
||||
nxsig_cleanup(tcb); /* Deallocate Signal lists */
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
leave_critical_section(flags);
|
||||
#endif
|
||||
|
||||
/* This function can be re-entered in certain cases. Set a flag
|
||||
* bit in the TCB to not that we have already completed this exit
|
||||
* processing.
|
||||
|
|
Loading…
Reference in a new issue