d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Take out the single-threading code in fork.

Browse Source 
After discussions with jeff, alc, (various Ironport people), david Xu,
and mostly Alfred (who found the problem) it has been demonstrated that this
is not needed for our implementations of threads and represents a real
(as in we've seen it happen a lot) deadlock danger.

Several points:
 Since forking multiple threads is not allowed, and posix states that
 any mutexes owned by othre threads wilol be owned in the child by
 phantom threads, and therads shouldn't ba accessing shared structures without
 protection, It can be proved that if this leads to the child process accessing
 inconsistent data, it's a programming error.

 The mode of thread_single() being used in fork() is the wrong one.
 It is using SINGLE_NO_EXIT when it should be using SINGLE_BOUNDARY.

 Even if this we used, System processes have no need to do it as they have
 no userland to get inconsistent.

  This commmit first fixes the above bugs to get tehm correct in CVS.
  then removes them with #ifdef.
  This is so that history contains the corrected version should it
  be needed in the future.
  This code may be needed if we implement the forkall() syscall from
  Solaris. It may be needed for other non-posix thread libraries
  at some time in the future, so let the code sit for a short while
  while I do some work on it anyhow.

This removes a reproducible lockup in NFS.
It may be argued that maybe doing a fork while holding a vnode lock may
not be the best idea in th efirst place but it shouldn't cause a deadlock.
The removal has been running under soak test for several days now.

This removal should be seriously considered for 7.0 and RELENG_6.

Note. There is code in the core-dumping code that may have a similar problem
with coredumping threaded processes

MFC After: 4 days
This commit is contained in:
Julian Elischer 2007-10-23 17:54:15 +00:00
parent 7f5004e7ba
commit e9271f5376
1 changed files with 15 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
sys/kern/kern_fork.c
View File

@ -208,7 +208,8 @@ fork1(td, flags, pages, procp)
* certain parts of a process from itself.
*/
if ((flags & RFPROC) == 0) {
if ((p1->p_flag & P_HADTHREADS) &&
#if 0 /* XXX no other OS tries to do this */
if (((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) &&
(flags & (RFCFDG | RFFDG))) {
PROC_LOCK(p1);
if (thread_single(SINGLE_BOUNDARY)) {
@ -217,6 +218,7 @@ fork1(td, flags, pages, procp)
}
PROC_UNLOCK(p1);
}
#endif
vm_forkproc(td, NULL, NULL, flags);
@ -236,32 +238,37 @@ fork1(td, flags, pages, procp)
if (flags & RFFDG)
fdunshare(p1, td);
if ((p1->p_flag & P_HADTHREADS) &&
#if 0 /* XXX no other OS tries to do this */
if (((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) &&
(flags & (RFCFDG | RFFDG))) {
PROC_LOCK(p1);
thread_single_end();
PROC_UNLOCK(p1);
}
#endif
*procp = NULL;
return (0);
}
#if 0 /* XXX no other OS tries to do this */
/*
* Note 1:1 allows for forking with one thread coming out on the
* other side with the expectation that the process is about to
* exec.
*/
if (p1->p_flag & P_HADTHREADS) {
if ((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) {
/*
* Systems processes don't need this.
* Idle the other threads for a second.
* Since the user space is copied, it must remain stable.
* In addition, all threads (from the user perspective)
* need to either be suspended or in the kernel,
* where they will try restart in the parent and will
* be aborted in the child.
* keep threadds at the boundary there.
*/
PROC_LOCK(p1);
if (thread_single(SINGLE_NO_EXIT)) {
if (thread_single(SINGLE_BOUNDARY)) {
/* Abort. Someone else is single threading before us. */
PROC_UNLOCK(p1);
return (ERESTART);
@ -273,6 +280,7 @@ fork1(td, flags, pages, procp)
* (or other safe places if we think of any).
*/
}
#endif
/* Allocate new proc. */
newproc = uma_zalloc(proc_zone, M_WAITOK);
@ -721,15 +729,17 @@ fork1(td, flags, pages, procp)
msleep(p1, &p2->p_mtx, PWAIT, "ppwait", 0);
PROC_UNLOCK(p2);
#if 0 /* XXX no other OS tries to do this */
/*
* If other threads are waiting, let them continue now.
*/
if (p1->p_flag & P_HADTHREADS) {
if ((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) {
PROC_LOCK(p1);
thread_single_end();
PROC_UNLOCK(p1);
}
#endif
/*
* Return child proc pointer to parent.
*/