From 85c788572d054bc2c41b84007875edbd37ad3ed5 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Tue, 8 Mar 2016 23:07:14 -0500 Subject: [PATCH] [SV 46261] Use pselect() for jobserver where supported. * Makefile.am, configure.ac: Check for pselect() and sys/select.h. * main.c (main): Block SIGCHLD if we have pselect() support. * posixos.c (jobserver_acquire): If we support pselect() then use it to query the jobserver pipe, while also listening for SIGCHLD. Also pselect() supports a timeout so avoid alarm() calls. --- Makefile.am | 4 +- configure.ac | 11 +++-- job.c | 13 ++---- main.c | 12 +++++ output.c | 2 +- posixos.c | 130 +++++++++++++++++++++++++++++++++++++++------------ 6 files changed, 126 insertions(+), 46 deletions(-) diff --git a/Makefile.am b/Makefile.am index 8c102a3c..96c4ae20 100644 --- a/Makefile.am +++ b/Makefile.am @@ -16,8 +16,8 @@ # You should have received a copy of the GNU General Public License along with # this program. If not, see . -AUTOMAKE_OPTIONS = 1.8 dist-bzip2 check-news -ACLOCAL_AMFLAGS = -I config +AUTOMAKE_OPTIONS = dist-bzip2 silent-rules std-options +ACLOCAL_AMFLAGS = -I config MAKE_HOST = @MAKE_HOST@ diff --git a/configure.ac b/configure.ac index 7062246a..4e2b6377 100644 --- a/configure.ac +++ b/configure.ac @@ -18,7 +18,7 @@ AC_INIT([GNU make],[4.1.90],[bug-make@gnu.org]) -AC_PREREQ([2.62]) +AC_PREREQ([2.69]) # Autoconf setup AC_CONFIG_AUX_DIR([config]) @@ -29,7 +29,7 @@ AC_CONFIG_HEADERS([config.h]) # We have to enable "foreign" because ChangeLog is auto-generated # We cannot enable -Werror because gettext 0.18.1 has invalid content # When we update gettext to 0.18.3 or better we can add it again. -AM_INIT_AUTOMAKE([1.11.1 silent-rules foreign -Wall]) +AM_INIT_AUTOMAKE([1.15 foreign -Werror -Wall]) # Checks for programs. AC_USE_SYSTEM_EXTENSIONS @@ -51,7 +51,7 @@ AC_ISC_POSIX AC_MINIX # Enable gettext, in "external" mode. -AM_GNU_GETTEXT_VERSION([0.18.1]) +AM_GNU_GETTEXT_VERSION([0.19.4]) AM_GNU_GETTEXT([external]) # This test must come as early as possible after the compiler configuration @@ -68,7 +68,8 @@ AC_HEADER_DIRENT AC_HEADER_STAT AC_HEADER_TIME AC_CHECK_HEADERS([stdlib.h locale.h unistd.h limits.h fcntl.h string.h \ - memory.h sys/param.h sys/resource.h sys/time.h sys/timeb.h]) + memory.h sys/param.h sys/resource.h sys/time.h sys/timeb.h \ + sys/select.h]) AM_PROG_CC_C_O AC_C_CONST @@ -135,7 +136,7 @@ AC_CHECK_FUNCS([strdup strndup mkstemp mktemp fdopen fileno \ dup dup2 getcwd realpath sigsetmask sigaction \ getgroups seteuid setegid setlinebuf setreuid setregid \ getrlimit setrlimit setvbuf pipe strerror strsignal \ - lstat readlink atexit isatty ttyname]) + lstat readlink atexit isatty ttyname pselect]) # We need to check declarations, not just existence, because on Tru64 this # function is not declared without special flags, which themselves cause diff --git a/job.c b/job.c index 084c1b87..be539c7d 100644 --- a/job.c +++ b/job.c @@ -527,10 +527,11 @@ child_error (struct child *child, /* Handle a dead child. This handler may or may not ever be installed. - If we're using the jobserver feature, we need it. First, installing it - ensures the read will interrupt on SIGCHLD. Second, we close the dup'd - read FD to ensure we don't enter another blocking read without reaping all - the dead children. In this case we don't need the dead_children count. + If we're using the jobserver feature without pselect(), we need it. + First, installing it ensures the read will interrupt on SIGCHLD. Second, + we close the dup'd read FD to ensure we don't enter another blocking read + without reaping all the dead children. In this case we don't need the + dead_children count. If we don't have either waitpid or wait3, then make is unreliable, but we use the dead_children count to reap children as best we can. */ @@ -548,10 +549,6 @@ child_handler (int sig UNUSED) /* The signal handler must called only once! */ signal (SIGCHLD, SIG_DFL); #endif - - /* This causes problems if the SIGCHLD interrupts a printf(). - DB (DB_JOBS, (_("Got a SIGCHLD; %u unreaped children.\n"), dead_children)); - */ } extern pid_t shell_function_pid; diff --git a/main.c b/main.c index 8b3dcfa7..fd939a8d 100644 --- a/main.c +++ b/main.c @@ -1870,6 +1870,18 @@ main (int argc, char **argv, char **envp) bsd_signal (SIGCLD, child_handler); # endif } + +#ifdef HAVE_PSELECT + /* If we have pselect() then we need to block SIGCHLD so it's deferred. */ + { + sigset_t block; + sigemptyset (&block); + sigaddset (&block, SIGCHLD); + if (sigprocmask (SIG_SETMASK, &block, NULL) < 0) + pfatal_with_name ("sigprocmask(SIG_SETMASK, SIGCHLD)"); + } +#endif + #endif #endif diff --git a/output.c b/output.c index 4b7bd5da..fef7ab23 100644 --- a/output.c +++ b/output.c @@ -46,7 +46,7 @@ unsigned int stdio_traced = 0; #define OUTPUT_ISSET(_out) ((_out)->out >= 0 || (_out)->err >= 0) -#ifdef HAVE_FCNTL +#ifdef HAVE_FCNTL_H # define STREAM_OK(_s) ((fcntl (fileno (_s), F_GETFD) != -1) || (errno != EBADF)) #else # define STREAM_OK(_s) 1 diff --git a/posixos.c b/posixos.c index 443c1159..7ff75117 100644 --- a/posixos.c +++ b/posixos.c @@ -21,6 +21,9 @@ this program. If not, see . */ #ifdef HAVE_FCNTL_H # include #endif +#if defined(HAVE_PSELECT) && defined(HAVE_SYS_SELECT_H) +# include +#endif #include "debug.h" #include "job.h" @@ -33,7 +36,9 @@ this program. If not, see . */ /* These track the state of the jobserver pipe. Passed to child instances. */ static int job_fds[2] = { -1, -1 }; -/* Used to signal read() that a SIGCHLD happened. Always CLOEXEC. */ +/* Used to signal read() that a SIGCHLD happened. Always CLOEXEC. + If we use pselect() this will never be created and always -1. + */ static int job_rfd = -1; /* Token written to the pipe (could be any character...) */ @@ -42,11 +47,16 @@ static char token = '+'; static int make_job_rfd () { +#ifdef HAVE_PSELECT + /* Pretend we succeeded. */ + return 0; +#else EINTRLOOP (job_rfd, dup (job_fds[0])); if (job_rfd >= 0) CLOSE_ON_EXEC (job_rfd); return job_rfd; +#endif } void @@ -80,19 +90,21 @@ jobserver_parse_arg (const char* arg) DB (DB_JOBS, (_("Jobserver client (fds %d,%d)\n"), job_fds[0], job_fds[1])); -#ifdef HAVE_FCNTL -# define FD_OK(_f) ((fcntl ((_f), F_GETFD) != -1) || (errno != EBADF)) +#ifdef HAVE_FCNTL_H +# define FD_OK(_f) (fcntl ((_f), F_GETFD) != -1) #else # define FD_OK(_f) 1 #endif - /* Create a duplicate pipe, that will be closed in the SIGCHLD handler. If - this fails with EBADF, the parent has closed the pipe on us because it - didn't think we were a submake. If so, warn then default to -j1. */ + /* Make sure our pipeline is valid, and (possibly) create a duplicate pipe, + that will be closed in the SIGCHLD handler. If this fails with EBADF, + the parent has closed the pipe on us because it didn't think we were a + submake. If so, warn and default to -j1. */ + if (!FD_OK (job_fds[0]) || !FD_OK (job_fds[1]) || make_job_rfd () < 0) { if (errno != EBADF) - pfatal_with_name (_("dup jobserver")); + pfatal_with_name (_("jobserver pipeline")); O (error, NILF, _("warning: jobserver unavailable: using -j1. Add '+' to parent make rule.")); @@ -186,7 +198,85 @@ void jobserver_post_child () #endif } -/* The acquire algorithm goes like this (from job.c): +void +jobserver_signal () +{ + if (job_rfd >= 0) + { + close (job_rfd); + job_rfd = -1; + } +} + +void +jobserver_pre_acquire () +{ + /* Make sure we have a dup'd FD. */ + if (job_rfd < 0 && job_fds[0] >= 0 && make_job_rfd () < 0) + pfatal_with_name (_("duping jobs pipe")); +} + +#ifdef HAVE_PSELECT + +/* Use pselect() to atomically wait for both a signal and a file descriptor. + It also provides a timeout facility so we don't need to use SIGALRM. + + This method relies on the fact that SIGCHLD will be blocked everywhere, + and only unblocked (atomically) within the pselect() call, so we can + never miss a SIGCHLD. + */ +int +jobserver_acquire (int timeout) +{ + sigset_t empty; + fd_set readfds; + struct timespec spec; + struct timespec *specp = NULL; + int r; + char intake; + + sigemptyset (&empty); + + FD_ZERO (&readfds); + FD_SET (job_fds[0], &readfds); + + if (timeout) + { + /* Alarm after one second (is this too granular?) */ + spec.tv_sec = 1; + spec.tv_nsec = 0; + specp = &spec; + } + + r = pselect (job_fds[0]+1, &readfds, NULL, NULL, specp, &empty); + + if (r == -1) + { + /* Better be SIGCHLD. */ + if (errno != EINTR) + pfatal_with_name (_("pselect jobs pipe")); + return 0; + } + + if (r == 0) + /* Timeout. */ + return 0; + + /* The read FD is ready: read it! */ + EINTRLOOP (r, read (job_fds[0], &intake, 1)); + if (r < 0) + pfatal_with_name (_("read jobs pipe")); + + /* What does it mean if read() returns 0? It shouldn't happen because only + the master make can reap all the tokens and close the write side...?? */ + return r; +} + +#else + +/* This method uses a "traditional" UNIX model for waiting on both a signal + and a file descriptor. However, it's complex and since we have a SIGCHLD + handler installed we need to check ALL system calls for EINTR: painful! Read a token. As long as there's no token available we'll block. We enable interruptible system calls before the read(2) so that if we get a @@ -264,28 +354,6 @@ set_child_handler_action_flags (int set_handler, int set_alarm) #endif } -void -jobserver_signal () -{ - if (job_rfd >= 0) - { - close (job_rfd); - job_rfd = -1; - } -} - -void -jobserver_pre_acquire () -{ - /* Make sure we have a dup'd FD. */ - if (job_rfd < 0 && job_fds[0] >= 0) - { - DB (DB_JOBS, ("Duplicate the job FD\n")); - if (make_job_rfd () < 0) - pfatal_with_name (_("duping jobs pipe")); - } -} - int jobserver_acquire (int timeout) { @@ -317,4 +385,6 @@ jobserver_acquire (int timeout) return 0; } +#endif + #endif /* MAKE_JOBSERVER */