NetBSD Problem Report #55677
From www@netbsd.org Mon Sep 21 17:24:47 2020
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 704BF1A9217
for <gnats-bugs@gnats.NetBSD.org>; Mon, 21 Sep 2020 17:24:47 +0000 (UTC)
Message-Id: <20200921172446.463F51A9239@mollari.NetBSD.org>
Date: Mon, 21 Sep 2020 17:24:46 +0000 (UTC)
From: arichardson@freebsd.org
Reply-To: arichardson@freebsd.org
To: gnats-bugs@NetBSD.org
Subject: Significantly speed up libthr/mutex_test and make more reliable
X-Send-Pr-Version: www-1.0
>Number: 55677
>Category: bin
>Synopsis: Significantly speed up libthr/mutex_test and make more reliable
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: bin-bug-people
>State: open
>Class: change-request
>Submitter-Id: net
>Arrival-Date: Mon Sep 21 17:25:00 +0000 2020
>Originator: Alex Richardson
>Release:
>Organization:
FreeBSD
>Environment:
>Description:
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 this almost always passed
with < 100,000 iterations and only reliably failed with the current
limit of 10 million.
I noticed this issue because the mutex:mutex{2,3} and timedmutex:mutex{2,3}
tests were always timing out on RISC-V QEMU in our CheriBSD Jenkins.
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.
>How-To-Repeat:
Run t_mutex.c on fully-emulated QEMU (e.g. RISC-V).
>Fix:
diff --git a/tests/lib/libpthread/t_mutex.c b/tests/lib/libpthread/t_mutex.c
index ef277fa7d..ba8a7de4b 100644
--- a/tests/lib/libpthread/t_mutex.c
+++ b/tests/lib/libpthread/t_mutex.c
@@ -128,16 +128,33 @@ ATF_TC_BODY(mutex1, tc)
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
}
+/*
+ * 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 */
+ __asm__ volatile("");
+ }
+ return value + 1;
+}
+
+static volatile bool thread2_started = false;
+
static void *
mutex2_threadfunc(void *arg)
{
long count = *(int *)arg;
+ thread2_started = true;
printf("2: Second thread (%p). Count is %ld\n", pthread_self(), count);
while (count--) {
PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
- global_x++;
+ global_x = increment(global_x);
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
}
@@ -151,7 +168,7 @@ ATF_TC_HEAD(mutex2, tc)
}
ATF_TC_BODY(mutex2, tc)
{
- int count, count2;
+ int count, count2, num_increments;
pthread_t new;
void *joinval;
@@ -160,18 +177,27 @@ ATF_TC_BODY(mutex2, tc)
PTHREAD_REQUIRE(pthread_mutex_init(&mutex, NULL));
global_x = 0;
- count = count2 = 10000000;
+ num_increments = count = count2 = 1000;
+ if (getenv("NUM_ITERATIONS") != NULL) {
+ num_increments = count = count2 = atoi(getenv("NUM_ITERATIONS"));
+ }
+ printf("Will use %d iterations\n", num_increments);
+
PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
+ thread2_started = false;
PTHREAD_REQUIRE(pthread_create(&new, NULL, mutex2_threadfunc, &count2));
printf("1: Thread %p\n", pthread_self());
-
+ 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());
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
while (count--) {
PTHREAD_REQUIRE(mutex_lock(&mutex, &ts_lengthy));
- global_x++;
+ global_x = increment(global_x);
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
}
@@ -180,19 +206,25 @@ 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);
- ATF_REQUIRE_EQ(global_x, 20000000);
+ 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);
}
+static volatile bool thread3_started = false;
+
static void *
mutex3_threadfunc(void *arg)
{
long count = *(int *)arg;
+ thread3_started = true;
printf("2: Second thread (%p). Count is %ld\n", pthread_self(), count);
while (count--) {
PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
- global_x++;
+ global_x = increment(global_x);
PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
}
@@ -207,25 +239,32 @@ ATF_TC_HEAD(mutex3, tc)
}
ATF_TC_BODY(mutex3, tc)
{
- int count, count2;
+ int count, count2, num_increments;
pthread_t new;
void *joinval;
printf("1: Mutex-test 3\n");
global_x = 0;
- count = count2 = 10000000;
+ num_increments = count = count2 = 1000;
+ if (getenv("NUM_ITERATIONS") != NULL) {
+ num_increments = count = count2 = atoi(getenv("NUM_ITERATIONS"));
+ }
+ printf("Will use %d iterations\n", num_increments);
PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
PTHREAD_REQUIRE(pthread_create(&new, NULL, mutex3_threadfunc, &count2));
printf("1: Thread %p\n", pthread_self());
-
+ 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());
PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
while (count--) {
PTHREAD_REQUIRE(mutex_lock(&static_mutex, &ts_lengthy));
- global_x++;
+ global_x = increment(global_x);
PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
}
@@ -234,7 +273,10 @@ 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);
- ATF_REQUIRE_EQ(global_x, 20000000);
+ 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);
}
static void *
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.