svn commit: r368055 - head/contrib/netbsd-tests/lib/libpthread

Alex Richardson arichardson at FreeBSD.org
Thu Nov 26 13:31:58 UTC 2020


Author: arichardson
Date: Thu Nov 26 13:31:57 2020
New Revision: 368055
URL: https://svnweb.freebsd.org/changeset/base/368055

Log:
  Significantly speed up libthr/mutex_test and make more reliable
  
  Instead of using a simple global++ as the data race, with this change we
  perform the increment by loading the global, delaying for a bit and then
  storing back the incremented value. If I move the increment outside of the
  mutex protected range, I can now see the data race with only 100 iterations
  on amd64 in almost all cases. Before this change such a racy test almost
  always passed with < 100,000 iterations and only reliably failed with the
  current limit of 10 million.
  
  I noticed this poorly written test because the mutex:mutex{2,3} and
  timedmutex:mutex{2,3} tests were always timing out on our CheriBSD Jenkins.
  Writing good concurrency tests is hard so I won't attempt to do so, but this
  change should make the test more likely to fail if pthread_mutex_lock is not
  implemented correctly while also significantly reducing the time it takes to
  run these four tests. It will also reduce the time it takes for QEMU RISC-V
  testsuite runs by almost 40 minutes (out of currently 7 hours).
  
  Reviewed By:	brooks, ngie
  Differential Revision: https://reviews.freebsd.org/D26473

Modified:
  head/contrib/netbsd-tests/lib/libpthread/t_mutex.c

Modified: head/contrib/netbsd-tests/lib/libpthread/t_mutex.c
==============================================================================
--- head/contrib/netbsd-tests/lib/libpthread/t_mutex.c	Thu Nov 26 10:17:56 2020	(r368054)
+++ head/contrib/netbsd-tests/lib/libpthread/t_mutex.c	Thu Nov 26 13:31:57 2020	(r368055)
@@ -35,6 +35,9 @@ __RCSID("$NetBSD: t_mutex.c,v 1.15 2017/01/16 16:23:41
 #include <inttypes.h> /* For UINT16_MAX */
 #include <pthread.h>
 #include <stdio.h>
+#ifdef __FreeBSD__
+#include <stdlib.h>
+#endif
 #include <string.h>
 #include <errno.h>
 #include <time.h>
@@ -128,16 +131,41 @@ ATF_TC_BODY(mutex1, tc)
 	PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
 }
 
+#ifdef __FreeBSD__
+/*
+ * Increment the value using a noinline function that includes a small delay
+ * to increase the window for the RMW data race.
+ */
+__noinline static int
+increment(int value)
+{
+	for (volatile int i = 0; i < 100; i++) {
+		/* Small delay between read+write to increase chance of race */
+		__compiler_membar();
+	}
+	return value + 1;
+}
+
+static volatile bool thread2_started = false;
+#endif
+
 static void *
 mutex2_threadfunc(void *arg)
 {
 	long count = *(int *)arg;
 
+#ifdef __FreeBSD__
+	thread2_started = true;
+#endif
 	printf("2: Second thread (%p). Count is %ld\n", pthread_self(), count);
 
 	while (count--) {
 		PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
+#ifdef __FreeBSD__
+		global_x = increment(global_x);
+#else
 		global_x++;
+#endif
 		PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
 	}
 
@@ -153,16 +181,13 @@ ATF_TC_HEAD(mutex2, tc)
 	atf_tc_set_md_var(tc, "timeout", "40");
 #endif
 #endif
-
-#ifdef __FreeBSD__
-#if defined(__riscv)
-	atf_tc_set_md_var(tc, "timeout", "600");
-#endif
-#endif
 }
 ATF_TC_BODY(mutex2, tc)
 {
 	int count, count2;
+#ifdef __FreeBSD__
+	int num_increments;
+#endif
 	pthread_t new;
 	void *joinval;
 
@@ -177,18 +202,39 @@ ATF_TC_BODY(mutex2, tc)
 	PTHREAD_REQUIRE(pthread_mutex_init(&mutex, NULL));
 	
 	global_x = 0;
+#ifdef __FreeBSD__
+	num_increments = count = count2 = 1000;
+	if (getenv("NUM_ITERATIONS") != NULL) {
+		num_increments = count = count2 =
+		    MIN(INT_MAX, strtoul(getenv("NUM_ITERATIONS"), NULL, 10));
+	}
+	printf("Will use %d iterations\n", num_increments);
+#else
 	count = count2 = 10000000;
+#endif
 
 	PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
+#ifdef __FreeBSD__
+	thread2_started = false;
+#endif
 	PTHREAD_REQUIRE(pthread_create(&new, NULL, mutex2_threadfunc, &count2));
 
 	printf("1: Thread %p\n", pthread_self());
-
+#ifdef __FreeBSD__
+	while (!thread2_started) {
+		/* Wait for thread 2 to start to increase chance of race */
+	}
+	printf("1: Unlocking to start increment loop %p\n", pthread_self());
+#endif
 	PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
 
 	while (count--) {
 		PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
+#ifdef __FreeBSD__
+		global_x = increment(global_x);
+#else
 		global_x++;
+#endif
 		PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
 	}
 
@@ -197,7 +243,14 @@ ATF_TC_BODY(mutex2, tc)
 	PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
 	printf("1: Thread joined. X was %d. Return value (long) was %ld\n",
 		global_x, (long)joinval);
+#ifdef __FreeBSD__
+	ATF_REQUIRE_EQ_MSG(count, -1, "%d", count);
+	ATF_REQUIRE_EQ_MSG((long)joinval, -1, "%ld", (long)joinval);
+	ATF_REQUIRE_EQ_MSG(global_x, num_increments * 2, "%d vs %d", global_x,
+	    num_increments * 2);
+#else
 	ATF_REQUIRE_EQ(global_x, 20000000);
+#endif
 
 #ifdef __NetBSD__
 #if defined(__powerpc__)
@@ -210,16 +263,27 @@ ATF_TC_BODY(mutex2, tc)
 #endif
 }
 
