Date: Fri, 05 Dec 2008 12:54:40 -0800 From: Julian Elischer <julian@elischer.org> To: Konstantin Belousov <kib@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r185647 - in head/sys: kern sys Message-ID: <49399510.2030104@elischer.org> In-Reply-To: <200812052050.mB5KoOcV072648@svn.freebsd.org> References: <200812052050.mB5KoOcV072648@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin Belousov wrote: > Author: kib > Date: Fri Dec 5 20:50:24 2008 > New Revision: 185647 > URL: http://svn.freebsd.org/changeset/base/185647 > > Log: > Several threads in a process may do vfork() simultaneously. Then, all > parent threads sleep on the parent' struct proc until corresponding > child releases the vmspace. Each sleep is interlocked with proc mutex of > the child, that triggers assertion in the sleepq_add(). The assertion > requires that at any time, all simultaneous sleepers for the channel use > the same interlock. We DID have a thread_single() in the fork code so that only one thread would proceed through the fork itself at a time. However it was removed last year as it proved to be a problem in some cases. Maybe we need to look at replacing it in some way. > > Silent the assertion by using conditional variable allocated in the > child. Broadcast the variable event on exec() and exit(). > > Since struct proc * sleep wait channel is overloaded for several > unrelated events, I was unable to remove wakeups from the places where > cv_broadcast() is added, except exec(). > > Reported and tested by: ganbold > Suggested and reviewed by: jhb > MFC after: 2 week > > Modified: > head/sys/kern/kern_exec.c > head/sys/kern/kern_exit.c > head/sys/kern/kern_fork.c > head/sys/kern/kern_proc.c > head/sys/sys/proc.h > > Modified: head/sys/kern/kern_exec.c > ============================================================================== > --- head/sys/kern/kern_exec.c Fri Dec 5 20:40:02 2008 (r185646) > +++ head/sys/kern/kern_exec.c Fri Dec 5 20:50:24 2008 (r185647) > @@ -609,7 +609,7 @@ interpret: > p->p_flag |= P_EXEC; > if (p->p_pptr && (p->p_flag & P_PPWAIT)) { > p->p_flag &= ~P_PPWAIT; > - wakeup(p->p_pptr); > + cv_broadcast(&p->p_pwait); > } > > /* > > Modified: head/sys/kern/kern_exit.c > ============================================================================== > --- head/sys/kern/kern_exit.c Fri Dec 5 20:40:02 2008 (r185646) > +++ head/sys/kern/kern_exit.c Fri Dec 5 20:50:24 2008 (r185647) > @@ -543,6 +543,7 @@ exit1(struct thread *td, int rv) > * proc lock. > */ > wakeup(p->p_pptr); > + cv_broadcast(&p->p_pwait); > sched_exit(p->p_pptr, td); > PROC_SLOCK(p); > p->p_state = PRS_ZOMBIE; > @@ -774,6 +775,7 @@ loop: > PROC_UNLOCK(p); > tdsignal(t, NULL, SIGCHLD, p->p_ksi); > wakeup(t); > + cv_broadcast(&p->p_pwait); > PROC_UNLOCK(t); > sx_xunlock(&proctree_lock); > return (0); > > Modified: head/sys/kern/kern_fork.c > ============================================================================== > --- head/sys/kern/kern_fork.c Fri Dec 5 20:40:02 2008 (r185646) > +++ head/sys/kern/kern_fork.c Fri Dec 5 20:50:24 2008 (r185647) > @@ -754,7 +754,7 @@ again: > */ > PROC_LOCK(p2); > while (p2->p_flag & P_PPWAIT) > - msleep(p1, &p2->p_mtx, PWAIT, "ppwait", 0); > + cv_wait(&p2->p_pwait, &p2->p_mtx); > PROC_UNLOCK(p2); > > /* > > Modified: head/sys/kern/kern_proc.c > ============================================================================== > --- head/sys/kern/kern_proc.c Fri Dec 5 20:40:02 2008 (r185646) > +++ head/sys/kern/kern_proc.c Fri Dec 5 20:50:24 2008 (r185647) > @@ -231,6 +231,7 @@ proc_init(void *mem, int size, int flags > bzero(&p->p_mtx, sizeof(struct mtx)); > mtx_init(&p->p_mtx, "process lock", NULL, MTX_DEF | MTX_DUPOK); > mtx_init(&p->p_slock, "process slock", NULL, MTX_SPIN | MTX_RECURSE); > + cv_init(&p->p_pwait, "ppwait"); > TAILQ_INIT(&p->p_threads); /* all threads in proc */ > EVENTHANDLER_INVOKE(process_init, p); > p->p_stats = pstats_alloc(); > > Modified: head/sys/sys/proc.h > ============================================================================== > --- head/sys/sys/proc.h Fri Dec 5 20:40:02 2008 (r185646) > +++ head/sys/sys/proc.h Fri Dec 5 20:50:24 2008 (r185647) > @@ -40,6 +40,7 @@ > > #include <sys/callout.h> /* For struct callout. */ > #include <sys/event.h> /* For struct klist. */ > +#include <sys/condvar.h> > #ifndef _KERNEL > #include <sys/filedesc.h> > #endif > @@ -540,6 +541,7 @@ struct proc { > STAILQ_HEAD(, ktr_request) p_ktr; /* (o) KTR event queue. */ > LIST_HEAD(, mqueue_notifier) p_mqnotifier; /* (c) mqueue notifiers.*/ > struct kdtrace_proc *p_dtrace; /* (*) DTrace-specific data. */ > + struct cv p_pwait; /* (*) wait cv for exit/exec */ > }; > > #define p_session p_pgrp->pg_session
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49399510.2030104>