| Message ID | 20200610144129.27659-2-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: seriously improve savevm performance | expand |
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 <den@openvz.org> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Max Reitz <mreitz@redhat.com> > CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > CC: Denis Plotnikov <dplotnikov@virtuozzo.com> > --- > 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.
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 <den@openvz.org> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Max Reitz <mreitz@redhat.com> >> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> CC: Denis Plotnikov <dplotnikov@virtuozzo.com> >> --- >> 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
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;
The patch preserves the constraint that the only waiter is allowed. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Max Reitz <mreitz@redhat.com> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> CC: Denis Plotnikov <dplotnikov@virtuozzo.com> --- block/aio_task.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)