From d18129bbf5e22fc2f54aed6dd8d27ef761280901 Mon Sep 17 00:00:00 2001 From: Matt Birkholz Date: Sat, 20 Dec 2014 12:27:28 -0700 Subject: [PATCH] smp: share: uxio.o --- README.txt | 44 +++++++++++++++++++++++++++++-- src/microcode/uxio.c | 63 +++++++++++++++++++++++++++++++++++--------- src/microcode/uxio.h | 39 ++++++++++++++++++++++++--- 3 files changed, 128 insertions(+), 18 deletions(-) diff --git a/README.txt b/README.txt index 5690b41df..9d2666cfd 100644 --- a/README.txt +++ b/README.txt @@ -691,8 +691,48 @@ command line. The remaining 12 belong to the 7 microcode modules and writers are required to serialize.) uxio.o: - 00000004 C OS_channel_table_size - 00000004 C channel_table + 00000004 C OS_channel_table_size locked + 00000004 C channel_table locked + + OK. Locked up, except the reload_cleanup, which presumably is + done during a gc-wait (during Prim_band_load), and during + OS_reset (which is called where?). + + I assume a word is atomically updated by the underlying + architecture and a pointer, e.g. channel_table, is a word. + + Readers of the channel_table needn't serialize because they + index into channel_table, which is atomically updated by + channel_allocate to point to a new table, a copy of the + original table. All entry members are word-sized and smaller, + and so should be reliably read in spite of simultaneous + writes. + + Writers DO need to lock so that they do not update the old + table AFTER another pthread has copied it to a new table. + Writers also need to serialize when updating the single bit + members like internal and nonblocking. + + The only writer of channel_table (after initialization or + reset) is allocate_channel, use of which is now serialized. + Writers were found by the following commandline. + + cd src/microcode && find -type f | xargs grep -nH -E '[^_]channel_table[^_]' + + The writers of channel_table entry members are assumed to have + a macro like CHANNEL_DESCRIPTOR on the left hand side, so + writers were found with the following commandline. These + writers (other than initializers) now all grab the + channel_table_mutex before setting flags. + + cd src/microcode && find -type f | xargs grep -nH -E '(CHANNEL_(ALLOCATED|DESCRIPTOR|TYPE|INTERNAL|NONBLOCKING).*=)|MAKE_CHANNEL|MARK_CHANNEL_CLOSED' + + Atomically updating two words, channel_table and OS_channel_ + table_size should be problematic except that the _table_size + is always increasing. Updating _table_size before channel_ + table would encourage indexing beyond the end of the table, + but updating channel_table first ensures that the table + entries exist before their indices are valid. uxproc.o: 00000004 C OS_process_table_size diff --git a/src/microcode/uxio.c b/src/microcode/uxio.c index c60ce906f..3bd0c5dbb 100644 --- a/src/microcode/uxio.c +++ b/src/microcode/uxio.c @@ -24,7 +24,6 @@ USA. */ -#include "scheme.h" #include "prims.h" #include "ux.h" #include "uxio.h" @@ -33,6 +32,13 @@ USA. Tchannel OS_channel_table_size; struct channel * channel_table; +#ifdef ENABLE_SMP +pthread_mutex_t channel_table_mutex = MUTEX_INITIALIZER; +# ifdef ENABLE_DEBUGGING_TOOLS +bool channel_table_locked_p = false; +# endif +#endif + #ifndef HAVE_POLL #ifdef HAVE_SELECT static struct timeval zero_timeout; @@ -43,6 +49,9 @@ static void UX_channel_close_all (void) { Tchannel channel; +#if defined(ENABLE_SMP) && defined(ENABLE_DEBUGGING_TOOLS) + assert (channel_table_locked_p == false); +#endif for (channel = 0; (channel < OS_channel_table_size); channel += 1) if (CHANNEL_OPEN_P (channel)) OS_channel_close_noerror (channel); @@ -77,6 +86,9 @@ UX_initialize_channels (void) void UX_reset_channels (void) { +#if defined(ENABLE_SMP) && defined(ENABLE_DEBUGGING_TOOLS) + assert(channel_table_locked_p == false); +#endif UX_free (channel_table); channel_table = 0; OS_channel_table_size = 0; @@ -86,6 +98,9 @@ Tchannel channel_allocate (void) { Tchannel channel; +#if defined(ENABLE_SMP) && defined(ENABLE_DEBUGGING_TOOLS) + assert(channel_table_locked_p == true); +#endif for (channel = 0; (channel < OS_channel_table_size); channel += 1) if (CHANNEL_CLOSED_P (channel)) return (channel); @@ -99,10 +114,10 @@ channel_allocate (void) error_out_of_channels (); return (NO_CHANNEL); } - OS_channel_table_size = new_size; - channel_table = new_table; for (channel = old_size; (channel < new_size); channel += 1) MARK_CHANNEL_CLOSED (channel); + channel_table = new_table; + OS_channel_table_size = new_size; return (old_size); } } @@ -124,14 +139,20 @@ OS_channel_close (Tchannel channel) { if (! (CHANNEL_INTERNAL (channel))) { - int status = (UX_close (CHANNEL_DESCRIPTOR (channel))); + while (true) + { + int status = (UX_close (CHANNEL_DESCRIPTOR (channel))); + if (status < 0) + if (errno == EINTR) + deliver_pending_interrupts (); + else + error_system_call (errno, syscall_close); + else + break; + } + LOCK_CHANNELS(); MARK_CHANNEL_CLOSED (channel); - if (status < 0) - switch (errno) - { - case EINTR: deliver_pending_interrupts (); break; - case EBADF: error_system_call (errno, syscall_close); break; - } + UNLOCK_CHANNELS(); } } @@ -140,8 +161,20 @@ OS_channel_close_noerror (Tchannel channel) { if (! (CHANNEL_INTERNAL (channel))) { - (void) UX_close (CHANNEL_DESCRIPTOR (channel)); + while (true) + { + int status = (UX_close (CHANNEL_DESCRIPTOR (channel))); + if (status < 0) + { + if (errno == EINTR) + deliver_pending_interrupts (); + } + else + break; + } + LOCK_CHANNELS(); MARK_CHANNEL_CLOSED (channel); + UNLOCK_CHANNELS(); } } @@ -379,7 +412,9 @@ OS_channel_nonblocking (Tchannel channel) ioctl (fd, FIONBIO, (&true)); } #endif + LOCK_CHANNELS(); (CHANNEL_NONBLOCKING (channel)) = 1; + UNLOCK_CHANNELS(); } void @@ -397,7 +432,9 @@ OS_channel_blocking (Tchannel channel) ioctl(fd,FIONBIO,&false); } #endif + LOCK_CHANNELS(); (CHANNEL_NONBLOCKING (channel)) = 0; + UNLOCK_CHANNELS(); } #else /* not FCNTL_NONBLOCK */ @@ -597,7 +634,6 @@ safe_poll (struct pollfd *fds, nfds_t nfds, int blockp) UX_sigprocmask (SIG_SETMASK, &old, NULL); } #else /* not HAVE_PPOLL */ - /* There is a signal "hole" here, but what can we do without ppoll()? */ INTERRUPTABLE_EXTENT (n, (((OS_process_any_status_change ()) || (pending_interrupts_p ())) @@ -806,7 +842,6 @@ safe_select (int nfds, SELECT_TYPE *readfds, SELECT_TYPE *writefds, int blockp) UX_sigprocmask (SIG_SETMASK, &old, NULL); } #else /* not HAVE_PSELECT */ - /* There is a signal "hole" here, but what can we do without pselect()? */ INTERRUPTABLE_EXTENT (n, (((OS_process_any_status_change ()) || (pending_interrupts_p ())) @@ -908,6 +943,7 @@ OS_pause (void) n = SELECT_INTERRUPT; } UX_sigprocmask (SIG_SETMASK, &old, NULL); + #else /* not HAVE_SIGSUSPEND */ INTERRUPTABLE_EXTENT (n, (((OS_process_any_status_change ()) @@ -918,6 +954,7 @@ OS_pause (void) n = SELECT_PROCESS_STATUS_CHANGE; else n = SELECT_INTERRUPT; + #endif return (n); } diff --git a/src/microcode/uxio.h b/src/microcode/uxio.h index e2222dbd3..bfb55063a 100644 --- a/src/microcode/uxio.h +++ b/src/microcode/uxio.h @@ -46,15 +46,48 @@ struct channel #define CHANNEL_NONBLOCKING(channel) \ ((channel_table [(channel)]) . nonblocking) -#define MAKE_CHANNEL(descriptor, type, receiver) \ +#ifdef ENABLE_SMP +# include +extern pthread_mutex_t channel_table_mutex; +# ifdef ENABLE_DEBUGGING_TOOLS +extern bool channel_table_locked_p; +# define LOCK_CHANNELS() do \ { \ - Tchannel MAKE_CHANNEL_temp = (channel_allocate ()); \ + pthread_mutex_lock (&channel_table_mutex); \ + channel_table_locked_p = true; \ +} while (false) +# define UNLOCK_CHANNELS() do \ +{ \ + pthread_mutex_unlock (&channel_table_mutex); \ + channel_table_locked_p = false; \ +} while (false) +# else +# define LOCK_CHANNELS() do \ +{ \ + pthread_mutex_lock (&channel_table_mutex); \ +} while (false) +# define UNLOCK_CHANNELS() do \ +{ \ + pthread_mutex_unlock (&channel_table_mutex); \ +} while (false) +# endif +#else +# define LOCK_CHANNELS() do { } while (false) +# define UNLOCK_CHANNELS() do { } while (false) +#endif + +#define MAKE_CHANNEL(descriptor, type, receiver) do \ +{ \ + Tchannel MAKE_CHANNEL_temp; \ + LOCK_CHANNELS(); \ + MAKE_CHANNEL_temp = (channel_allocate ()); \ (CHANNEL_DESCRIPTOR (MAKE_CHANNEL_temp)) = (descriptor); \ (CHANNEL_TYPE (MAKE_CHANNEL_temp)) = (type); \ (CHANNEL_INTERNAL (MAKE_CHANNEL_temp)) = 0; \ (CHANNEL_NONBLOCKING (MAKE_CHANNEL_temp)) = 0; \ + UNLOCK_CHANNELS(); \ receiver (MAKE_CHANNEL_temp); \ -} +} while (false) extern struct channel * channel_table; extern Tchannel channel_allocate (void); -- 2.25.1