git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Stephen R. van den Berg" <srb@cuci.nl>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 2/3] git-daemon: make the signal handler almost a no-op
Date: Thu, 14 Aug 2008 02:18:58 +0200	[thread overview]
Message-ID: <20080814001858.GB14939@cuci.nl> (raw)
In-Reply-To: <7v1w0sjw47.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
>"Stephen R. van den Berg" <srb@cuci.nl> writes:
>> by exploiting the fact that systemcalls get interrupted by signals;
>> and even they aren't, all zombies will be collected before the next
>> accept().

>Dscho may want to say something about "even they aren't..." part, after he
>comes back to the keyboard.

That should have read "even if they aren't".  Nonetheless, I don't know
systems where it doesn't work this way, but even if a system resisted,
the problem is mitigated by the fact that we reap the children before
every accept.

>> Fix another error() -> logerror() call.

>which should have been in 1/3, I suppose.

Sort of, yes, it was a bit messy to get it out in one piece.

>> @@ -1036,10 +1034,7 @@ int main(int argc, char **argv)
>>  	gid_t gid = 0;
>>  	int i;

>> -	/* Without this we cannot rely on waitpid() to tell
>> -	 * what happened to our children.
>> -	 */
>> -	signal(SIGCHLD, SIG_DFL);
>> +	child_handler(0);

>Why?

child_handler() now does barely more than setup the signal handler,
which is exactly what we want to do here.

>With your change, the first part happens to be almost no-op, but I do not
>think it justifies this hunk.

>After all, we might even want to do something like:

>	static void child_handler(int signo)
>        {
>        	if (USE_SYSV_SIGNAL_SEMANTICS)
>                	signal(SIGCHLD, child_handler);

>and have the compiler optimize out the signal rearming with

>	cc CFLAGS=-DUSE_SYSV_SIGNAL_SEMANTICS=0

In return I ask: why?
There is no particular performance reason to optimise this.
So in order to keep the code simpler, it might make an extra unneeded
systemcall on some systems when the signal is processed.  I don't think
it's worth our while to optimise this further.
-- 
Sincerely,
           Stephen R. van den Berg.

"And now for something *completely* different!"

  reply	other threads:[~2008-08-14  0:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-13  8:43 [PATCH 1/3] git-daemon: logging done right Stephen R. van den Berg
2008-08-13  8:43 ` [PATCH 2/3] git-daemon: make the signal handler almost a no-op Stephen R. van den Berg
2008-08-14  0:09   ` Junio C Hamano
2008-08-14  0:18     ` Stephen R. van den Berg [this message]
2008-08-14  1:09       ` Junio C Hamano
2008-08-14  7:47         ` Stephen R. van den Berg
2008-08-14  8:26           ` Junio C Hamano
2008-08-14 10:13             ` Stephen R. van den Berg
2008-08-14 13:41   ` Johannes Schindelin
2008-08-13  8:43 ` [PATCH 3/3] git-daemon: rewrite kindergarden Stephen R. van den Berg
2008-08-13  8:58   ` Stephen R. van den Berg
2008-08-13  9:00   ` [PATCH] " Stephen R. van den Berg
2008-08-13 10:40     ` [PATCH] git-daemon: rewrite kindergarden, new option --max-connections Stephen R. van den Berg
2008-08-13  9:05   ` [PATCH 3/3] git-daemon: rewrite kindergarden Petr Baudis
2008-08-13 10:37     ` Stephen R. van den Berg
2008-08-13 23:13 ` [PATCH 1/3] git-daemon: logging done right Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080814001858.GB14939@cuci.nl \
    --to=srb@cuci.nl \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).