Date: Mon, 21 Dec 2009 13:04:39 -0800 From: Doug Barton <dougb@FreeBSD.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: freebsd-rc@freebsd.org Subject: Re: [PATCH] use pwait in wait_for_pids Message-ID: <4B2FE2E7.8080907@FreeBSD.org> In-Reply-To: <20091220214846.GA66977@stack.nl> References: <20091220143623.GC46060@stack.nl> <20091220214846.GA66977@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B2FE2E7.8080907>