Skip to content

Commit

Permalink
ETHER: Don't spin in the libslirp reader thread
Browse files Browse the repository at this point in the history
Avoid spinning in the reader thread executing sim_slirp_select() when
libslirp isn't managing any sockets. Waits on a condition variable until
libslirp registers a socket via register_poll_socket(), at which point,
the condition variable is signaled.

- sim_atomic.h: Add atomic value primitives to ensure value
  synchronization across threads. Required for ARM-based platforms,
  useful on Intel.

  At the moment, these primitives are used by NAT(libslirp). However,
  there are many other places in SIMH where they would be useful if
  used.

  Does not (yet) replace the AIO intrinsics or synchronization code in
  sim_defs.h.

- sim_slirp: Add no_sockets_cv condvar, no_sockets_lock mutex. If
  libslirp hasn't registered any sockets (which includes the TCP and UDP
  redirections), sim_slirp_select() waits on the condvar until
  awakened when register_poll_socket() encounters its first socket.

  Simulators that don't use AIO, but end up calling sim_slirp_select():
  Sleep for 5 ms and return 0. It's a punt.

- sim_ether: Close and cleanup the underlying Ethernet device before
  cleaning up the other reader thread synchronization primitives.
  Otherwise, the NAT(libslirp) reader thread will not exit.
  • Loading branch information
bscottm committed Dec 28, 2024
1 parent 294e970 commit 1257973
Show file tree
Hide file tree
Showing 5 changed files with 371 additions and 29 deletions.
273 changes: 273 additions & 0 deletions sim_atomic.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,273 @@
/* Atomic get/put/add/sub/inc/dec support. Fashioned after the SDL2 approach to
* atomic variables. Uses compiler and platform intrinsics whenever possible,
* falling back to a mutex-based approach when necessary.
*
* Supported compilers: GCC and Clang across all platforms (including Windows,
* if compiling within a MinGW environment.)
*
* Platforms: Windows with MSVC.
*
* Types:
*
* sim_atomic_value_t: The wrapper structure that encapsulates the atomic
* value. Normally, if using GCC, Clang or MSVC, the value is managed via the
* appropriate intrinsics. The fallback is a pthread mutex.
*
* sim_atomic_type_t: The underlying type inside the sim_atomic_value_t wrapper.
* This is a long for GCC and Clang, LONG for Windows.
*
* Clang's atomic intrinsics are slightly different as compared to GCC: For add,
* subtract operations, Clang's atomics fetch and return the old value. So,
* Clang takes an extra atomic load to fetch the new value.
*
* sim_atomic_init(sim_atomic_value_t *): Initialize an atomic wrapper. The
* value is set to 0 and the mutex is initialized (when necessary.)
*
* sim_atomic_destroy(sim_atomic_value_t *p): The inverse of sim_atomic_init().
* The value is set to -1. When necessary, the mutex is destroyed.
*
* sim_atomic_get(sim_atomic_value_t *p): Atomically returns the
* sim_atomic_type_t value in the wrapper.
*
* sim_atomic_put(sim_atomic_value_t *p, sim_atomic_type_t newval): Atomically
* stores a new value in the wrapper.
*
* sim_atomic_add, sim_atomic_sub(sim_atomic_value_t *p, sim_atomic_type_t x):
* Atomically add or subtract a quantity to or from the wrapper's value.
*
* sim_atomic_inc, sim_atomic_dec(sim_atomic_value_t *p): Atomically increment or
* decrement the wrapper's value.
*/

#if !defined(SIM_ATOMIC_H)

#include <pthread.h>

/* TODO: defined(__DECC_VER) && defined(_IA64) -- DEC C on Itanium looks like it
* might be the same as Windows' interlocked API. Contribtion/correction needed. */

/* NON-FEATURE: Older GCC __sync_* primitives. These have been deprecated for a long
* time. Explicitly not supported. */

