Pádraig Brady wrote:
> There was a recent enough report on helgrind reporting issues with it:
> https://lists.gnu.org/archive/html/bug-gnulib/2015-07/msg00032.html
I would view this as a false positive. The test uses some 'volatile'
variables to communicate among threads, and 'valgrind --tool=helgrind'
does not know about it. There are no mentions of 'volatile' in the
helgrind documentation http://valgrind.org/docs/manual/hg-manual.html,
and there are similar reports at
https://sourceforge.net/p/valgrind/mailman/valgrind-users/thread/5038A955.6060108%40acm.org/#msg29721076
> I've seen this test take a minute or so on a 40 core system.
The running time is a different problem. I observe a long running time
with "#define EXPLICIT_YIELD 0". But the default is "#define EXPLICIT_YIELD 1".
When I use the attached patch to avoid the use of 'volatile' and replace it
by a lock, like explained in
http://stackoverflow.com/questions/10091112/using-a-global-variable-to-control-a-while-loop-in-a-separate-thread,
it does not change the run time that I observe on a 4-core system:
$ time ./test-lock
Starting test_lock ... OK
Starting test_rwlock ... OK
Starting test_recursive_lock ... OK
Starting test_once ... OK
real 0m1.770s
user 0m1.240s
sys 0m10.268s
Can someone test how this looks like on a 40-core system?
Bruno
diff --git a/tests/test-lock.c b/tests/test-lock.c
index cb734b4..f4c14f2 100644
--- a/tests/test-lock.c
+++ b/tests/test-lock.c
@@ -50,6 +50,11 @@
Uncomment this to see if the operating system has a fair scheduler. */
#define EXPLICIT_YIELD 1
+/* Whether to use 'volatile' on some variables that communicate information
+ between threads. If set to zero, a lock is used to protect these variables,
+ rather than 'volatile'. */
+#define USE_VOLATILE 0
+
/* Whether to print debugging messages. */
#define ENABLE_DEBUGGING 0
@@ -170,12 +175,38 @@ lock_mutator_thread (void *arg)
return NULL;
}
+#if USE_VOLATILE
static volatile int lock_checker_done;
+static int is_lock_checker_done()
+{
+ return lock_checker_done;
+}
+static void set_lock_checker_done()
+{
+ lock_checker_done = 1;
+}
+#else
+static int lock_checker_done;
+gl_lock_define_initialized(static, lock_checker_done_lock)
+static int is_lock_checker_done()
+{
+ gl_lock_lock (lock_checker_done_lock);
+ int result = lock_checker_done;
+ gl_lock_unlock (lock_checker_done_lock);
+ return result;
+}
+static void set_lock_checker_done()
+{
+ gl_lock_lock (lock_checker_done_lock);
+ lock_checker_done = 1;
+ gl_lock_unlock (lock_checker_done_lock);
+}
+#endif
static void *
lock_checker_thread (void *arg)
{
- while (!lock_checker_done)
+ while (! is_lock_checker_done ())
{
dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
gl_lock_lock (my_lock);
@@ -210,7 +241,7 @@ test_lock (void)
/* Wait for the threads to terminate. */
for (i = 0; i < THREAD_COUNT; i++)
gl_thread_join (threads[i], NULL);
- lock_checker_done = 1;
+ set_lock_checker_done ();
gl_thread_join (checkerthread, NULL);
check_accounts ();
}
@@ -254,12 +285,38 @@ rwlock_mutator_thread (void *arg)
return NULL;
}
+#if USE_VOLATILE
static volatile int rwlock_checker_done;
+static int is_rwlock_checker_done()
+{
+ return rwlock_checker_done;
+}
+static void set_rwlock_checker_done()
+{
+ rwlock_checker_done = 1;
+}
+#else
+static int rwlock_checker_done;
+gl_lock_define_initialized(static, rwlock_checker_done_lock)
+static int is_rwlock_checker_done()
+{
+ gl_lock_lock (rwlock_checker_done_lock);
+ int result = rwlock_checker_done;
+ gl_lock_unlock (rwlock_checker_done_lock);
+ return result;
+}
+static void set_rwlock_checker_done()
+{
+ gl_lock_lock (rwlock_checker_done_lock);
+ rwlock_checker_done = 1;
+ gl_lock_unlock (rwlock_checker_done_lock);
+}
+#endif
static void *
rwlock_checker_thread (void *arg)
{
- while (!rwlock_checker_done)
+ while (! is_rwlock_checker_done ())
{
dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ());
gl_rwlock_rdlock (my_rwlock);
@@ -295,7 +352,7 @@ test_rwlock (void)
/* Wait for the threads to terminate. */
for (i = 0; i < THREAD_COUNT; i++)
gl_thread_join (threads[i], NULL);
- rwlock_checker_done = 1;
+ set_rwlock_checker_done ();
for (i = 0; i < THREAD_COUNT; i++)
gl_thread_join (checkerthreads[i], NULL);
check_accounts ();
@@ -356,12 +413,38 @@ reclock_mutator_thread (void *arg)
return NULL;
}
+#if USE_VOLATILE
static volatile int reclock_checker_done;
+static int is_reclock_checker_done()
+{
+ return reclock_checker_done;
+}
+static void set_reclock_checker_done()
+{
+ reclock_checker_done = 1;
+}
+#else
+static int reclock_checker_done;
+gl_lock_define_initialized(static, reclock_checker_done_lock)
+static int is_reclock_checker_done()
+{
+ gl_lock_lock (reclock_checker_done_lock);
+ int result = reclock_checker_done;
+ gl_lock_unlock (reclock_checker_done_lock);
+ return result;
+}
+static void set_reclock_checker_done()
+{
+ gl_lock_lock (reclock_checker_done_lock);
+ reclock_checker_done = 1;
+ gl_lock_unlock (reclock_checker_done_lock);
+}
+#endif
static void *
reclock_checker_thread (void *arg)
{
- while (!reclock_checker_done)
+ while (! is_reclock_checker_done ())
{
dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
gl_recursive_lock_lock (my_reclock);
@@ -396,7 +479,7 @@ test_recursive_lock (void)
/* Wait for the threads to terminate. */
for (i = 0; i < THREAD_COUNT; i++)
gl_thread_join (threads[i], NULL);
- reclock_checker_done = 1;
+ set_reclock_checker_done ();
gl_thread_join (checkerthread, NULL);
check_accounts ();
}
