From owner-freebsd-rc@FreeBSD.ORG Mon Dec 21 21:04:52 2009 Return-Path: Delivered-To: freebsd-rc@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 723231065679 for ; Mon, 21 Dec 2009 21:04:52 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: from mail2.fluidhosting.com (mx21.fluidhosting.com [204.14.89.4]) by mx1.freebsd.org (Postfix) with ESMTP id 1BC0F8FC0C for ; Mon, 21 Dec 2009 21:04:52 +0000 (UTC) Received: (qmail 14610 invoked by uid 399); 21 Dec 2009 21:04:50 -0000 Received: from localhost (HELO foreign.dougb.net) (dougb@dougbarton.us@127.0.0.1) by localhost with ESMTPAM; 21 Dec 2009 21:04:50 -0000 X-Originating-IP: 127.0.0.1 X-Sender: dougb@dougbarton.us Message-ID: <4B2FE2E7.8080907@FreeBSD.org> Date: Mon, 21 Dec 2009 13:04:39 -0800 From: Doug Barton Organization: http://SupersetSolutions.com/ User-Agent: Thunderbird 2.0.0.23 (X11/20091206) MIME-Version: 1.0 To: Jilles Tjoelker References: <20091220143623.GC46060@stack.nl> <20091220214846.GA66977@stack.nl> In-Reply-To: <20091220214846.GA66977@stack.nl> X-Enigmail-Version: 0.96.0 OpenPGP: id=D5B2F0FB Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-rc@freebsd.org Subject: Re: [PATCH] use pwait in wait_for_pids X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Dec 2009 21:04:52 -0000 Jilles Tjoelker wrote: > On Sun, Dec 20, 2009 at 03:36:23PM +0100, Jilles Tjoelker wrote: >> Here is a patch to use the new pwait utility in wait_for_pids. > >> This patch still works if pwait is not available, using the old sleep >> method. > >> The redirection on the pwait command serves to squelch "No such process" >> and "pwait: not found" errors. Well, "no such process" would be the result of a race (the thing exited in the microseconds between the 'kill -0' test right above this and actually executing pwait) but I agree we want to protect against it. I'd prefer we don't get a 'pwait: not found' error of course, but I think that optimizing for the common case (it's there and it works) is better in this context then testing for it first. >> The braces are necessary to redirect the >> "not found" error even for sh(1) that doesn't have the fix in r197820 >> (which has not been MFC'ed). Do you plan to MFC it? Personally I would rather wait to update rc.subr with simpler code, completely aside from the inherent value of the fix. It's far more likely that a user will have a base with an up-to-date /bin/sh and pwait and an rc.subr that lag behinds it than the other way around. > Index: etc/rc.subr > =================================================================== > --- etc/rc.subr (revision 200442) > +++ etc/rc.subr (working copy) > @@ -390,7 +390,11 @@ > _list=$_nlist > echo -n ${_prefix:-"Waiting for PIDS: "}$_list > _prefix=", " > - sleep 2 > + if { pwait $_list; } 2>/dev/null; then > + break > + else > + sleep 2 > + fi I would prefer to simplify this down to: pwait $_list 2>/dev/null || sleep 2 for a couple of reasons. First, well, it's simpler. :) Second while I have every confidence that the pwait code will work as advertised, I would prefer to run back through the loop "just in case." In terms of actual performance it will be a very minor pessimization, especially in the most common case where there is only one pid in the list. Does this sound reasonable? Doug -- Improve the effectiveness of your Internet presence with a domain name makeover! http://SupersetSolutions.com/