Merge branch '3977-add-isc_spinlock-implementation' into 'main'

Add isc_spinlock unit with shim pthread_spin implementation

Closes #3977

See merge request isc-projects/bind9!7749

CHANGES

@@ -1,3 +1,13 @@
+6154.	[func]		Add spinlock implementation.  The spinlock is much
+			smaller (8 bytes) than pthread_mutex (40 bytes), so
+			it can be easily embedded into objects for more
+			fine-grained locking (per-object vs per-bucket).
+
+			On the other hand, the spinlock is unsuitable for
+			situations where the lock might be held for a long
+			time as it keeps the waiting threads in a spinning
+			busy loop. [GL #3977]
+
 6153.	[bug]		Fix the streaming protocols (TCP, TLS) shutdown
 			sequence. [GL #4011]
 

configure.ac

@@ -553,7 +553,7 @@ LIBS="$PTHREAD_LIBS $LIBS"
 CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
 CC="$PTHREAD_CC"
 
-AC_CHECK_FUNCS([pthread_attr_getstacksize pthread_attr_setstacksize pthread_barrier_init])
+AC_CHECK_FUNCS([pthread_attr_getstacksize pthread_attr_setstacksize pthread_barrier_init pthread_spin_init])
 
 # [pairwise: --with-locktype=adaptive, --with-locktype=standard]
 AC_ARG_WITH([locktype],

lib/isc/Makefile.am

@@ -80,6 +80,7 @@ libisc_la_HEADERS =			\
 	include/isc/signal.h		\
 	include/isc/siphash.h		\
 	include/isc/sockaddr.h		\
+	include/isc/spinlock.h		\
 	include/isc/stack.h		\
 	include/isc/stats.h		\
 	include/isc/stdio.h		\

lib/isc/include/isc/spinlock.h

@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#pragma once
+
+/*! \file */
+
+#include <pthread.h>
+#include <stdlib.h>
+
+#include <isc/atomic.h>
+#include <isc/lang.h>
+#include <isc/util.h>
+
+ISC_LANG_BEGINDECLS
+
+/*
+ * We use macros instead of static inline functions so that the exact code
+ * location can be reported when PTHREADS_RUNTIME_CHECK() fails or when mutrace
+ * reports lock contention.
+ */
+
+#ifdef ISC_TRACK_PTHREADS_OBJECTS
+
+#define isc_spinlock_init(sp)               \
+	{                                   \
+		*sp = malloc(sizeof(**sp)); \
+		isc__spinlock_init(*sp);    \
+	}
+#define isc_spinlock_lock(sp)	isc__spinlock_lock(*sp)
+#define isc_spinlock_unlock(sp) isc__spinlock_unlock(*sp)
+#define isc_spinlock_destroy(sp)            \
+	{                                   \
+		isc__spinlock_destroy(*sp); \
+		free((void *)*sp);          \
+	}
+
+#else /* ISC_TRACK_PTHREADS_OBJECTS */
+
+#define isc_spinlock_init(sp)	 isc__spinlock_init(sp)
+#define isc_spinlock_lock(sp)	 isc__spinlock_lock(sp)
+#define isc_spinlock_unlock(sp)	 isc__spinlock_unlock(sp)
+#define isc_spinlock_destroy(sp) isc__spinlock_destroy(sp)
+
+#endif /* ISC_TRACK_PTHREADS_OBJECTS */
+
+#if HAVE_PTHREAD_SPIN_INIT
+
+#if ISC_TRACK_PTHREADS_OBJECTS
+typedef pthread_spinlock_t *isc_spinlock_t;
+#else  /* ISC_TRACK_PTHREADS_OBJECTS */
+typedef pthread_spinlock_t isc_spinlock_t;
+#endif /* ISC_TRACK_PTHREADS_OBJECTS */
+
+#define isc__spinlock_init(sp)                                             \
+	{                                                                  \
+		int _ret = pthread_spin_init(sp, PTHREAD_PROCESS_PRIVATE); \
+		PTHREADS_RUNTIME_CHECK(pthread_spin_init, _ret);           \
+	}
+
+#define isc__spinlock_lock(sp)                                   \
+	{                                                        \
+		int _ret = pthread_spin_lock(sp);                \
+		PTHREADS_RUNTIME_CHECK(pthread_spin_lock, _ret); \
+	}
+
+#define isc__spinlock_unlock(sp)                                   \
+	{                                                          \
+		int _ret = pthread_spin_unlock(sp);                \
+		PTHREADS_RUNTIME_CHECK(pthread_spin_unlock, _ret); \
+	}
+
+#define isc__spinlock_destroy(sp)                                   \
+	{                                                           \
+		int _ret = pthread_spin_destroy(sp);                \
+		PTHREADS_RUNTIME_CHECK(pthread_spin_destroy, _ret); \
+	}
+
+#else /* HAVE_PTHREAD_SPIN_INIT */
+
+#if ISC_TRACK_PTHREADS_OBJECTS
+typedef atomic_uint_fast32_t *isc_spinlock_t;
+#else  /* ISC_TRACK_PTHREADS_OBJECTS */
+typedef atomic_uint_fast32_t isc_spinlock_t;
+#endif /* ISC_TRACK_PTHREADS_OBJECTS */
+
+#define isc__spinlock_init(sp)      \
+	{                           \
+		atomic_init(sp, 0); \
+	}
+
+#define isc__spinlock_lock(sp)                                  \
+	{                                                       \
+		while (!atomic_compare_exchange_weak_acq_rel(   \
+			sp, &(uint_fast32_t){ 0 }, 1))          \
+		{                                               \
+			do {                                    \
+				isc_pause();                    \
+			} while (atomic_load_relaxed(sp) != 0); \
+		}                                               \
+	}
+
+#define isc__spinlock_unlock(sp)             \
+	{                                    \
+		atomic_store_release(sp, 0); \
+	}
+
+#define isc__spinlock_destroy(sp)             \
+	{                                     \
+		INSIST(atomic_load(sp) == 0); \
+	}
+
+#endif
+
+ISC_LANG_ENDDECLS

lib/isc/include/isc/util.h

@@ -116,6 +116,21 @@
 
 #include <isc/result.h> /* Contractual promise. */
 
+#define SPINLOCK(sp)                                                           \
+	{                                                                      \
+		ISC_UTIL_TRACE(fprintf(stderr, "SPINLOCKING %p %s %d\n", (sp), \
+				       __FILE__, __LINE__));                   \
+		isc_spinlock_lock((sp));                                       \
+		ISC_UTIL_TRACE(fprintf(stderr, "SPINLOCKED %p %s %d\n", (sp),  \
+				       __FILE__, __LINE__));                   \
+	}
+#define SPINUNLOCK(sp)                                                    \
+	{                                                                 \
+		isc_spinlock_unlock((sp));                                \
+		ISC_UTIL_TRACE(fprintf(stderr, "SPINUNLOCKED %p %s %d\n", \
+				       (sp), __FILE__, __LINE__));        \
+	}
+
 #define LOCK(lp)                                                           \
 	{                                                                  \
 		ISC_UTIL_TRACE(fprintf(stderr, "LOCKING %p %s %d\n", (lp), \

tests/include/tests/isc.h

@@ -42,6 +42,9 @@ setup_mctx(void **state);
 int
 teardown_mctx(void **state);
 
+int
+setup_workers(void **state);
+
 int
 setup_loopmgr(void **state);
 int
@@ -172,8 +175,8 @@ teardown_managers(void **state);
                                                                     \
 		signal(SIGPIPE, SIG_IGN);                           \
                                                                     \
-		isc_mem_debugging |= ISC_MEM_DEBUGRECORD;           \
+		setup_mctx(NULL);                                   \
-		isc_mem_create(&mctx);                              \
+		setup_workers(NULL);                                \
                                                                     \
 		r = cmocka_run_group_tests(tests, setup, teardown); \
                                                                     \

tests/isc/Makefile.am

@@ -41,6 +41,7 @@ check_PROGRAMS =	\
 	safe_test	\
 	siphash_test	\
 	sockaddr_test	\
+	spinlock_test	\
 	stats_test	\
 	symtab_test	\
 	tcp_test	\

tests/isc/spinlock_test.c

@@ -0,0 +1,225 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <fcntl.h>
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#ifdef HAVE_PTHREAD_SPIN_INIT
+#define HAD_PTHREAD_SPIN_INIT 1
+#undef HAVE_PTHREAD_SPIN_INIT
+#endif
+
+#include <isc/atomic.h>
+#include <isc/file.h>
+#include <isc/mem.h>
+#include <isc/os.h>
+#include <isc/pause.h>
+#include <isc/result.h>
+#include <isc/spinlock.h>
+#include <isc/stdio.h>
+#include <isc/thread.h>
+#include <isc/time.h>
+#include <isc/util.h>
+
+#include <tests/isc.h>
+
+static unsigned int loops = 100;
+static unsigned int delay_loop = 1;
+
+static int
+setup_env(void **unused __attribute__((__unused__))) {
+	char *env = getenv("ISC_BENCHMARK_LOOPS");
+	if (env != NULL) {
+		loops = atoi(env);
+	}
+	assert_int_not_equal(loops, 0);
+
+	env = getenv("ISC_BENCHMARK_DELAY");
+	if (env != NULL) {
+		delay_loop = atoi(env);
+	}
+	assert_int_not_equal(delay_loop, 0);
+
+	return (0);
+}
+
+ISC_RUN_TEST_IMPL(isc_spinlock) {
+	isc_spinlock_t lock;
+
+	isc_spinlock_init(&lock);
+
+	for (size_t i = 0; i < loops; i++) {
+		isc_spinlock_lock(&lock);
+		isc_pause_n(delay_loop);
+		isc_spinlock_unlock(&lock);
+	}
+
+	isc_spinlock_destroy(&lock);
+}
+
+#define ITERS 20
+
+#define DC	200
+#define CNT_MIN 800
+#define CNT_MAX 1600
+
+static size_t shared_counter = 0;
+static size_t expected_counter = SIZE_MAX;
+
+#if HAD_PTHREAD_SPIN_INIT
+static pthread_spinlock_t spin;
+
+static isc_threadresult_t
+pthread_spin_thread(isc_threadarg_t arg) {
+	size_t cont = *(size_t *)arg;
+
+	for (size_t i = 0; i < loops; i++) {
+		pthread_spin_lock(&spin);
+		size_t v = shared_counter;
+		isc_pause_n(delay_loop);
+		shared_counter = v + 1;
+		pthread_spin_unlock(&spin);
+		isc_pause_n(cont);
+	}
+
+	return ((isc_threadresult_t)0);
+}
+#endif
+
+static isc_spinlock_t lock;
+
+static isc_threadresult_t
+isc_spinlock_thread(isc_threadarg_t arg) {
+	size_t cont = *(size_t *)arg;
+
+	for (size_t i = 0; i < loops; i++) {
+		isc_spinlock_lock(&lock);
+		size_t v = shared_counter;
+		isc_pause_n(delay_loop);
+		shared_counter = v + 1;
+		isc_spinlock_unlock(&lock);
+		isc_pause_n(cont);
+	}
+
+	return ((isc_threadresult_t)0);
+}
+
+ISC_RUN_TEST_IMPL(isc_spinlock_benchmark) {
+	isc_thread_t *threads = isc_mem_get(mctx, sizeof(*threads) * workers);
+	isc_time_t ts1, ts2;
+	double t;
+	int dc;
+	size_t cont;
+
+	memset(threads, 0, sizeof(*threads) * workers);
+
+	expected_counter = ITERS * workers * loops *
+			   ((CNT_MAX - CNT_MIN) / DC + 1);
+
+	/* PTHREAD SPINLOCK */
+
+#if HAD_PTHREAD_SPIN_INIT
+	int r = pthread_spin_init(&spin, PTHREAD_PROCESS_PRIVATE);
+	assert_int_not_equal(r, -1);
+
+	ts1 = isc_time_now_hires();
+
+	shared_counter = 0;
+	dc = DC;
+	for (size_t l = 0; l < ITERS; l++) {
+		for (cont = (dc > 0) ? CNT_MIN : CNT_MAX;
+		     cont <= CNT_MAX && cont >= CNT_MIN; cont += dc)
+		{
+			for (size_t i = 0; i < workers; i++) {
+				isc_thread_create(pthread_spin_thread, &cont,
+						  &threads[i]);
+			}
+			for (size_t i = 0; i < workers; i++) {
+				isc_thread_join(threads[i], NULL);
+			}
+		}
+		dc = -dc;
+	}
+	assert_int_equal(shared_counter, expected_counter);
+
+	ts2 = isc_time_now_hires();
+
+	t = isc_time_microdiff(&ts2, &ts1);
+
+	printf("[ TIME     ] isc_spinlock_benchmark: %zu pthread_spin "
+	       "loops in "
+	       "%u threads, %2.3f seconds, %2.3f calls/second\n",
+	       shared_counter, workers, t / 1000000.0,
+	       shared_counter / (t / 1000000.0));
+
+	r = pthread_spin_destroy(&spin);
+	assert_int_not_equal(r, -1);
+#endif
+
+	/* ISC SPINLOCK */
+
+	isc_spinlock_init(&lock);
+
+	ts1 = isc_time_now_hires();
+
+	dc = DC;
+	shared_counter = 0;
+	for (size_t l = 0; l < ITERS; l++) {
+		for (cont = (dc > 0) ? CNT_MIN : CNT_MAX;
+		     cont <= CNT_MAX && cont >= CNT_MIN; cont += dc)
+		{
+			for (size_t i = 0; i < workers; i++) {
+				isc_thread_create(isc_spinlock_thread, &cont,
+						  &threads[i]);
+			}
+			for (size_t i = 0; i < workers; i++) {
+				isc_thread_join(threads[i], NULL);
+			}
+		}
+		dc = -dc;
+	}
+	assert_int_equal(shared_counter, expected_counter);
+
+	ts2 = isc_time_now_hires();
+
+	t = isc_time_microdiff(&ts2, &ts1);
+
+	printf("[ TIME     ] isc_spinlock_benchmark: %zu isc_spinlock loops "
+	       "in %u "
+	       "threads, %2.3f seconds, %2.3f calls/second\n",
+	       shared_counter, workers, t / 1000000.0,
+	       shared_counter / (t / 1000000.0));
+
+	isc_spinlock_destroy(&lock);
+
+	isc_mem_put(mctx, threads, sizeof(*threads) * workers);
+}
+
+ISC_TEST_LIST_START
+
+ISC_TEST_ENTRY(isc_spinlock)
+#if !defined(__SANITIZE_THREAD__)
+ISC_TEST_ENTRY(isc_spinlock_benchmark)
+#endif /* __SANITIZE_THREAD__ */
+
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN_CUSTOM(setup_env, NULL)

tests/libtest/isc.c

@@ -51,6 +51,24 @@ adjustnofile(void) {
 	}
 }
 
+int
+setup_workers(void **state ISC_ATTR_UNUSED) {
+	char *env_workers = getenv("ISC_TASK_WORKERS");
+	if (env_workers != NULL) {
+		workers = atoi(env_workers);
+	} else {
+		workers = isc_os_ncpus();
+
+		/* We always need at least two loops for some of the tests */
+		if (workers < 2) {
+			workers = 2;
+		}
+	}
+	INSIST(workers != 0);
+
+	return (0);
+}
+
 int
 setup_mctx(void **state ISC_ATTR_UNUSED) {
 	isc_mem_debugging |= ISC_MEM_DEBUGRECORD;
@@ -68,23 +86,9 @@ teardown_mctx(void **state ISC_ATTR_UNUSED) {
 
 int
 setup_loopmgr(void **state ISC_ATTR_UNUSED) {
-	char *env_workers = NULL;
-
 	REQUIRE(mctx != NULL);
 
-	env_workers = getenv("ISC_TASK_WORKERS");
+	setup_workers(state);
-	if (env_workers != NULL) {
-		workers = atoi(env_workers);
-	}
-
-	if (workers == 0) {
-		workers = isc_os_ncpus();
-
-		/* We always need at least two loops for some of the tests */
-		if (workers < 2) {
-			workers = 2;
-		}
-	}
 
 	isc_loopmgr_create(mctx, workers, &loopmgr);
 	mainloop = isc_loop_main(loopmgr);