[PATCH] use pwait in wait_for_pids

Doug Barton dougb at FreeBSD.org
Mon Dec 21 21:04:52 UTC 2009


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/



More information about the freebsd-rc mailing list