#if (defined(_WIN32) || defined(_WIN64)) || \
(defined(__ATOMIC_ACQ_REL) && defined(__ATOMIC_SEQ_CST) && defined(__ATOMIC_ACQUIRE))
/* Atomic operations available! */
#define HAVE_ATOMIC_PRIMS 1
#else
#define HAVE_ATOMIC_PRIMS 0
#endif

/*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=
* Value type and wrapper for atomics:
*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=*/

#if !defined(_WIN32) && !defined(_WIN64)
typedef long sim_atomic_type_t;
#else
typedef LONG sim_atomic_type_t;
#endif

typedef struct {
#if !HAVE_ATOMIC_PRIMS
/* If the compiler doesn't support atomic intrinsics, the backup plan is
* a mutex. */
pthread_mutex_t value_lock;
#endif

volatile sim_atomic_type_t value;
} sim_atomic_value_t;

/*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=
* Initialization, destruction:
*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=*/

static SIM_INLINE void sim_atomic_init(sim_atomic_value_t *p)
{
#if !HAVE_ATOMIC_PRIMS
pthread_mutex_init(&p->value_lock, NULL);
#endif
p-> value = 0;
}

static SIM_INLINE void sim_atomic_destroy(sim_atomic_value_t *p)
{
#if !HAVE_ATOMIC_PRIMS
pthread_mutex_destroy(&p->value_lock);
#endif
p-> value = -1;
}

/*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=
* Primitives:
*~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=*/

static SIM_INLINE sim_atomic_type_t sim_atomic_get(sim_atomic_value_t *p)
{
sim_atomic_type_t retval;

#if HAVE_ATOMIC_PRIMS
# if defined(__ATOMIC_ACQUIRE) && (defined(__GNUC__) || defined(__clang__))
__atomic_load(&p->value, &retval, __ATOMIC_ACQUIRE);
# elif defined(_WIN32) || defined(_WIN64)
# if defined(_M_IX86) || defined(_M_X64)
/* Intel Total Store Ordering optimization. */
retval = p->value;
# else
retval = InterlockedOr(&p->value, 0);
# endif
# else
# error "sim_atomic_get: No intrinsic?"
retval = -1;
# endif
#else
pthread_mutex_lock(&p->value_lock);
retval = p->value;
pthread_mutex_unlock(&p->value_lock);
#endif

return retval;
}

static SIM_INLINE void sim_atomic_put(sim_atomic_value_t *p, sim_atomic_type_t newval)
{
#if HAVE_ATOMIC_PRIMS
# if defined(__ATOMIC_SEQ_CST) && (defined(__GNUC__) || defined(__clang__))
/* Clang doesn't like __ATOMIC_ACQ_REL for atomic stores. And for good reason: sequential
* consistency is the appropriate memory ordering for storing values. */
__atomic_store(&p->value, &newval, __ATOMIC_SEQ_CST);
# elif defined(_WIN32) || defined(_WIN64)
# if defined(_M_IX86) || defined(_M_X64)
/* Intel Total Store Ordering optimization. */
p->value = newval;
# else
sim_atomic_type_t retval, cmp;

do {
cmp = sim_atomic_int_get(p);
retval = InterlockedCompareExchange(&p->value, newval, cmp);
} while (retval != cmp);
# endif
# else
# error "sim_atomic_put: No intrinsic?"
# endif
#else
pthread_mutex_lock(&p->value_lock);
p->value = newval;
pthread_mutex_unlock(&p->value_lock);
#endif
}

