diff mbox

[v6,05/18] tests: adjust test-thread-pool to new aio_poll() semantics

Message ID 1374765505-14356-6-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi July 25, 2013, 3:18 p.m. UTC
aio_poll(ctx, true) will soon block when fd handlers have been set.
Previously aio_poll() would return early if all .io_flush() returned
false.  This means we need to check the equivalent of the .io_flush()
condition *before* calling aio_poll(ctx, true) to avoid deadlock.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-thread-pool.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Wayne Xia July 29, 2013, 7:51 a.m. UTC | #1
I am confused for sometime about the reason of the change, I think
it can be concluded as: "The meaning of aio_poll() will be changed, so
change the caller in this test case."

Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>


> aio_poll(ctx, true) will soon block when fd handlers have been set.
> Previously aio_poll() would return early if all .io_flush() returned
> false.  This means we need to check the equivalent of the .io_flush()
> condition *before* calling aio_poll(ctx, true) to avoid deadlock.
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   tests/test-thread-pool.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
> index b62338f..8188d1a 100644
> --- a/tests/test-thread-pool.c
> +++ b/tests/test-thread-pool.c
> @@ -40,19 +40,13 @@ static void done_cb(void *opaque, int ret)
>       active--;
>   }
> 
> -/* Wait until all aio and bh activity has finished */
> -static void qemu_aio_wait_all(void)
> -{
> -    while (aio_poll(ctx, true)) {
> -        /* Do nothing */
> -    }
> -}
> -
>   static void test_submit(void)
>   {
>       WorkerTestData data = { .n = 0 };
>       thread_pool_submit(pool, worker_cb, &data);
> -    qemu_aio_wait_all();
> +    while (data.n == 0) {
> +        aio_poll(ctx, true);
> +    }
>       g_assert_cmpint(data.n, ==, 1);
>   }
> 
> @@ -65,7 +59,9 @@ static void test_submit_aio(void)
>       /* The callbacks are not called until after the first wait.  */
>       active = 1;
>       g_assert_cmpint(data.ret, ==, -EINPROGRESS);
> -    qemu_aio_wait_all();
> +    while (data.ret == -EINPROGRESS) {
> +        aio_poll(ctx, true);
> +    }
>       g_assert_cmpint(active, ==, 0);
>       g_assert_cmpint(data.n, ==, 1);
>       g_assert_cmpint(data.ret, ==, 0);
> @@ -103,7 +99,9 @@ static void test_submit_co(void)
> 
>       /* qemu_aio_wait_all will execute the rest of the coroutine.  */
> 
> -    qemu_aio_wait_all();
> +    while (data.ret == -EINPROGRESS) {
> +        aio_poll(ctx, true);
> +    }
> 
>       /* Back here after the coroutine has finished.  */
> 
> @@ -187,7 +185,9 @@ static void test_cancel(void)
>       }
> 
>       /* Finish execution and execute any remaining callbacks.  */
> -    qemu_aio_wait_all();
> +    while (active > 0) {
> +        aio_poll(ctx, true);
> +    }
>       g_assert_cmpint(active, ==, 0);
>       for (i = 0; i < 100; i++) {
>           if (data[i].n == 3) {
>
Stefan Hajnoczi July 29, 2013, 2:26 p.m. UTC | #2
On Mon, Jul 29, 2013 at 03:51:08PM +0800, Wenchao Xia wrote:
> I am confused for sometime about the reason of the change, I think
> it can be concluded as: "The meaning of aio_poll() will be changed, so
> change the caller in this test case."

Exactly, that's what the subject line means.

Stefan
diff mbox

Patch

diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index b62338f..8188d1a 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -40,19 +40,13 @@  static void done_cb(void *opaque, int ret)
     active--;
 }
 
-/* Wait until all aio and bh activity has finished */
-static void qemu_aio_wait_all(void)
-{
-    while (aio_poll(ctx, true)) {
-        /* Do nothing */
-    }
-}
-
 static void test_submit(void)
 {
     WorkerTestData data = { .n = 0 };
     thread_pool_submit(pool, worker_cb, &data);
-    qemu_aio_wait_all();
+    while (data.n == 0) {
+        aio_poll(ctx, true);
+    }
     g_assert_cmpint(data.n, ==, 1);
 }
 
@@ -65,7 +59,9 @@  static void test_submit_aio(void)
     /* The callbacks are not called until after the first wait.  */
     active = 1;
     g_assert_cmpint(data.ret, ==, -EINPROGRESS);
-    qemu_aio_wait_all();
+    while (data.ret == -EINPROGRESS) {
+        aio_poll(ctx, true);
+    }
     g_assert_cmpint(active, ==, 0);
     g_assert_cmpint(data.n, ==, 1);
     g_assert_cmpint(data.ret, ==, 0);
@@ -103,7 +99,9 @@  static void test_submit_co(void)
 
     /* qemu_aio_wait_all will execute the rest of the coroutine.  */
 
-    qemu_aio_wait_all();
+    while (data.ret == -EINPROGRESS) {
+        aio_poll(ctx, true);
+    }
 
     /* Back here after the coroutine has finished.  */
 
@@ -187,7 +185,9 @@  static void test_cancel(void)
     }
 
     /* Finish execution and execute any remaining callbacks.  */
-    qemu_aio_wait_all();
+    while (active > 0) {
+        aio_poll(ctx, true);
+    }
     g_assert_cmpint(active, ==, 0);
     for (i = 0; i < 100; i++) {
         if (data[i].n == 3) {