From 4e7ee4fc01524432028e073bcac96c67a82bf312 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Fri, 23 Jul 1999 22:46:47 +0000 Subject: [PATCH] * Bugfix for NT archive support. * Rework how the jobserver stuff works. --- ChangeLog | 90 +++++++++++++++++++++++++----- arscan.c | 2 +- configure.in | 40 ++++++++++---- job.c | 177 +++++++++++++++++++++++++++++++++++++---------------------- main.c | 66 ++++++++++++++++------ 5 files changed, 264 insertions(+), 111 deletions(-) diff --git a/ChangeLog b/ChangeLog index 403826f..8177034 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,60 @@ +1999-07-23 Paul D. Smith + + * job.c (my_job_token): This variable controls whether we've + handed our personal token to a subprocess or not. Note we could + probably infer this from the value of job_slots_used, but it's + clearer to just keep it separately. Job_slots_used isn't really + relevant when running the job server. + (free_job_token): New function: free a job token. If we don't + have one, no-op. If we have the personal token, reclaim it. If + we have another token, write it back to the pipe. + (reap_children): Call free_job_token. + (free_child): Call free_job_token. + (start_job_command): Remove duplicate test for '+' in the command. + If we don't appear to be running a recursive make, close the + jobserver filedescriptors. + (start_waiting_job): If our personal token is available, use that + instead of going to the server pipe. + (*): Add the token value to many debugging statements, and print + the child target name in addition to the ptr hex value. + Change the default "no token" value from '\0' to '-' so it looks + better in the output. + + * main.c (main): Install the child_handler with sigaction() + instead of signal() if we have it. On SysV systems, signal() uses + SysV semantics which are a pain. But sigaction() always does what + we want. + (main): If we got job server FDs from the environment, test them + to see if they're open. If not, the parent make closed them + because it didn't think we were a submake. Print a warning and + suggestion to use "+" on the submake invocation, and hard-set to + -j1 for this instance of make. + (main): Change the algorithm for assigning slots to be more + robust. Previously make checked to see if it thought a subprocess + was a submake and if so, didn't give it a token. Since make's + don't consume tokens we could spawn many of makes fighting for a + small number of tokens. Plus this is unreliable because submakes + might not be recognized by the parent (see above) then all the + tokens could be used up by unrecognized makes, and no one could + run. Now every make consumes a token from its parent. However, + the make can also use this token to spawn a child. If the make + wants more than one, it goes to the jobserver pipe. Thus there + will never be more than N makes running for -jN, and N*2 processes + (N makes and their N children). Every make can always run at + least one job, and we'll never deadlock. (Note the closing of the + pipe for non-submakes also solves this, but this is still a better + algorithm.) So! Only put N-1 tokens into the pipe, since the + topmost make keeps one for itself. + + * configure.in: Find sigaction. Disable job server support unless + the system provides it, in addition to either waitpid() or + wait3(). + +1999-07-22 Rob Tulloh + + * arscan.c (ar_member_touch) [WINDOWS32]: The ar_date field is a + string on Windows, not a timestamp. + 1999-07-21 Paul D. Smith * Version 3.77.90 released. @@ -111,21 +168,24 @@ already have one; if we're waiting on the load to go down start_waiting_job() might get called twice on the same file. - * remake.c (update_goal_chain): If we try to update a goal and it - doesn't complete (e.g., parallel builds) remember that by setting - the `deferred' flag in the goal structure. Later when we're - determining the return value we consider a goal updated if either - the mtime has changed _or_ this flag was set. We need this - because the mtime doesn't change during the update_file() function - if we started a job running; instead it's set during the - reap_children() call. So, the code doesn't know it was updated - and returns a status of -1 (nothing done). This is OK during - "normal" builds since our caller (main) treats these cases - identically in that case, but if you're building makefiles the - difference is very important (whether we re-exec or not). - - * dep.h: Add a `deferred' flag to track whether a goal was run but - not completed (parallel builds). + * filedef.h (struct file): Add new field, mtime_before_update. + When notice_finished_file runs it assigns the cached last_mtime to + this field. + * remake.c (update_goal_chain): Notice that a file wasn't updated + by asking if it changed (g->changed) and comparing the current + cached time (last_mtime) with the previous one, stored in + mtime_before_update. The previous check ("did last_mtime changed + during the run of update_file?") fails for parallel builds because + last_mtime is set during reap_children, before update_file is run. + This causes update_goal_chain to always return -1 (nothing + rebuilt) when running parallel (-jN). This is OK during "normal" + builds since our caller (main) treats these cases identically in + that case, but if when rebuilding makefiles the difference is very + important, as it controls whether we re-exec or not. + * file.c (file_hash_enter): Copy the mtime_before_update field. + (snap_deps): Initialize mtime_before_update to -1. + * main.c (main): Initialize mtime_before_update on old (-o) and + new (-W) files. 1999-07-08 Paul D. Smith diff --git a/arscan.c b/arscan.c index b1b7d10..908be7f 100644 --- a/arscan.c +++ b/arscan.c @@ -780,7 +780,7 @@ ar_member_touch (arname, memname) #else fstat (fd, &statbuf); #endif -#if defined(ARFMAG) || defined(ARFZMAG) || defined(AIAMAG) +#if defined(ARFMAG) || defined(ARFZMAG) || defined(AIAMAG) || defined(WINDOWS32) /* Advance member's time to that time */ for (i = 0; i < sizeof ar_hdr.ar_date; i++) ar_hdr.ar_date[i] = ' '; diff --git a/configure.in b/configure.in index 322455f..f2fb61b 100644 --- a/configure.in +++ b/configure.in @@ -56,8 +56,9 @@ AC_MSG_RESULT($ac_cv_check_symbol_$1)])dnl AC_CHECK_LIB(posix4, clock_gettime) AC_CHECK_FUNCS(memmove strdup psignal mktemp pstat_getdynamic \ - clock_gettime dup2 getcwd sigsetmask getgroups setlinebuf \ - seteuid setegid setreuid setregid strerror strsignal pipe) + clock_gettime dup2 getcwd sigsetmask sigaction getgroups \ + setlinebuf seteuid setegid setreuid setregid \ + strerror strsignal pipe) AC_CHECK_SYMBOL(sys_siglist) AC_FUNC_ALLOCA AC_FUNC_VFORK @@ -129,11 +130,16 @@ dnl See if we can handle the job server feature, and if the user wants it. AC_ARG_ENABLE(job-server, [ --disable-job-server Disallow recursive make communication during -jN], - [make_cv_job_server="$enableval"], + [make_cv_job_server="$enableval" user_job_server="$enableval"], [make_cv_job_server="yes"]) -case "$ac_cv_func_pipe $make_cv_job_server" in - "yes yes") AC_DEFINE(MAKE_JOBSERVER) ;; +has_wait_nohang=yes +case "$ac_cv_func_waitpid/$ac_cv_func_wait3" in + no/no) has_wait_nohang=no ;; +esac + +case "$ac_cv_func_pipe/$ac_cv_func_sigaction/$has_wait_nohang/$make_cv_job_server" in + yes/yes/yes/yes) AC_DEFINE(MAKE_JOBSERVER) ;; esac dnl Allow building with dmalloc @@ -232,13 +238,23 @@ case "$with_customs" in fi ;; esac -case "$ac_cv_func_waitpid/$ac_cv_func_wait3" in - no/no) echo - echo "WARNING: Your system has neither waitpid() nor wait3()." - echo " Without one of these, signal handling is unreliable." - echo " You should be aware that running GNU make with -j" - echo " could result in erratic behavior." - echo ;; +case "$has_wait_nohang" in + no) echo + echo "WARNING: Your system has neither waitpid() nor wait3()." + echo " Without one of these, signal handling is unreliable." + echo " You should be aware that running GNU make with -j" + echo " could result in erratic behavior." + echo ;; +esac + +case "$make_cv_job_server/$user_job_server" in + no/yes) echo + echo "WARNING: Make job server requires a POSIX-ish system that" + echo " supports the pipe(), sigaction(), and either" + echo " waitpid() or wait3() functions. Your system doesn't" + echo " appear to provide one or more of those." + echo " Disabling job server support." + echo ;; esac dnl Local Variables: diff --git a/job.c b/job.c index 57c3202..ed8f64b 100644 --- a/job.c +++ b/job.c @@ -191,6 +191,11 @@ unsigned int job_slots_used = 0; static int good_stdin_used = 0; +/* Specifies whether the current process's reserved job token is in use. + '+' means it's available, '-' means it isn't. */ + +static char my_job_token = '+'; + /* Chain of children waiting to run until the load average goes down. */ static struct child *waiting_jobs = 0; @@ -198,6 +203,9 @@ static struct child *waiting_jobs = 0; /* Non-zero if we use a *real* shell (always so on Unix). */ int unixy_shell = 1; + +/* #define debug_flag 1 */ + #ifdef WINDOWS32 /* @@ -209,6 +217,40 @@ int w32_kill(int pid, int sig) } #endif /* WINDOWS32 */ + +#ifndef MAKE_JOBSERVER +# define free_job_token(c) +#else +static void +free_job_token (child) + struct child *child; +{ + switch (child->job_token) + { + case '-': + /* If this child doesn't have a token, punt. */ + return; + + case '+': + /* If this child has the reserved token, take it back. */ + my_job_token = '+'; + break; + + default: + /* Write any other job tokens back to the pipe. */ + write (job_fds[1], &child->job_token, 1); + break; + } + + if (debug_flag) + printf ("Released token `%c' for child 0x%08lx (%s).\n", + child->job_token, (unsigned long int) child, child->file->name); + + child->job_token = '-'; +} +#endif + + /* Write an error message describing the exit status given in EXIT_CODE, EXIT_SIG, and COREDUMP, for the target TARGET_NAME. Append "(ignored)" if IGNORED is nonzero. */ @@ -342,9 +384,10 @@ reap_children (block, err) any_remote |= c->remote; any_local |= ! c->remote; if (debug_flag) - printf ("Live child 0x%08lx PID %ld%s\n", - (unsigned long int) c, - (long) c->pid, c->remote ? " (remote)" : ""); + printf ("Live child 0x%08lx (%s) PID %ld token %c%s\n", + (unsigned long int) c, c->file->name, + (long) c->pid, c->job_token, + c->remote ? " (remote)" : ""); #ifdef VMS break; #endif @@ -507,10 +550,10 @@ reap_children (block, err) else { if (debug_flag) - printf ("Reaping %s child 0x%08lx PID %ld%s\n", + printf ("Reaping %s child 0x%08lx PID %ld token %c%s\n", child_failed ? "losing" : "winning", - (unsigned long int) c, - (long) c->pid, c->remote ? " (remote)" : ""); + (unsigned long int) c, (long) c->pid, c->job_token, + c->remote ? " (remote)" : ""); if (c->sh_batch_file) { if (debug_flag) @@ -607,9 +650,9 @@ reap_children (block, err) notice_finished_file (c->file); if (debug_flag) - printf ("Removing child 0x%08lx PID %ld%s from chain.\n", - (unsigned long int) c, - (long) c->pid, c->remote ? " (remote)" : ""); + printf ("Removing child 0x%08lx PID %ld token %c%s from chain.\n", + (unsigned long int) c, (long) c->pid, c->job_token, + c->remote ? " (remote)" : ""); /* Block fatal signals while frobnicating the list, so that children and job_slots_used are always consistent. Otherwise @@ -618,18 +661,9 @@ reap_children (block, err) live and call reap_children again. */ block_sigs (); -#ifdef MAKE_JOBSERVER /* If this job has a token out, return it. */ - if (c->job_token) - { - assert(job_slots_used > 0); - write (job_fds[1], &c->job_token, 1); - if (debug_flag) - printf ("Released token `%c' for child 0x%08lx.\n", - c->job_token, (unsigned long int) c); - c->job_token = 0; - } -#endif + free_job_token(c); + /* There is now another slot open. */ if (job_slots_used > 0) --job_slots_used; @@ -680,18 +714,10 @@ free_child (child) free ((char *) child->environment); } -#ifdef MAKE_JOBSERVER /* If this child has a token it hasn't relinquished, give it up now. This can happen if the job completes immediately, mainly because all the command lines evaluated to empty strings. */ - if (child->job_token) - { - write (job_fds[1], &child->job_token, 1); - if (debug_flag) - printf ("Freed token `%c' for child 0x%08lx.\n", - child->job_token, (unsigned long int) child); - } -#endif + free_job_token(child); free ((char *) child); } @@ -764,7 +790,7 @@ start_job_command (child) flags |= COMMANDS_RECURSE; else if (*p == '-') child->noerror = 1; - else if (!isblank (*p) && *p != '+') + else if (!isblank (*p)) break; ++p; } @@ -986,6 +1012,15 @@ start_job_command (child) { /* We are the child side. */ unblock_sigs (); + + /* If we aren't running a recursive command and we have a jobserver + pipe, close it before exec'ing. */ + if (!(flags & COMMANDS_RECURSE) && job_fds[0] >= 0) + { + close (job_fds[0]); + close (job_fds[1]); + } + child_execute_job (child->good_stdin ? 0 : bad_stdin, 1, argv, child->environment); } @@ -1139,47 +1174,54 @@ start_waiting_job (c) if (!c->remote) { #ifdef MAKE_JOBSERVER - /* If this is not a recurse command and we are controlling - multiple jobs, and we don't yet have one, obtain a token before - starting child. */ - if (job_fds[0] >= 0 && !f->cmds->any_recurse && !c->job_token) - { - fd_set rfds; + /* If we are controlling multiple jobs, and we don't yet have one, + * obtain a token before starting the child. */ + if (job_fds[0] >= 0) + { + while (c->job_token == '-') + /* If the reserved token is available, just use that. */ + if (my_job_token != '-') + { + c->job_token = my_job_token; + my_job_token = '-'; + } + /* Read a token. We set the non-blocking bit on this earlier, so + if there's no token to be read we'll return immediately. */ + else if (read (job_fds[0], &c->job_token, 1) < 1) + { + fd_set rfds; + int r; - FD_ZERO(&rfds); - FD_SET(job_fds[0], &rfds); + FD_ZERO(&rfds); + FD_SET(job_fds[0], &rfds); - /* Read a token. We set the non-blocking bit on this earlier, - so if there's no token to be read we'll fall through to the - select. The select blocks until (a) there's data to read, - in which case we come back around and try to grab the token - before someone else does, or (b) a signal, such as SIGCHLD, - is caught (because we installed a handler for it). If the - latter, call reap_children() to try to free up some slots. */ + /* The select blocks until (a) there's data to read, in which + case we come back around and try to grab the token before + someone else does, or (b) a signal (SIGCHLD), is reported + (because we installed a handler for it). If the latter, + call reap_children() to try to free up some slots. */ - while (read (job_fds[0], &c->job_token, 1) < 1) - { - int r = select (job_fds[0]+1, SELECT_FD_SET_CAST &rfds, - NULL, NULL, NULL); + r = select (job_fds[0]+1, SELECT_FD_SET_CAST &rfds, + NULL, NULL, NULL); - if (r < 0) - { + if (r < 0) + { #ifdef EINTR - if (errno != EINTR) - /* We should definitely handle this more gracefully! - What kinds of things can happen here? ^C closes the - pipe? Something else closes it? */ - pfatal_with_name ("read jobs pipe"); + if (errno != EINTR) + /* We should definitely handle this more gracefully! + What kinds of things can happen here? ^C closes the + pipe? Something else closes it? */ + pfatal_with_name ("read jobs pipe"); #endif - /* We were interrupted; handle any dead children. */ - reap_children (1, 0); - } - } + /* We were interrupted; handle any dead children. */ + reap_children (1, 0); + } + } - assert(c->job_token != 0); + assert(c->job_token != '-'); if (debug_flag) - printf ("Obtained token `%c' for child 0x%08lx.\n", - c->job_token, (unsigned long int) c); + printf ("Obtained token `%c' for child 0x%08lx (%s).\n", + c->job_token, (unsigned long int) c, c->file->name); } #endif /* If we are running at least one job already and the load average @@ -1203,9 +1245,10 @@ start_waiting_job (c) case cs_running: c->next = children; if (debug_flag) - printf ("Putting child 0x%08lx PID %ld%s on the chain.\n", - (unsigned long int) c, - (long) c->pid, c->remote ? " (remote)" : ""); + printf ("Putting child 0x%08lx (%s) PID %ld token %c%s on the chain.\n", + (unsigned long int) c, c->file->name, + (long) c->pid, c->job_token, + c->remote ? " (remote)" : ""); children = c; /* One more job slot is in use. */ ++job_slots_used; @@ -1368,7 +1411,7 @@ new_job (file) c->command_ptr = 0; c->environment = 0; c->sh_batch_file = NULL; - c->job_token = 0; + c->job_token = '-'; /* Fetch the first command line to be run. */ job_next_command (c); diff --git a/main.c b/main.c index 1950956..f2ec3c9 100644 --- a/main.c +++ b/main.c @@ -1172,19 +1172,32 @@ int main (int argc, char ** argv) functionality here and rely on the signal handler and counting children. - Also, if we're using the jobs pipe we need a signal handler so that + If we're using the jobs pipe we need a signal handler so that SIGCHLD is not ignored; we need it to interrupt select(2) in - job.c:start_waiting_job() if we're waiting on the pipe for a token. */ + job.c:start_waiting_job() if we're waiting on the pipe for a token. + + Use sigaction where possible as it's more reliable. */ { extern RETSIGTYPE child_handler PARAMS ((int sig)); /* Set up to handle children dying. This must be done before reading in the makefiles so that `shell' function calls will work. */ + +#ifndef HAVE_SIGACTION +# define HANDLESIG(s) signal(s, child_handler) +#else +# define HANDLESIG(s) sigaction(s, &sa, NULL) + struct sigaction sa; + + bzero ((char *)&sa, sizeof (struct sigaction)); + sa.sa_handler = child_handler; +#endif + # if defined SIGCHLD - (void) signal (SIGCHLD, child_handler); + (void) HANDLESIG (SIGCHLD); # endif # if defined SIGCLD && SIGCLD != SIGCHLD - (void) signal (SIGCLD, child_handler); + (void) HANDLESIG (SIGCLD); # endif } #endif @@ -1267,21 +1280,38 @@ int main (int argc, char ** argv) #ifdef MAKE_JOBSERVER /* If extended jobs are available then the -j option can have one of 4 - formats: (1) not specified: default is "1"; (2) specified with no - value: default is "0" (infinite); (3) specified with a single - value: this means the user wants N job slots; or (4) specified with - 2 values separated by commas. The latter means we're a submake and - the two values are the read and write FDs, respectively, for the - pipe. Note this last form is undocumented for the user! - */ + formats: (1) not specified: default is "1"; (2) specified with no value: + default is "0" (infinite); (3) specified with a single value: this means + the user wants N job slots; or (4) specified with 2 values separated by + a comma. The latter means we're a submake; the two values are the read + and write FDs, respectively, for the pipe. Note this last form is + undocumented for the user! */ + sscanf(job_slots_str, "%d", &job_slots); { char *cp = index(job_slots_str, ','); + /* In case #4, get the FDs. */ if (cp && sscanf(cp+1, "%d", &job_fds[1]) == 1) { - job_fds[0] = job_slots; - job_slots = 0; + /* Set up the first FD and set job_slots to 0. The combination of a + pipe + !job_slots means we're using the jobserver. If !job_slots + and we don't have a pipe, we can start infinite jobs. */ + job_fds[0] = job_slots; + job_slots = 0; + + /* Make sure the pipe is open! The parent might have closed it + because it didn't think we were a submake. If so, print a warning + then default to -j1. */ + if (fcntl (job_fds[0], F_GETFL, 0) < 0 + || fcntl (job_fds[1], F_GETFL, 0) < 0) + { + error (NILF, + "warning: jobserver unavailable (using -j1). Add `+' to parent make rule."); + job_slots = 1; + job_fds[0] = job_fds[1] = -1; + job_slots_str = "1"; + } } } @@ -1293,14 +1323,18 @@ int main (int argc, char ** argv) char buf[(sizeof("1024")*2)+1]; char c = '0'; - if (pipe(job_fds) < 0) - pfatal_with_name("creating jobs pipe"); + if (pipe (job_fds) < 0) + pfatal_with_name ("creating jobs pipe"); /* Set the read FD to nonblocking; we'll use select() to wait for it in job.c. */ fcntl (job_fds[0], F_SETFL, O_NONBLOCK); - for (; job_slots; --job_slots) + /* Every make assumes that it always has one job it can run. For the + submakes it's the token they were given by their parent. For the + top make, we just subtract one from the number the user wants. */ + + while (--job_slots) { write(job_fds[1], &c, 1); if (c == '9') -- 2.7.4