From ae2e9cefd8487db4cf4918e36b821340a83e76ca Mon Sep 17 00:00:00 2001 From: Matt Birkholz Date: Sat, 20 Dec 2014 14:38:24 -0700 Subject: [PATCH] smp: share: uxproc.o --- README.txt | 53 ++++++++++++++++++++++----- src/microcode/uxproc.c | 82 ++++++++++++++++++++++++++++++++---------- 2 files changed, 108 insertions(+), 27 deletions(-) diff --git a/README.txt b/README.txt index 9d2666cfd..73f4bea5f 100644 --- a/README.txt +++ b/README.txt @@ -735,14 +735,51 @@ command line. The remaining 12 belong to the 7 microcode modules and entries exist before their indices are valid. uxproc.o: - 00000004 C OS_process_table_size - 00000004 b foreground_child_process - 00000020 b grabbed_signal_mask - 00000004 C process_table - 00000008 b process_tick - 00000000 b scheme_ctty_fd - 00000004 C scheme_jc_status - 0000000c b sync_tick + 00000004 C OS_process_table_size locked + 00000004 b foreground_child_process single-threaded, console owner + 00000020 b grabbed_signal_mask eliminated + 00000004 C process_table locked + 00000008 b process_tick locked + 00000000 b scheme_ctty_fd single-threaded, console owner + 00000004 C scheme_jc_status read-only, UX_initialize_processes + 0000000c b sync_tick single-threaded, I/O waiter + + OK. process_table works like channel_table, and is locked up + in the same way. One wrinkle: the SIGCHLD handler wants to + modify a process_table entry member and so should grab the + process_table_mutex. If a SIGCHLD interrupts a processor + holding the mutex, it will deadlock, so SIGCHLDs must be + masked before the mutex is grabbed and unmasked before SIGCHLD + is unmasked. + + Readers of the process_table needn't serialize with writers. + Like channel_table, process_table is atomically updated by + process_allocate. Writers must grab the table's mutex but + only after masking SIGCHLD. + + The only writer of process_table (after initialization or + reset) is allocate_process, use of which is now serialized. + + cd src/microcode && find -type f | xargs grep -nH -E '[^_]process_table[^_]' + + The writers of process_table entry members are assumed to have + a macro like PROCESS_ID on the left hand side. These writers + now all grab the process_table_mutex before setting flags. + + cd src/microcode && find -type f | xargs grep -nH -E '(PROCESS_(ID|TICK|SYNC_TICK|RAW_REASON|REASON|RAW_STATUS|STATUS|JC_STATUS)[^=]*=[^=])|NEW_RAW_STATUS|PROCESS_STATUS_SYNC' + + There is (currently) just one console, one controlling + terminal. Multiple Scheme threads simultaneously munging + foreground_child_process and/or scheme_ctty_fd are crazy. + Presume any reasonable Scheme code using these is already + single-threaded by the runtime system. + + process_tick is only modified by NEW_RAW_STATUS, which is + locked up. + + sync_tick is only modified by OS_process_status_sync_all which + is presumably used in single-threaded mode (i.e. only by the + I/O waiter). uxsig.o: 00000000 b blocked_signals diff --git a/src/microcode/uxproc.c b/src/microcode/uxproc.c index 55780bdc5..06266a3ac 100644 --- a/src/microcode/uxproc.c +++ b/src/microcode/uxproc.c @@ -24,7 +24,7 @@ USA. */ -#include "scheme.h" +#include "prims.h" #include "ux.h" #include "uxproc.h" #include "uxio.h" @@ -45,7 +45,7 @@ static void subprocess_death (pid_t pid, int * status); static void stop_signal_handler (int signo); static void give_terminal_to (Tprocess process); static void get_terminal_back (void); -static void process_wait (Tprocess process); +static void process_wait (Tprocess process, sigset_t *mask); static int child_setup_tty (int fd); Tprocess OS_process_table_size; @@ -58,8 +58,21 @@ static Tprocess foreground_child_process; static long process_tick; static long sync_tick; + +#ifdef ENABLE_SMP +static pthread_mutex_t mutex = MUTEX_INITIALIZER; +# ifdef ENABLE_DEBUGGING_TOOLS +static bool locked_p = false; +# define ASSERT_LOCKED_P(boo) do { assert (locked_p == boo); } while (false) +# endif +#endif +#if !defined(ENABLE_SMP) || !defined(ENABLE_DEBUGGING_TOOLS) +# define ASSERT_LOCKED_P(code) do {} while (false) +#endif + #define NEW_RAW_STATUS(process, status, reason) do \ { \ + ASSERT_LOCKED_P (true); \ (PROCESS_RAW_STATUS (process)) = (status); \ (PROCESS_RAW_REASON (process)) = (reason); \ (PROCESS_TICK (process)) = (++process_tick); \ @@ -67,6 +80,7 @@ static long sync_tick; #define PROCESS_STATUS_SYNC(process) do \ { \ + ASSERT_LOCKED_P (true); \ (PROCESS_STATUS (process)) = (PROCESS_RAW_STATUS (process)); \ (PROCESS_REASON (process)) = (PROCESS_RAW_REASON (process)); \ (PROCESS_SYNC_TICK (process)) = (PROCESS_TICK (process)); \ @@ -111,12 +125,10 @@ block_jc_signals (void) transaction_record_action (tat_always, restore_signal_mask, outside); } -static sigset_t grabbed_signal_mask; - static void -grab_signal_mask (void) +grab_signal_mask (sigset_t *mask) { - UX_sigprocmask (SIG_BLOCK, 0, (&grabbed_signal_mask)); + UX_sigprocmask (SIG_BLOCK, 0, mask); } #else /* not HAVE_POSIX_SIGNALS */ @@ -143,7 +155,13 @@ block_sigchld (void) #endif /* not HAVE_SIGHOLD */ #define block_jc_signals block_sigchld -#define grab_signal_mask() + +#define sigset_t int +static void +grab_signal_mask (sigset_t *mask) +{ + *mask = -1; +} #endif /* not HAVE_POSIX_SIGNALS */ @@ -204,6 +222,7 @@ static Tprocess process_allocate (void) { Tprocess process; + ASSERT_LOCKED_P (true); for (process = 0; (process < OS_process_table_size); process += 1) if ((PROCESS_RAW_STATUS (process)) == process_status_free) { @@ -221,10 +240,10 @@ process_allocate (void) error_out_of_processes (); return (NO_PROCESS); } - OS_process_table_size = new_size; - process_table = new_table; for (process = old_size; (process < new_size); process += 1) OS_process_deallocate (process); + process_table = new_table; + OS_process_table_size = new_size; process = old_size; } { @@ -238,8 +257,13 @@ process_allocate (void) void OS_process_deallocate (Tprocess process) { + transaction_begin(); + block_sigchld(); + LOCK(); (PROCESS_ID (process)) = 0; (PROCESS_RAW_STATUS (process)) = process_status_free; + UNLOCK(); + transaction_commit(); } Tprocess @@ -257,6 +281,7 @@ OS_make_subprocess (const char * filename, Tchannel channel_err) { pid_t child_pid; + sigset_t mask; volatile Tprocess child; volatile enum process_jc_status child_jc_status = process_jc_status_no_ctty; @@ -277,12 +302,15 @@ OS_make_subprocess (const char * filename, } transaction_begin (); - child = (process_allocate ()); - grab_signal_mask (); + grab_signal_mask (&mask); if (ctty_type == process_ctty_type_inherit_fg) block_jc_signals (); else block_sigchld (); + /* The sigchld handler wants the mutex, so a SIGCHLD between LOCK + and UNLOCK will deadlock. */ + LOCK(); + child = (process_allocate ()); STD_UINT_SYSTEM_CALL (syscall_vfork, child_pid, (UX_vfork ())); if (child_pid > 0) @@ -294,6 +322,7 @@ OS_make_subprocess (const char * filename, (PROCESS_RAW_REASON (child)) = 0; (PROCESS_TICK (child)) = process_tick; PROCESS_STATUS_SYNC (child); + UNLOCK(); /* If we are doing job control for the child, make sure the child is in its own progress group before returning, so that we can @@ -318,7 +347,7 @@ OS_make_subprocess (const char * filename, if (ctty_type == process_ctty_type_inherit_fg) { give_terminal_to (child); - process_wait (child); + process_wait (child, &mask); } transaction_commit (); @@ -495,9 +524,11 @@ OS_process_status_sync (Tprocess process) { transaction_begin (); block_sigchld (); + LOCK(); { int result = ((PROCESS_TICK (process)) != (PROCESS_SYNC_TICK (process))); if (result) PROCESS_STATUS_SYNC (process); + UNLOCK(); transaction_commit (); return (result); } @@ -508,9 +539,11 @@ OS_process_status_sync_all (void) { transaction_begin (); block_sigchld (); + LOCK(); { int result = (process_tick != sync_tick); if (result) sync_tick = process_tick; + UNLOCK(); transaction_commit (); return (result); } @@ -603,37 +636,45 @@ OS_process_continue_background (Tprocess process) { transaction_begin (); block_sigchld (); + LOCK(); if ((PROCESS_RAW_STATUS (process)) == process_status_stopped) { NEW_RAW_STATUS (process, process_status_running, 0); process_send_signal (process, SIGCONT); } + UNLOCK(); transaction_commit (); } void OS_process_continue_foreground (Tprocess process) { + sigset_t mask; + transaction_begin (); - grab_signal_mask (); + grab_signal_mask (&mask); block_jc_signals (); give_terminal_to (process); + LOCK(); if ((PROCESS_RAW_STATUS (process)) == process_status_stopped) { NEW_RAW_STATUS (process, process_status_running, 0); process_send_signal (process, SIGCONT); } - process_wait (process); + UNLOCK(); + process_wait (process, &mask); transaction_commit (); } void OS_process_wait (Tprocess process) { + sigset_t mask; + transaction_begin (); - grab_signal_mask (); + grab_signal_mask (&mask); block_jc_signals (); - process_wait (process); + process_wait (process, &mask); transaction_commit (); } @@ -681,12 +722,12 @@ get_terminal_back (void) } static void -process_wait (Tprocess process) +process_wait (Tprocess process, sigset_t *mask) { #ifdef HAVE_POSIX_SIGNALS while (((PROCESS_RAW_STATUS (process)) == process_status_running) && (! (pending_interrupts_p ()))) - UX_sigsuspend (&grabbed_signal_mask); + UX_sigsuspend (mask); #else /* not HAVE_POSIX_SIGNALS */ enum process_status status = (PROCESS_RAW_STATUS (process)); while ((status == process_status_running) @@ -719,7 +760,9 @@ find_process (pid_t pid) static void subprocess_death (pid_t pid, int * status) { - Tprocess process = (find_process (pid)); + Tprocess process; + LOCK(); + process = (find_process (pid)); if (process != NO_PROCESS) { if (WIFEXITED (*status)) @@ -732,6 +775,7 @@ subprocess_death (pid_t pid, int * status) NEW_RAW_STATUS (process, process_status_signalled, (WTERMSIG (*status))); } + UNLOCK(); } static void -- 2.25.1