Re: [PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine - please ignore

2020-06-10 Thread Denis V. Lunev
On 6/10/20 9:58 PM, Denis V. Lunev wrote:
> The patch preserves the constraint that the only waiter is allowed.
>
> The patch renames AioTaskPool->main_co to wake_co and removes
> AioTaskPool->waiting flag. wake_co keeps coroutine, which is
> waiting for wakeup on worker completion. Thus 'waiting' flag
> in this semantics is equivalent to 'wake_co != NULL'.
>
> Signed-off-by: Denis V. Lunev 
> CC: Kevin Wolf 
> CC: Max Reitz 
> CC: Vladimir Sementsov-Ogievskiy 
> CC: Denis Plotnikov 
> ---
>  block/aio_task.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/block/aio_task.c b/block/aio_task.c
> index 88989fa248..5183b0729d 100644
> --- a/block/aio_task.c
> +++ b/block/aio_task.c
> @@ -27,11 +27,10 @@
>  #include "block/aio_task.h"
>  
>  struct AioTaskPool {
> -Coroutine *main_co;
> +Coroutine *wake_co;
>  int status;
>  int max_busy_tasks;
>  int busy_tasks;
> -bool waiting;
>  };
>  
>  static void coroutine_fn aio_task_co(void *opaque)
> @@ -52,21 +51,21 @@ static void coroutine_fn aio_task_co(void *opaque)
>  
>  g_free(task);
>  
> -if (pool->waiting) {
> -pool->waiting = false;
> -aio_co_wake(pool->main_co);
> +if (pool->wake_co != NULL) {
> +aio_co_wake(pool->wake_co);
> +pool->wake_co = NULL;
>  }
>  }
>  
>  void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
>  {
>  assert(pool->busy_tasks > 0);
> -assert(qemu_coroutine_self() == pool->main_co);
> +assert(pool->wake_co == NULL);
>  
> -pool->waiting = true;
> +pool->wake_co = qemu_coroutine_self();
>  qemu_coroutine_yield();
>  
> -assert(!pool->waiting);
> +assert(pool->wake_co == NULL);
>  assert(pool->busy_tasks < pool->max_busy_tasks);
>  }
>  
> @@ -98,7 +97,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int 
> max_busy_tasks)
>  {
>  AioTaskPool *pool = g_new0(AioTaskPool, 1);
>  
> -pool->main_co = qemu_coroutine_self();
> +pool->wake_co = NULL;
>  pool->max_busy_tasks = max_busy_tasks;
>  
>  return pool;
please ignore



[PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine

2020-06-10 Thread Denis V. Lunev
The patch preserves the constraint that the only waiter is allowed.

The patch renames AioTaskPool->main_co to wake_co and removes
AioTaskPool->waiting flag. wake_co keeps coroutine, which is
waiting for wakeup on worker completion. Thus 'waiting' flag
in this semantics is equivalent to 'wake_co != NULL'.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Vladimir Sementsov-Ogievskiy 
CC: Denis Plotnikov 
---
 block/aio_task.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/block/aio_task.c b/block/aio_task.c
index 88989fa248..5183b0729d 100644
--- a/block/aio_task.c
+++ b/block/aio_task.c
@@ -27,11 +27,10 @@
 #include "block/aio_task.h"
 
 struct AioTaskPool {
-Coroutine *main_co;
+Coroutine *wake_co;
 int status;
 int max_busy_tasks;
 int busy_tasks;
-bool waiting;
 };
 
 static void coroutine_fn aio_task_co(void *opaque)
@@ -52,21 +51,21 @@ static void coroutine_fn aio_task_co(void *opaque)
 
 g_free(task);
 
-if (pool->waiting) {
-pool->waiting = false;
-aio_co_wake(pool->main_co);
+if (pool->wake_co != NULL) {
+aio_co_wake(pool->wake_co);
+pool->wake_co = NULL;
 }
 }
 
 void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
 {
 assert(pool->busy_tasks > 0);
-assert(qemu_coroutine_self() == pool->main_co);
+assert(pool->wake_co == NULL);
 
-pool->waiting = true;
+pool->wake_co = qemu_coroutine_self();
 qemu_coroutine_yield();
 
-assert(!pool->waiting);
+assert(pool->wake_co == NULL);
 assert(pool->busy_tasks < pool->max_busy_tasks);
 }
 
@@ -98,7 +97,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int 
max_busy_tasks)
 {
 AioTaskPool *pool = g_new0(AioTaskPool, 1);
 
-pool->main_co = qemu_coroutine_self();
+pool->wake_co = NULL;
 pool->max_busy_tasks = max_busy_tasks;
 
 return pool;
-- 
2.17.1




[PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine

2020-06-10 Thread Denis V. Lunev
The patch preserves the constraint that the only waiter is allowed.

The patch renames AioTaskPool->main_co to wake_co and removes
AioTaskPool->waiting flag. wake_co keeps coroutine, which is
waiting for wakeup on worker completion. Thus 'waiting' flag
in this semantics is equivalent to 'wake_co != NULL'.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Vladimir Sementsov-Ogievskiy 
CC: Denis Plotnikov 
---
 block/aio_task.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/block/aio_task.c b/block/aio_task.c
index 88989fa248..5183b0729d 100644
--- a/block/aio_task.c
+++ b/block/aio_task.c
@@ -27,11 +27,10 @@
 #include "block/aio_task.h"
 
 struct AioTaskPool {
-Coroutine *main_co;
+Coroutine *wake_co;
 int status;
 int max_busy_tasks;
 int busy_tasks;
-bool waiting;
 };
 
 static void coroutine_fn aio_task_co(void *opaque)
@@ -52,21 +51,21 @@ static void coroutine_fn aio_task_co(void *opaque)
 
 g_free(task);
 
-if (pool->waiting) {
-pool->waiting = false;
-aio_co_wake(pool->main_co);
+if (pool->wake_co != NULL) {
+aio_co_wake(pool->wake_co);
+pool->wake_co = NULL;
 }
 }
 
 void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
 {
 assert(pool->busy_tasks > 0);
-assert(qemu_coroutine_self() == pool->main_co);
+assert(pool->wake_co == NULL);
 
-pool->waiting = true;
+pool->wake_co = qemu_coroutine_self();
 qemu_coroutine_yield();
 
-assert(!pool->waiting);
+assert(pool->wake_co == NULL);
 assert(pool->busy_tasks < pool->max_busy_tasks);
 }
 
@@ -98,7 +97,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int 
max_busy_tasks)
 {
 AioTaskPool *pool = g_new0(AioTaskPool, 1);
 
-pool->main_co = qemu_coroutine_self();
+pool->wake_co = NULL;
 pool->max_busy_tasks = max_busy_tasks;
 
 return pool;
-- 
2.17.1




Re: [PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine

2020-06-10 Thread Denis V. Lunev
On 6/10/20 6:10 PM, Vladimir Sementsov-Ogievskiy wrote:
> 10.06.2020 17:41, Denis V. Lunev wrote:
>> The patch preserves the constraint that the only waiter is allowed.
>>
>> Signed-off-by: Denis V. Lunev 
>> CC: Kevin Wolf 
>> CC: Max Reitz 
>> CC: Vladimir Sementsov-Ogievskiy 
>> CC: Denis Plotnikov 
>> ---
>>   block/aio_task.c | 8 
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/aio_task.c b/block/aio_task.c
>> index 88989fa248..f338049147 100644
>> --- a/block/aio_task.c
>> +++ b/block/aio_task.c
>> @@ -27,7 +27,7 @@
>>   #include "block/aio_task.h"
>>     struct AioTaskPool {
>> -    Coroutine *main_co;
>> +    Coroutine *wake_co;
>>   int status;
>>   int max_busy_tasks;
>>   int busy_tasks;
>> @@ -54,15 +54,15 @@ static void coroutine_fn aio_task_co(void *opaque)
>>     if (pool->waiting) {
>>   pool->waiting = false;
>> -    aio_co_wake(pool->main_co);
>> +    aio_co_wake(pool->wake_co);
>>   }
>>   }
>>     void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
>>   {
>>   assert(pool->busy_tasks > 0);
>> -    assert(qemu_coroutine_self() == pool->main_co);
>>   +    pool->wake_co = qemu_coroutine_self();
>>   pool->waiting = true;
>>   qemu_coroutine_yield();
>>   @@ -98,7 +98,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int
>> max_busy_tasks)
>>   {
>>   AioTaskPool *pool = g_new0(AioTaskPool, 1);
>>   -    pool->main_co = qemu_coroutine_self();
>> +    pool->wake_co = NULL;
>>   pool->max_busy_tasks = max_busy_tasks;
>>     return pool;
>>
>
> With such approach, if several coroutines will wait simultaneously,
> the only one will be finally woken and other will hang.
>
> I think, we should use CoQueue here: CoQueue instead of wake_co,
> qemu_co_queue_wait in wait_one, and qemu_co_queue_next instead of
> aio_co_wake.
>
>
I will make a check, but for now it would be enough to
add
  assert(!pool->waiting);
at the beginning of aio_task_pool_wait_one

Den



Re: [PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine

2020-06-10 Thread Vladimir Sementsov-Ogievskiy

10.06.2020 17:41, Denis V. Lunev wrote:

The patch preserves the constraint that the only waiter is allowed.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Vladimir Sementsov-Ogievskiy 
CC: Denis Plotnikov 
---
  block/aio_task.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/aio_task.c b/block/aio_task.c
index 88989fa248..f338049147 100644
--- a/block/aio_task.c
+++ b/block/aio_task.c
@@ -27,7 +27,7 @@
  #include "block/aio_task.h"
  
  struct AioTaskPool {

-Coroutine *main_co;
+Coroutine *wake_co;
  int status;
  int max_busy_tasks;
  int busy_tasks;
@@ -54,15 +54,15 @@ static void coroutine_fn aio_task_co(void *opaque)
  
  if (pool->waiting) {

  pool->waiting = false;
-aio_co_wake(pool->main_co);
+aio_co_wake(pool->wake_co);
  }
  }
  
  void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)

  {
  assert(pool->busy_tasks > 0);
-assert(qemu_coroutine_self() == pool->main_co);
  
+pool->wake_co = qemu_coroutine_self();

  pool->waiting = true;
  qemu_coroutine_yield();
  
@@ -98,7 +98,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int max_busy_tasks)

  {
  AioTaskPool *pool = g_new0(AioTaskPool, 1);
  
-pool->main_co = qemu_coroutine_self();

+pool->wake_co = NULL;
  pool->max_busy_tasks = max_busy_tasks;
  
  return pool;




With such approach, if several coroutines will wait simultaneously, the only 
one will be finally woken and other will hang.

I think, we should use CoQueue here: CoQueue instead of wake_co, 
qemu_co_queue_wait in wait_one, and qemu_co_queue_next instead of aio_co_wake.


--
Best regards,
Vladimir



[PATCH 1/2] aio: allow to wait for coroutine pool from different coroutine

2020-06-10 Thread Denis V. Lunev
The patch preserves the constraint that the only waiter is allowed.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Vladimir Sementsov-Ogievskiy 
CC: Denis Plotnikov 
---
 block/aio_task.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/aio_task.c b/block/aio_task.c
index 88989fa248..f338049147 100644
--- a/block/aio_task.c
+++ b/block/aio_task.c
@@ -27,7 +27,7 @@
 #include "block/aio_task.h"
 
 struct AioTaskPool {
-Coroutine *main_co;
+Coroutine *wake_co;
 int status;
 int max_busy_tasks;
 int busy_tasks;
@@ -54,15 +54,15 @@ static void coroutine_fn aio_task_co(void *opaque)
 
 if (pool->waiting) {
 pool->waiting = false;
-aio_co_wake(pool->main_co);
+aio_co_wake(pool->wake_co);
 }
 }
 
 void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
 {
 assert(pool->busy_tasks > 0);
-assert(qemu_coroutine_self() == pool->main_co);
 
+pool->wake_co = qemu_coroutine_self();
 pool->waiting = true;
 qemu_coroutine_yield();
 
@@ -98,7 +98,7 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int 
max_busy_tasks)
 {
 AioTaskPool *pool = g_new0(AioTaskPool, 1);
 
-pool->main_co = qemu_coroutine_self();
+pool->wake_co = NULL;
 pool->max_busy_tasks = max_busy_tasks;
 
 return pool;
-- 
2.17.1