static SIM_INLINE sim_atomic_type_t sim_atomic_add(sim_atomic_value_t *p, sim_atomic_type_t x)
{
sim_atomic_type_t retval;

#if HAVE_ATOMIC_PRIMS
# if defined(__ATOMIC_ACQ_REL)
# if defined(__GNUC__)
retval = __atomic_add_fetch(&p->value, x, __ATOMIC_ACQ_REL);
# elif defined(__clang__)
# if __LONG_WIDTH__ == 32
(void) __atomic_fetch_add_4(&p->value, x, __ATOMIC_ACQ_REL);
# else if __LONG_WIDTH__ == 64
(void) __atomic_fetch_add_8(&p->value, x, __ATOMIC_ACQ_REL);
# endif
/* Return the updated value. */
retval = sim_atomic_get(p);
# else
# error "sim_atomic_add: No __ATOMIC_ACQ_REL intrinsic?"
# endif
# elif defined(_WIN32) || defined(_WIN64)
retval = InterlockedAdd(&p->value, x);
# else
# error "sim_atomic_add: No intrinsic?"
# endif
#else
pthread_mutex_lock(&p->value_lock);
p->value += newval;
retval = p->value;
pthread_mutex_unlock(&p->value_lock);
#endif

return retval;
}

static SIM_INLINE sim_atomic_type_t sim_atomic_sub(sim_atomic_value_t *p, sim_atomic_type_t x)
{
sim_atomic_type_t retval;

#if HAVE_ATOMIC_PRIMS
# if defined(__ATOMIC_ACQ_REL)
# if defined(__GNUC__)
retval = __atomic_sub_fetch(&p->value, x, __ATOMIC_ACQ_REL);
# elif defined(__clang__)
# if __LONG_WIDTH__ == 32
__atomic_fetch_sub_4(&p->value, x, __ATOMIC_ACQ_REL);
# else if __LONG_WIDTH__ == 64
__atomic_fetch_sub_8(&p->value, x, __ATOMIC_ACQ_REL);
# endif
/* Return the updated value. */
retval = sim_atomic_get(p);
# else
# error "sim_atomic_sub: No __ATOMIC_ACQ_REL intrinsic?"
# endif
# elif defined(_WIN32) || defined(_WIN64)
/* No InterlockedSub() -- use the math identity. */
retval = sim_atomic_add(p, -x);
# else
# error "sim_atomic_sub: No intrinsic?"
# endif
#else
pthread_mutex_lock(&p->value_lock);
p->value -= newval;
retval = p->value;
pthread_mutex_unlock(&p->value_lock);
#endif

return retval;
}

static SIM_INLINE sim_atomic_type_t sim_atomic_inc(sim_atomic_value_t *p)
{
sim_atomic_type_t retval;

#if HAVE_ATOMIC_PRIMS
# if !defined(_WIN32) && !defined(_WIN64)
retval = sim_atomic_add(p, 1);
# elif defined(_WIN32) || defined(_WIN64)
retval = InterlockedIncrement(&p->value);
# else
# error "sim_atomic_inc: No intrinsic?"
# endif
#else
pthread_mutex_lock(&p->value_lock);
retval = ++p->value;
pthread_mutex_unlock(&p->value_lock);
#endif

return retval;
}

static SIM_INLINE sim_atomic_type_t sim_atomic_dec(sim_atomic_value_t *p)
{
sim_atomic_type_t retval;

#if HAVE_ATOMIC_PRIMS
# if !defined(_WIN32) && !defined(_WIN64)
retval = sim_atomic_sub(p, 1);
# elif defined(_WIN32) || defined(_WIN64)
retval = InterlockedDecrement(&p->value);
# else
# error "sim_atomic_dec: No intrinsic?"
# endif
#else
pthread_mutex_lock(&p->value_lock);
retval = --p->value;
pthread_mutex_unlock(&p->value_lock);
#endif

return retval;
}


#define SIM_ATOMIC_H
#endif
7 changes: 6 additions & 1 deletion sim_ether.c
Original file line number Diff line number Diff line change
Expand Up @@ -2674,13 +2674,19 @@ dev->fd_handle = 0;
dev->have_host_nic_phy_addr = 0;

#if defined (USE_READER_THREAD)
/* Close the ethernet device first. */
_eth_close_port (dev->eth_api, pcap, pcap_fd);

