From owner-freebsd-hackers@FreeBSD.ORG Mon Aug 13 11:50:30 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6F26E1065672; Mon, 13 Aug 2012 11:50:30 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 838FF8FC0C; Mon, 13 Aug 2012 11:50:29 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q7DBoYsl086109; Mon, 13 Aug 2012 14:50:34 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q7DBoL1Z005567; Mon, 13 Aug 2012 14:50:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q7DBoLkm005566; Mon, 13 Aug 2012 14:50:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 13 Aug 2012 14:50:21 +0300 From: Konstantin Belousov To: David Xu Message-ID: <20120813115021.GE2352@deviant.kiev.zoral.com.ua> References: <20120730102408.GA19983@stack.nl> <20120730105303.GU2676@deviant.kiev.zoral.com.ua> <20120805215432.GA28704@stack.nl> <20120806082535.GI2676@deviant.kiev.zoral.com.ua> <20120809105648.GA79814@stack.nl> <20120809110850.GA2425@deviant.kiev.zoral.com.ua> <20120810101302.GF2425@deviant.kiev.zoral.com.ua> <5026F4B1.5030000@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="idY8LE8SD6/8DnRI" Content-Disposition: inline In-Reply-To: <5026F4B1.5030000@gmail.com> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-hackers@freebsd.org, davidxu@freebsd.org, Jilles Tjoelker Subject: Re: system() using vfork() or posix_spawn() and libthr X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Aug 2012 11:50:30 -0000 --idY8LE8SD6/8DnRI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Aug 12, 2012 at 08:11:29AM +0800, David Xu wrote: > On 2012/08/10 18:13, Konstantin Belousov wrote: > >On Thu, Aug 09, 2012 at 02:08:50PM +0300, Konstantin Belousov wrote: > >>Third alternative, which seems to be even better, is to restore > >>single-threading of the parent for vfork(). > single-threading is slow for large threaded process, don't know if it > is necessary for vfork(), POSIX says nothing about threaded process. I agree that with both of your statements. But, being fast but allowing silent process corruption is not good behaviour. Either we need to actually support vfork() for threaded processes, or disable it with some error code. I prefer to support it. I believe that vfork() should be wrapped by libthr in the same way as fork() is wrapped. Not sure should we call atfork handlers, for now I decided not to call, since the handlers assume separate address spaces for parent/child. But we could only call parent handler in child, however weird this sounds. The real complication with wrapping is the fact that we cannot return from wrapper in child without destroying parent state. So I tried to prototype the code to handle the wrapping in the same frame, thus neccessity of using asm. Below is WIP, only for amd64 ATM. diff --git a/lib/libthr/arch/amd64/Makefile.inc b/lib/libthr/arch/amd64/Mak= efile.inc index e6d99ec..476d26a 100644 --- a/lib/libthr/arch/amd64/Makefile.inc +++ b/lib/libthr/arch/amd64/Makefile.inc @@ -1,3 +1,4 @@ #$FreeBSD$ =20 -SRCS+=3D pthread_md.c _umtx_op_err.S +CFLAGS+=3D-I${.CURDIR}/../libc/${MACHINE_CPUARCH} +SRCS+=3D pthread_md.c _umtx_op_err.S vfork.S diff --git a/lib/libthr/arch/amd64/amd64/vfork.S b/lib/libthr/arch/amd64/am= d64/vfork.S new file mode 100644 index 0000000..07d813d --- /dev/null +++ b/lib/libthr/arch/amd64/amd64/vfork.S @@ -0,0 +1,74 @@ +/*- + * Copyright (c) 2012 Konstantin Belousov + * Copyright (c) 1990 The Regents of the University of California. + * All rights reserved. + * + * This code is derived from software contributed to Berkeley by + * William Jolitz. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 4. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURP= OSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENT= IAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STR= ICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W= AY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#if defined(SYSLIBC_SCCS) && !defined(lint) + .asciz "@(#)Ovfork.s 5.1 (Berkeley) 4/23/90" +#endif /* SYSLIBC_SCCS and not lint */ +#include +__FBSDID("$FreeBSD$"); + +#include "SYS.h" + + .weak _vfork + .set _vfork,__sys_vfork + .weak vfork + .set vfork,__sys_vfork +ENTRY(__sys_vfork) + call _thr_vfork_pre + popq %rsi /* fetch return address (%rsi preserved) */ + mov $SYS_vfork,%rax + KERNCALL + jb 2f + cmpl $0,%eax + jne 1f + pushq %rsi + pushq %rsi /* twice for stack alignment */ + call _thr_vfork_post + popq %rsi + popq %rsi + xorl %eax,%eax +1: + jmp *%rsi +2: + pushq %rsi + pushq %rax + call _thr_vfork_post + call PIC_PLT(CNAME(__error)) + popq %rdx + movq %rdx,(%rax) + movq $-1,%rax + movq $-1,%rdx + retq +END(__sys_vfork) + + .section .note.GNU-stack,"",%progbits diff --git a/lib/libthr/pthread.map b/lib/libthr/pthread.map index 355edea..40d14b4 100644 --- a/lib/libthr/pthread.map +++ b/lib/libthr/pthread.map @@ -157,6 +157,7 @@ FBSD_1.0 { system; tcdrain; usleep; + vfork; wait; wait3; wait4; diff --git a/lib/libthr/thread/Makefile.inc b/lib/libthr/thread/Makefile.inc index ddde6e9..92e82ac 100644 --- a/lib/libthr/thread/Makefile.inc +++ b/lib/libthr/thread/Makefile.inc @@ -55,4 +55,5 @@ SRCS+=3D \ thr_switch_np.c \ thr_symbols.c \ thr_umtx.c \ + thr_vfork.c \ thr_yield.c diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_privat= e.h index ba272fe..30f3de2 100644 --- a/lib/libthr/thread/thr_private.h +++ b/lib/libthr/thread/thr_private.h @@ -777,6 +777,8 @@ int _thr_setscheduler(lwpid_t, int, const struct sched_= param *) __hidden; void _thr_signal_prefork(void) __hidden; void _thr_signal_postfork(void) __hidden; void _thr_signal_postfork_child(void) __hidden; +void _thr_vfork_pre(void) __hidden; +void _thr_vfork_post(void) __hidden; void _thr_try_gc(struct pthread *, struct pthread *) __hidden; int _rtp_to_schedparam(const struct rtprio *rtp, int *policy, struct sched_param *param) __hidden; @@ -833,6 +835,7 @@ pid_t __sys_getpid(void); ssize_t __sys_read(int, void *, size_t); ssize_t __sys_write(int, const void *, size_t); void __sys_exit(int); +pid_t __sys_vfork(void); #endif =20 static inline int diff --git a/lib/libthr/thread/thr_vfork.c b/lib/libthr/thread/thr_vfork.c new file mode 100644 index 0000000..bed7db5 --- /dev/null +++ b/lib/libthr/thread/thr_vfork.c @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2012 Konstantin Belousov + * Copyright (c) 2005 David Xu + * Copyright (c) 2003 Daniel Eischen + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Neither the name of the author nor the names of any co-contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURP= OSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENT= IAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STR= ICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W= AY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * + * $FreeBSD$ + */ + +/* + * Copyright (c) 1995-1998 John Birrell + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the author nor the names of any co-contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY JOHN BIRRELL AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURP= OSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENT= IAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STR= ICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W= AY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include "namespace.h" +#include +#include +#include +#include +#include +#include +#include +#include "un-namespace.h" + +#include "libc_private.h" +#include "rtld_lock.h" +#include "thr_private.h" + +static int rtld_locks[MAX_RTLD_LOCKS]; +static int cancelsave, was_threaded; + +void +_thr_vfork_pre(void) +{ + struct pthread *curthread; + + curthread =3D _get_curthread(); + _thr_rwl_rdlock(&_thr_atfork_lock); + cancelsave =3D curthread->no_cancel; + curthread->no_cancel =3D 1; + _thr_signal_block(curthread); + _thr_signal_prefork(); + if (_thr_isthreaded() !=3D 0) { + was_threaded =3D 1; + _malloc_prefork(); + _rtld_atfork_pre(rtld_locks); + } else + was_threaded =3D 0; +} + +void +_thr_vfork_post(void) +{ + struct pthread *curthread; + + _thr_signal_postfork(); + if (was_threaded) { + _rtld_atfork_post(rtld_locks); + _malloc_postfork(); + } + curthread =3D _get_curthread(); + _thr_signal_unblock(curthread); + curthread->no_cancel =3D cancelsave; + _thr_rwlock_unlock(&_thr_atfork_lock); + if (curthread->cancel_async) + _thr_testcancel(curthread); +} diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 6cb95cd..8735c25 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -150,11 +150,7 @@ sys_vfork(struct thread *td, struct vfork_args *uap) int error, flags; struct proc *p2; =20 -#ifdef XEN - flags =3D RFFDG | RFPROC; /* validate that this is still an issue */ -#else flags =3D RFFDG | RFPROC | RFPPWAIT | RFMEM; -#endif =09 error =3D fork1(td, flags, 0, &p2, NULL, 0); if (error =3D=3D 0) { td->td_retval[0] =3D p2->p_pid; @@ -756,7 +752,7 @@ fork1(struct thread *td, int flags, int pages, struct p= roc **procp, struct thread *td2; struct vmspace *vm2; vm_ooffset_t mem_charged; - int error; + int error, single_threaded; static int curfail; static struct timeval lastfail; #ifdef PROCDESC @@ -815,6 +811,19 @@ fork1(struct thread *td, int flags, int pages, struct = proc **procp, } #endif =20 + if (((p1->p_flag & (P_HADTHREADS | P_SYSTEM)) =3D=3D P_HADTHREADS) && + (flags & RFPPWAIT) !=3D 0) { + PROC_LOCK(p1); + if (thread_single(SINGLE_BOUNDARY)) { + PROC_UNLOCK(p1); + error =3D ERESTART; + goto fail2; + } + PROC_UNLOCK(p1); + single_threaded =3D 1; + } else + single_threaded =3D 0; + mem_charged =3D 0; vm2 =3D NULL; if (pages =3D=3D 0) @@ -945,6 +954,12 @@ fail1: if (vm2 !=3D NULL) vmspace_free(vm2); uma_zfree(proc_zone, newproc); + if (single_threaded) { + PROC_LOCK(p1); + thread_single_end(); + PROC_UNLOCK(p1); + } +fail2: #ifdef PROCDESC if (((flags & RFPROCDESC) !=3D 0) && (fp_procdesc !=3D NULL)) { fdclose(td->td_proc->p_fd, fp_procdesc, *procdescp, td); --idY8LE8SD6/8DnRI Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAlAo6f0ACgkQC3+MBN1Mb4jgmACgyP388z/lJaYP1qBUqT3O14+Q JuwAoNF6duwiBrmMBwAPXD+fu6sCH36l =Kcl3 -----END PGP SIGNATURE----- --idY8LE8SD6/8DnRI--