apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin <share-apa...@think42.com>
Subject Re: Patch 1 for apr_atomic.c (Solaris 10)
Date Mon, 30 Oct 2006 09:28:31 GMT
On Mon, Oct 30, 2006 at 01:48:50AM -0700, Justin Erenkrantz wrote:
> On 10/30/06, Colin Hirsch <mail@cohi.at> wrote:
> >Hm ... to trigger the bug in apr_atomic_inc32 without waiting for hours one
> >would need at least two CPUs, each performing an apr_atomic_inc32 in a 
> >tight
> >loop; one would then need to save all return values, and check whether 
> >every
> >number occurs only once.
> >
> >Of course this test is still not 100% deterministic...
> >
> >To make it work reasonably well one would need to have large arrays for the
> >return values, one per thread, all pages touched by memset; and as 
> >simultaneous
> >a start of the threads as possible, probably via a broadcast on a condition
> >variable.
> >
> >Any simpler ideas?
> 
> No, but I have a 32-way T2000 and a 4-way SMP x86 box at my disposal.
> So, those HW conditions should be fairly easy to satisfy.  -- justin

I just looked at testatomic ... there's a function test_atomics_threaded,
but I wonder how much actually gets tested with only 20000 iterations and
two other threads being created before another thread works on the same
variable...!

The following patch against testatomic adds a test for atomic_inc32 that
should fail on Solaris 10 with the old implementation, and succeed with
my new one... It also cranks up the number of iterations, and only runs
one test at a time, to increase the possibility of catching problems.

Ciao, Colin


Index: testatomic.c
===================================================================
--- testatomic.c	(revision 468850)
+++ testatomic.c	(working copy)
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <stdlib.h>
+
 #include "testutil.h"
 #include "apr_strings.h"
 #include "apr_thread_proc.h"
@@ -187,11 +189,12 @@
 volatile apr_uint32_t z = 0; /* no locks */
 apr_status_t exit_ret_val = 123; /* just some made up number to check on later */
 
-#define NUM_THREADS 40
-#define NUM_ITERATIONS 20000
+#define NUM_THREADS 8
+#define NUM_ITERATIONS 200000
+
 void * APR_THREAD_FUNC thread_func_mutex(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS; i++) {
         apr_thread_mutex_lock(thread_lock);
@@ -204,7 +207,7 @@
 
 void * APR_THREAD_FUNC thread_func_atomic(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS ; i++) {
         apr_atomic_inc32(&y);
@@ -216,9 +219,37 @@
     return NULL;
 }
 
+int test_array_compare(const void * l, const void * r)
+{
+   apr_uint32_t a = *(const apr_uint32_t *)l;
+   apr_uint32_t b = *(const apr_uint32_t *)r;
+
+   if (a < b) {
+      return -1;
+   }
+   else {
+      return a > b;
+   }
+}
+
+apr_uint32_t atomic_test_array[ NUM_ITERATIONS * NUM_THREADS ];
+
+void * APR_THREAD_FUNC thread_func_atomic_inc(apr_thread_t *thd, void *data)
+{
+   unsigned i;
+
+   apr_uint32_t * array = (apr_uint32_t *)data;
+
+   for ( i = 0; i < NUM_ITERATIONS; ++i ) {
+      array[ i ] = apr_atomic_inc32(&y);
+   }
+   apr_thread_exit(thd, exit_ret_val);
+   return NULL;
+}
+
 void * APR_THREAD_FUNC thread_func_none(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS ; i++) {
         z++;
@@ -230,13 +261,9 @@
 static void test_atomics_threaded(abts_case *tc, void *data)
 {
     apr_thread_t *t1[NUM_THREADS];
-    apr_thread_t *t2[NUM_THREADS];
-    apr_thread_t *t3[NUM_THREADS];
     apr_status_t s1[NUM_THREADS]; 
-    apr_status_t s2[NUM_THREADS];
-    apr_status_t s3[NUM_THREADS];
     apr_status_t rv;
-    int i;
+    unsigned i;
 
 #ifdef HAVE_PTHREAD_SETCONCURRENCY
     pthread_setconcurrency(8);
@@ -246,27 +273,77 @@
     APR_ASSERT_SUCCESS(tc, "Could not create lock", rv);
 
     for (i = 0; i < NUM_THREADS; i++) {
-        apr_status_t r1, r2, r3;
+        apr_status_t r1;
         r1 = apr_thread_create(&t1[i], NULL, thread_func_mutex, NULL, p);
-        r2 = apr_thread_create(&t2[i], NULL, thread_func_atomic, NULL, p);
-        r3 = apr_thread_create(&t3[i], NULL, thread_func_none, NULL, p);
-        ABTS_ASSERT(tc, "Failed creating threads",
-                 r1 == APR_SUCCESS && r2 == APR_SUCCESS && 
-                 r3 == APR_SUCCESS);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
     }
 
     for (i = 0; i < NUM_THREADS; i++) {
         apr_thread_join(&s1[i], t1[i]);
-        apr_thread_join(&s2[i], t2[i]);
-        apr_thread_join(&s3[i], t3[i]);
                      
         ABTS_ASSERT(tc, "Invalid return value from thread_join",
-                 s1[i] == exit_ret_val && s2[i] == exit_ret_val && 
-                 s3[i] == exit_ret_val);
+                    s1[i] == exit_ret_val );
     }
 
     ABTS_INT_EQUAL(tc, x, NUM_THREADS * NUM_ITERATIONS);
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_atomic, NULL, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
     ABTS_INT_EQUAL(tc, apr_atomic_read32(&y), NUM_THREADS * NUM_ITERATIONS);
+
+    y = 0;
+
+    for (i = 0; i < NUM_THREADS * NUM_ITERATIONS; ++i ) {
+       atomic_test_array[ i ] = 0;
+    }       
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_atomic_inc, atomic_test_array
+ i * NUM_ITERATIONS, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
+    qsort( atomic_test_array, NUM_THREADS * NUM_ITERATIONS, sizeof( apr_uint32_t ), &
test_array_compare );
+
+    y = 0;
+
+    for (i = 0; i < NUM_THREADS * NUM_ITERATIONS - 1; ++i ) {
+       y += ( atomic_test_array[ i ] == atomic_test_array[ i + 1 ] );
+    }
+
+    ABTS_ASSERT(tc, "Failed atomic increment return values", y == 0 );
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_none, NULL, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
     /* Comment out this test, because I have no clue what this test is
      * actually telling us.  We are checking something that may or may not
      * be true, and it isn't really testing APR at all.

Mime
View raw message