[Qemu-devel] [PATCH] test-thread-pool: be more reliable

Paolo Bonzini posted 1 patch 9 weeks ago
Test checkpatch passed
Test docker-clang@ubuntu passed
Test asan passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190314182535.18332-1-pbonzini@redhat.com
tests/test-thread-pool.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH] test-thread-pool: be more reliable

Posted by Paolo Bonzini 9 weeks ago
There is a rare race between the atomic_cmpxchg and
bdrv_aio_cancel/bdrv_aio_cancel_async invocations.  Detect it, the
only sensible we can do about it is to exit long_cb immediately.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/test-thread-pool.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index 9cdccb3a47..0b675923f6 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -27,9 +27,10 @@ static int worker_cb(void *opaque)
 static int long_cb(void *opaque)
 {
     WorkerTestData *data = opaque;
-    atomic_inc(&data->n);
-    g_usleep(2000000);
-    atomic_inc(&data->n);
+    if (atomic_cmpxchg(&data->n, 0, 1) == 0) {
+        g_usleep(2000000);
+        atomic_or(&data->n, 2);
+    }
     return 0;
 }
 
@@ -171,7 +172,7 @@ static void do_test_cancel(bool sync)
     /* Cancel the jobs that haven't been started yet.  */
     num_canceled = 0;
     for (i = 0; i < 100; i++) {
-        if (atomic_cmpxchg(&data[i].n, 0, 3) == 0) {
+        if (atomic_cmpxchg(&data[i].n, 0, 4) == 0) {
             data[i].ret = -ECANCELED;
             if (sync) {
                 bdrv_aio_cancel(data[i].aiocb);
@@ -185,7 +186,7 @@ static void do_test_cancel(bool sync)
     g_assert_cmpint(num_canceled, <, 100);
 
     for (i = 0; i < 100; i++) {
-        if (data[i].aiocb && data[i].n != 3) {
+        if (data[i].aiocb && atomic_read(&data[i].n) < 4) {
             if (sync) {
                 /* Canceling the others will be a blocking operation.  */
                 bdrv_aio_cancel(data[i].aiocb);
@@ -201,13 +202,22 @@ static void do_test_cancel(bool sync)
     }
     g_assert_cmpint(active, ==, 0);
     for (i = 0; i < 100; i++) {
-        if (data[i].n == 3) {
+        g_assert(data[i].aiocb == NULL);
+        switch (data[i].n) {
+        case 0:
+            fprintf(stderr, "Callback not canceled but never started?\n");
+            abort();
+        case 3:
+            /* Couldn't be canceled asynchronously, must have completed.  */
+            g_assert_cmpint(data[i].ret, ==, 0);
+            break;
+        case 4:
+            /* Could be canceled asynchronously, never started.  */
             g_assert_cmpint(data[i].ret, ==, -ECANCELED);
-            g_assert(data[i].aiocb == NULL);
-        } else {
-            g_assert_cmpint(data[i].n, ==, 2);
-            g_assert(data[i].ret == 0 || data[i].ret == -ECANCELED);
-            g_assert(data[i].aiocb == NULL);
+            break;
+        default:
+            fprintf(stderr, "Callback aborted while running?\n");
+            abort();
         }
     }
 }
-- 
2.20.1