+#ifdef __FreeBSD__
+static volatile bool thread3_started = false;
+#endif
+
 static void *
 mutex3_threadfunc(void *arg)
 {
 	long count = *(int *)arg;
 
+#ifdef __FreeBSD__
+	thread3_started = true;
+#endif
 	printf("2: Second thread (%p). Count is %ld\n", pthread_self(), count);
 
 	while (count--) {
 		PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
+#ifdef __FreeBSD__
+		global_x = increment(global_x);
+#else
 		global_x++;
+#endif
 		PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
 	}
 
@@ -236,16 +300,13 @@ ATF_TC_HEAD(mutex3, tc)
 	atf_tc_set_md_var(tc, "timeout", "40");
 #endif
 #endif
-
-#ifdef __FreeBSD__
-#if defined(__riscv)
-	atf_tc_set_md_var(tc, "timeout", "600");
-#endif
-#endif
 }
 ATF_TC_BODY(mutex3, tc)
 {
 	int count, count2;
+#ifdef __FreeBSD__
+	int num_increments;
+#endif
 	pthread_t new;
 	void *joinval;
 
@@ -258,18 +319,36 @@ ATF_TC_BODY(mutex3, tc)
 #endif
 
 	global_x = 0;
+#ifdef __FreeBSD__
+	num_increments = count = count2 = 1000;
+	if (getenv("NUM_ITERATIONS") != NULL) {
+		num_increments = count = count2 =
+		    MIN(INT_MAX, strtoul(getenv("NUM_ITERATIONS"), NULL, 10));
+	}
+	printf("Will use %d iterations\n", num_increments);
+#else
 	count = count2 = 10000000;
+#endif
 
 	PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
 	PTHREAD_REQUIRE(pthread_create(&new, NULL, mutex3_threadfunc, &count2));
 
 	printf("1: Thread %p\n", pthread_self());
-
+#ifdef __FreeBSD__
+	while (!thread3_started) {
+		/* Wait for thread 3 to start to increase chance of race */
+	}
+	printf("1: Unlocking to start increment loop %p\n", pthread_self());
+#endif
 	PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
 
 	while (count--) {
 		PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
+#ifdef __FreeBSD__
+		global_x = increment(global_x);
+#else
 		global_x++;
+#endif
 		PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
 	}
 
@@ -278,8 +357,14 @@ ATF_TC_BODY(mutex3, tc)
 	PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
 	printf("1: Thread joined. X was %d. Return value (long) was %ld\n",
 		global_x, (long)joinval);
+#ifdef __FreeBSD__
+	ATF_REQUIRE_EQ_MSG(count, -1, "%d", count);
+	ATF_REQUIRE_EQ_MSG((long)joinval, -1, "%ld", (long)joinval);
+	ATF_REQUIRE_EQ_MSG(global_x, num_increments * 2, "%d vs %d", global_x,
+	    num_increments * 2);
+#else
 	ATF_REQUIRE_EQ(global_x, 20000000);
-
+#endif
 #ifdef __NetBSD__
 #if defined(__powerpc__)
 	/* XXX force a timeout in ppc case since an un-triggered race


More information about the svn-src-head mailing list