From owner-freebsd-hackers@FreeBSD.ORG Tue Jun 14 01:39:15 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 98BF4106564A for ; Tue, 14 Jun 2011 01:39:15 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-yw0-f54.google.com (mail-yw0-f54.google.com [209.85.213.54]) by mx1.freebsd.org (Postfix) with ESMTP id 5472F8FC0C for ; Tue, 14 Jun 2011 01:39:15 +0000 (UTC) Received: by ywf7 with SMTP id 7so3378033ywf.13 for ; Mon, 13 Jun 2011 18:39:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=qVF6cwtWJOXwVfQxnIHtFIP44tC19Vfj4O6vQTyMPrM=; b=AhiQnQErKLrn7QwOsc0I7t0nseo5U9N+9mynj+f2qXDmOBM7OWFq7n0B97CsVXWKhy zOw5tOqqkKfnagFRqtqtBa6TWNdkXC0KeiJbnHDG0rsLJwDhdd1yOtOokivLMxE3VPVh 8QZlSQ5wFEPFeDKy37W2s9eNgQEz43gqVnca4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=q26rEuV2rpL7h7/FzBmNZuPV7+nJ7r14GBPJLN229FhDxkDg7KZlLqfHzAFbOIltn0 BQaRzWkUOv2PpkazpsRyrHnE7Fop2OCnj4e8zW61YaMjQsGvQiwMIYAz2zjfjon1Wkux P0G2ctTdxPPlwneAXFHgP40tJi5N3FdA9P7os= MIME-Version: 1.0 Received: by 10.150.74.17 with SMTP id w17mr7011437yba.274.1308015554403; Mon, 13 Jun 2011 18:39:14 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.150.216.3 with HTTP; Mon, 13 Jun 2011 18:39:14 -0700 (PDT) In-Reply-To: <20110613225210.GC65312@stack.nl> References: <20110613225210.GC65312@stack.nl> Date: Tue, 14 Jun 2011 09:39:14 +0800 X-Google-Sender-Auth: fVh2LLjOLnRQgMysZxSYYLroWjY Message-ID: From: Adrian Chadd To: Jilles Tjoelker Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] [RFC] sh(1) vfork support 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: Tue, 14 Jun 2011 01:39:15 -0000 If you're going to go down this path, would you mind making it depend upon the contents of an environment variable? Ie, DO_SH_VFORK=3D"yes|no" Then debugging can occur without having to recompile sh. I ask simply because I've been down this path before in other projects, and trying to debug vfork() vs fork() (and getting users to patch source code to test) became extremely tedious and annoying. 2c, Adrian On 14 June 2011 06:52, Jilles Tjoelker wrote: > The below patch changes sh to use vfork(2) instead of fork(2) in some > simple cases: a foreground simple command, not being a command > substitution, without redirections or assignments in a non-interactive > shell with job control disabled. > > By restricting the use of vfork, different from what NetBSD has done, I > limit what code is executed in the vforked child process. For example, > there is no way opening general redirections in a vforked background > process can work, as opening a fifo may block, and any long-term > blocking of a vforked-but-not-execed process is bad. > > Background simple commands are also excluded because our current > approach of expanding their arguments in the main shell environment is > incorrect (zsh is broken in the same way). > > The restriction to non-interactive shells with job control disabled also > limits the amount of code I have to copy to the vfork code path. > > Command substitutions containing one simple command could also use vfork > but this needs a little extra code. > > The patch does not depend on the memory sharing semantics of vfork. As a > result, however, it calls strerror() (and therefore all sorts of NLS > code) and stdio in the child, in the case that exec fails. Even in the > normal case, malloc() may be called to allocate space for the full > pathname of each executable to be tried. This could be fixed by > reserving enough space for the pathname first and passing the errno > value back through memory and printing the message in the parent process > but this also introduces additional differences with the regular code > path. > > The functions setjmp() and longjmp() are also used but only within a > process; no jump is made by the vforked process outside > vforkexecshell(). > > Simple tests on various microbenchmarks (i386, calling /bin/echo 10000 > times, possibly from multiple processes at once) show a performance > improvement of 20-30%. The gain might be bigger on embedded > architectures where less time has been spent to optimize fork(2). > > Unfortunately, it looks like rc.d may not be helped much by this. It > uses many subshells, most of which require a full fork() with the > current implementation of sh. > > Index: bin/sh/jobs.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- bin/sh/jobs.h =A0 =A0 =A0 (revision 223060) > +++ bin/sh/jobs.h =A0 =A0 =A0 (working copy) > @@ -91,6 +91,7 @@ > =A0void showjobs(int, int); > =A0struct job *makejob(union node *, int); > =A0pid_t forkshell(struct job *, union node *, int); > +pid_t vforkexecshell(struct job *, char **, char **, const char *, int); > =A0int waitforjob(struct job *, int *); > =A0int stoppedjobs(void); > =A0int backgndpidset(void); > Index: bin/sh/eval.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- bin/sh/eval.c =A0 =A0 =A0 (revision 223024) > +++ bin/sh/eval.c =A0 =A0 =A0 (working copy) > @@ -899,6 +899,15 @@ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pipe(pip) < 0) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0error("Pip= e call failed: %s", strerror(errno)); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (cmdentry.cmdtype =3D=3D CMDNORMAL && > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cmd->ncmd.redirect =3D=3D NULL && > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 varlist.list =3D=3D NULL && > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode =3D=3D FORK_FG && > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 !iflag && !mflag) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 vforkexecshell(jp, argv, en= vironment(), path, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cmdentry.u.index); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto parent; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (forkshell(jp, cmd, mode) !=3D 0) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto parent; =A0 =A0/* at = end of routine */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (flags & EV_BACKCMD) { > Index: bin/sh/jobs.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- bin/sh/jobs.c =A0 =A0 =A0 (revision 223060) > +++ bin/sh/jobs.c =A0 =A0 =A0 (working copy) > @@ -57,6 +57,7 @@ > =A0#undef CEOF =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* syntax.h redefin= es this */ > =A0#endif > =A0#include "redir.h" > +#include "exec.h" > =A0#include "show.h" > =A0#include "main.h" > =A0#include "parser.h" > @@ -884,7 +885,48 @@ > =A0} > > > +pid_t > +vforkexecshell(struct job *jp, char **argv, char **envp, const char *pat= h, int idx) > +{ > + =A0 =A0 =A0 pid_t pid; > + =A0 =A0 =A0 struct jmploc jmploc; > + =A0 =A0 =A0 struct jmploc *savehandler; > > + =A0 =A0 =A0 TRACE(("vforkexecshell(%%%td, %p, %d) called\n", jp - jobta= b, (void *)n, > + =A0 =A0 =A0 =A0 =A0 mode)); > + =A0 =A0 =A0 INTOFF; > + =A0 =A0 =A0 flushall(); > + =A0 =A0 =A0 savehandler =3D handler; > + =A0 =A0 =A0 pid =3D vfork(); > + =A0 =A0 =A0 if (pid =3D=3D -1) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 TRACE(("Vfork failed, errno=3D%d\n", errno)= ); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 INTON; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 error("Cannot fork: %s", strerror(errno)); > + =A0 =A0 =A0 } > + =A0 =A0 =A0 if (pid =3D=3D 0) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 TRACE(("Child shell %d\n", (int)getpid())); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (setjmp(jmploc.loc)) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 _exit(exception =3D=3D EXEX= EC ? exerrno : 2); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 handler =3D &jmploc; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 shellexec(argv, envp, path, idx); > + =A0 =A0 =A0 } > + =A0 =A0 =A0 handler =3D savehandler; > + =A0 =A0 =A0 if (jp) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct procstat *ps =3D &jp->ps[jp->nprocs+= +]; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ps->pid =3D pid; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ps->status =3D -1; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ps->cmd =3D nullstr; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 jp->foreground =3D 1; > +#if JOBS > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 setcurjob(jp); > +#endif > + =A0 =A0 =A0 } > + =A0 =A0 =A0 INTON; > + =A0 =A0 =A0 TRACE(("In parent shell: =A0child =3D %d\n", (int)pid)); > + =A0 =A0 =A0 return pid; > +} > + > + > =A0/* > =A0* Wait for job to finish. > =A0* > > -- > Jilles Tjoelker > _______________________________________________ > freebsd-hackers@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org= " >