/* Then continue to clean up the sync primitives that are no longer
* needed. */
pthread_join (dev->reader_thread, NULL);
pthread_mutex_destroy (&dev->lock);
pthread_cond_signal (&dev->writer_cond);
pthread_join (dev->writer_thread, NULL);
pthread_mutex_destroy (&dev->self_lock);
pthread_mutex_destroy (&dev->writer_lock);
pthread_cond_destroy (&dev->writer_cond);

if (1) {
ETH_WRITE_REQUEST *buffer;
while (NULL != (buffer = dev->write_buffers)) {
Expand All @@ -2695,7 +2701,6 @@ if (1) {
ethq_destroy (&dev->read_queue); /* release FIFO queue */
#endif

_eth_close_port (dev->eth_api, pcap, pcap_fd);
sim_messagef (SCPE_OK, "Eth: closed %s\n", dev->name);

/* clean up the mess */
Expand Down
23 changes: 19 additions & 4 deletions sim_slirp/sim_slirp.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,12 @@ SimSlirpNetwork *sim_slirp_open(const char *args, void *pkt_opaque, packet_callb
/* SIMH state/config */
slirp->args = (char *) calloc(1 + strlen(args), sizeof(char));
strcpy(slirp->args, args);
pthread_mutex_init(&slirp->libslirp_access, NULL);
#if defined(USE_READER_THREAD)
pthread_mutex_init(&slirp->libslirp_lock, NULL);
pthread_cond_init(&slirp->no_sockets_cv, NULL);
pthread_mutex_init(&slirp->no_sockets_lock, NULL);
#endif
sim_atomic_init(&slirp->n_sockets);

slirp->original_debflags = dptr->debflags;
dptr->debflags = sim_combine_debtabs(dptr->debflags, slirp_dbgtable);
Expand Down Expand Up @@ -524,7 +529,17 @@ void sim_slirp_close(SimSlirpNetwork *slirp)
slirp->fds = NULL;
#endif

pthread_mutex_destroy(&slirp->libslirp_access);
#if defined(USE_READER_THREAD)
sim_atomic_put(&slirp->n_sockets, -1);
pthread_cond_broadcast(&slirp->no_sockets_cv);

pthread_mutex_destroy(&slirp->libslirp_lock);
sim_atomic_destroy(&slirp->n_sockets);
pthread_cond_destroy(&slirp->no_sockets_cv);
pthread_mutex_destroy(&slirp->no_sockets_lock);
sim_atomic_destroy(&slirp->n_sockets);
#endif

free(slirp);
}

Expand Down Expand Up @@ -690,9 +705,9 @@ int sim_slirp_send(SimSlirpNetwork *slirp, const char *msg, size_t len, int flag
GLIB_UNUSED_PARAM(flags);

/* Just send the packet up to libslirp. */
pthread_mutex_lock(&slirp->libslirp_access);
pthread_mutex_lock(&slirp->libslirp_lock);
slirp_input(slirp->slirp_cxn, (const uint8_t *) msg, (int) len);
pthread_mutex_unlock(&slirp->libslirp_access);
pthread_mutex_unlock(&slirp->libslirp_lock);

return (int) len;
}
Expand Down
9 changes: 8 additions & 1 deletion sim_slirp/sim_slirp_network.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "sim_sock.h"
#include "sim_slirp.h"
#include "sim_atomic.h"
#include "libslirp.h"

#if !defined(SLIRP_NETWORK_STATE_H)
Expand Down Expand Up @@ -63,9 +64,15 @@ struct sim_slirp {

#if defined(USE_READER_THREAD)
/* Access lock to libslirp. libslirp is not threaded or protected. */
pthread_mutex_t libslirp_access;
pthread_mutex_t libslirp_lock;

/* Condvar, mutex when there are no sockets to poll or select. */
pthread_cond_t no_sockets_cv;
pthread_mutex_t no_sockets_lock;
#endif

sim_atomic_value_t n_sockets;

/* DNS search domains (argument copy) */
char *dns_search;
char **dns_search_domains;
Expand Down
Loading

0 comments on commit 1257973

Please sign in to comment.