Re: [systemd-devel] [PATCH] gssd: Improve scalability by not waiting for child processes

[Steve Dickson]

On 10/04/2015 04:19 AM, Florian Weimer wrote:
> * Steve Dickson:
> 
>> +static void
>> +sig_child(int signal)
>> +{
>> +int err;
>> +pid_t pid;
>> +
>> +/* Parent: just wait on child to exit and return */
>> +do {
>> +pid = wait();
>> +} while(pid == -1 && errno != -ECHILD);
>> +
>> +if (WIFSIGNALED(err))
>> +printerr(0, "WARNING: forked child was killed"
>> + "with signal %d\n", WTERMSIG(err));
>> +}
> 
> prinerr calls vfprintf or vsyslog.  Neither is safe to use in signal
> handlers, so you need to log this message in some other way.
Good point... but this patch was self NAK-ed due to it leaving
zombie processes during my testing.

steved.

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] gssd: Improve scalability by not waiting for child processes

[Florian Weimer]

* Steve Dickson:

> +static void
> +sig_child(int signal)
> +{
> + int err;
> + pid_t pid;
> +
> + /* Parent: just wait on child to exit and return */
> + do {
> + pid = wait();
> + } while(pid == -1 && errno != -ECHILD);
> +
> + if (WIFSIGNALED(err))
> + printerr(0, "WARNING: forked child was killed"
> +  "with signal %d\n", WTERMSIG(err));
> +}

prinerr calls vfprintf or vsyslog.  Neither is safe to use in signal
handlers, so you need to log this message in some other way.

Florian
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] gssd: Improve scalability by not waiting for child processes

[Steve Dickson]

Instead of waiting on every fork, which would
become a bottle neck during a mount storm, simply
set a SIGCHLD signal handler to do the wait on
the child process

Signed-off-by: Steve Dickson 
---
 utils/gssd/gssd.c  | 18 ++
 utils/gssd/gssd_proc.c | 11 ++-
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index e480349..8b778cb 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -44,11 +44,13 @@
 #define _GNU_SOURCE
 #endif
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -736,6 +738,21 @@ sig_die(int signal)
printerr(1, "exiting on signal %d\n", signal);
exit(0);
 }
+static void
+sig_child(int signal)
+{
+   int err;
+   pid_t pid;
+
+   /* Parent: just wait on child to exit and return */
+   do {
+   pid = wait();
+   } while(pid == -1 && errno != -ECHILD);
+
+   if (WIFSIGNALED(err))
+   printerr(0, "WARNING: forked child was killed"
+"with signal %d\n", WTERMSIG(err));
+}
 
 static void
 usage(char *progname)
@@ -902,6 +919,7 @@ main(int argc, char *argv[])
 
signal(SIGINT, sig_die);
signal(SIGTERM, sig_die);
+   signal(SIGCHLD, sig_child);
signal_set(_ev, SIGHUP, gssd_scan_cb, NULL);
signal_add(_ev, NULL);
event_set(_ev, inotify_fd, EV_READ | EV_PERSIST, 
gssd_inotify_cb, NULL);
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 11168b2..8f5ca03 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -656,16 +656,9 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int 
fd, char *tgtname,
/* fork() failed! */
printerr(0, "WARNING: unable to fork() to handle"
"upcall: %s\n", strerror(errno));
-   return;
+   /* FALLTHROUGH */
default:
-   /* Parent: just wait on child to exit and return */
-   do {
-   pid = wait();
-   } while(pid == -1 && errno != -ECHILD);
-
-   if (WIFSIGNALED(err))
-   printerr(0, "WARNING: forked child was killed"
-"with signal %d\n", WTERMSIG(err));
+   /* Parent: Return and wait for the SIGCHLD */
return;
}
 no_fork:
-- 
2.4.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel