From e7e4f9f950253ce5fb03f569868a21d6d7e3f8a2 Mon Sep 17 00:00:00 2001 From: Yaowei Bai Date: Wed, 14 Sep 2016 07:03:38 -0400 Subject: block: mirror: fix wrong comment of mirror_start Obviously, we should write to '@target'. Signed-off-by: Yaowei Bai Reviewed-by: Xiubo Li Reviewed-by: Eric Blake Message-id: 1473851019-7005-2-git-send-email-baiyaowei@cmss.chinamobile.com Signed-off-by: Stefan Hajnoczi --- include/block/block_int.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index ef3c047cb3..3e79228eb0 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -722,7 +722,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, * @errp: Error object. * * Start a mirroring operation on @bs. Clusters that are allocated - * in @bs will be written to @bs until the job is cancelled or + * in @bs will be written to @target until the job is cancelled or * manually completed. At the end of a successful mirroring job, * @bs will be switched to read from @target. */ -- cgit v1.2.3 From 6b9424689a32bb76942ec39f6d5c60b72eb002e0 Mon Sep 17 00:00:00 2001 From: Yaowei Bai Date: Wed, 14 Sep 2016 07:03:39 -0400 Subject: aio-posix: avoid unnecessary aio_epoll_enabled() calls As epoll whether enabled or not is a global setting, we can just check it only once rather than checking it with every node iteration. Through this we can avoid a lot of checks when epoll is not enabled. Signed-off-by: Yaowei Bai Reviewed-by: Xiubo Li Message-id: 1473851019-7005-3-git-send-email-baiyaowei@cmss.chinamobile.com Signed-off-by: Stefan Hajnoczi --- aio-posix.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/aio-posix.c b/aio-posix.c index 43162a9f29..4ef34dd175 100644 --- a/aio-posix.c +++ b/aio-posix.c @@ -431,11 +431,13 @@ bool aio_poll(AioContext *ctx, bool blocking) assert(npfd == 0); /* fill pollfds */ - QLIST_FOREACH(node, &ctx->aio_handlers, node) { - if (!node->deleted && node->pfd.events - && !aio_epoll_enabled(ctx) - && aio_node_check(ctx, node->is_external)) { - add_pollfd(node); + + if (!aio_epoll_enabled(ctx)) { + QLIST_FOREACH(node, &ctx->aio_handlers, node) { + if (!node->deleted && node->pfd.events + && aio_node_check(ctx, node->is_external)) { + add_pollfd(node); + } } } -- cgit v1.2.3 From eb7b5c35117bfb90dcfa881a0e3b23368c73fba6 Mon Sep 17 00:00:00 2001 From: Lin Ma Date: Mon, 26 Sep 2016 13:29:58 +0800 Subject: iothread: check iothread->ctx before aio_context_unref to avoid assertion if iothread->ctx is set to NULL, aio_context_unref triggers the assertion: g_source_unref: assertion 'source != NULL' failed. The patch fixes it. Signed-off-by: Lin Ma Reviewed-by: Paolo Bonzini Message-id: 20160926052958.10716-1-lma@suse.com Signed-off-by: Stefan Hajnoczi --- iothread.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iothread.c b/iothread.c index fb08a60b4b..fbeb8deb38 100644 --- a/iothread.c +++ b/iothread.c @@ -75,6 +75,9 @@ static void iothread_instance_finalize(Object *obj) iothread_stop(obj, NULL); qemu_cond_destroy(&iothread->init_done_cond); qemu_mutex_destroy(&iothread->init_done_lock); + if (!iothread->ctx) { + return; + } aio_context_unref(iothread->ctx); } -- cgit v1.2.3 From 844c82296f8bd6ceff8d170f3740740f3ce84b73 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Mon, 26 Sep 2016 17:24:10 +0200 Subject: libqos: fix qvring_init() "vq->desc[i].addr" is a 64bit value, so write it with writeq(), not writew(). struct vring_desc { __virtio64 addr; __virtio32 len; __virtio16 flags; __virtio16 next; }; Signed-off-by: Laurent Vivier Message-id: 1474903450-9605-1-git-send-email-lvivier@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/libqos/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c index 37ff860c16..105bccecaa 100644 --- a/tests/libqos/virtio.c +++ b/tests/libqos/virtio.c @@ -147,7 +147,7 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr) for (i = 0; i < vq->size - 1; i++) { /* vq->desc[i].addr */ - writew(vq->desc + (16 * i), 0); + writeq(vq->desc + (16 * i), 0); /* vq->desc[i].next */ writew(vq->desc + (16 * i) + 14, i + 1); } -- cgit v1.2.3 From f643e469f31df735fd5c42ab317f96ebfe749871 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 27 Sep 2016 16:18:34 +0100 Subject: coroutine: add qemu_coroutine_entered() function See the doc comments for a description of this new coroutine API. Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng Message-id: 1474989516-18255-2-git-send-email-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- include/qemu/coroutine.h | 13 +++++++++++++ util/qemu-coroutine.c | 5 +++++ 2 files changed, 18 insertions(+) diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 29a20782f0..e6a60d55fd 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -92,6 +92,19 @@ Coroutine *coroutine_fn qemu_coroutine_self(void); */ bool qemu_in_coroutine(void); +/** + * Return true if the coroutine is currently entered + * + * A coroutine is "entered" if it has not yielded from the current + * qemu_coroutine_enter() call used to run it. This does not mean that the + * coroutine is currently executing code since it may have transferred control + * to another coroutine using qemu_coroutine_enter(). + * + * When several coroutines enter each other there may be no way to know which + * ones have already been entered. In such situations this function can be + * used to avoid recursively entering coroutines. + */ +bool qemu_coroutine_entered(Coroutine *co); /** diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c index 3cbf225487..737bffa984 100644 --- a/util/qemu-coroutine.c +++ b/util/qemu-coroutine.c @@ -146,3 +146,8 @@ void coroutine_fn qemu_coroutine_yield(void) self->caller = NULL; qemu_coroutine_switch(self, to, COROUTINE_YIELD); } + +bool qemu_coroutine_entered(Coroutine *co) +{ + return co->caller; +} -- cgit v1.2.3 From afe16f3f477bc656932a640a09769db486446f07 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 27 Sep 2016 16:18:35 +0100 Subject: test-coroutine: test qemu_coroutine_entered() Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng Message-id: 1474989516-18255-3-git-send-email-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/test-coroutine.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c index 6431dd6d7c..abd97c23c1 100644 --- a/tests/test-coroutine.c +++ b/tests/test-coroutine.c @@ -52,6 +52,47 @@ static void test_self(void) qemu_coroutine_enter(coroutine); } +/* + * Check that qemu_coroutine_entered() works + */ + +static void coroutine_fn verify_entered_step_2(void *opaque) +{ + Coroutine *caller = (Coroutine *)opaque; + + g_assert(qemu_coroutine_entered(caller)); + g_assert(qemu_coroutine_entered(qemu_coroutine_self())); + qemu_coroutine_yield(); + + /* Once more to check it still works after yielding */ + g_assert(qemu_coroutine_entered(caller)); + g_assert(qemu_coroutine_entered(qemu_coroutine_self())); + qemu_coroutine_yield(); +} + +static void coroutine_fn verify_entered_step_1(void *opaque) +{ + Coroutine *self = qemu_coroutine_self(); + Coroutine *coroutine; + + g_assert(qemu_coroutine_entered(self)); + + coroutine = qemu_coroutine_create(verify_entered_step_2, self); + g_assert(!qemu_coroutine_entered(coroutine)); + qemu_coroutine_enter(coroutine); + g_assert(!qemu_coroutine_entered(coroutine)); + qemu_coroutine_enter(coroutine); +} + +static void test_entered(void) +{ + Coroutine *coroutine; + + coroutine = qemu_coroutine_create(verify_entered_step_1, NULL); + g_assert(!qemu_coroutine_entered(coroutine)); + qemu_coroutine_enter(coroutine); +} + /* * Check that coroutines may nest multiple levels */ @@ -389,6 +430,7 @@ int main(int argc, char **argv) g_test_add_func("/basic/yield", test_yield); g_test_add_func("/basic/nesting", test_nesting); g_test_add_func("/basic/self", test_self); + g_test_add_func("/basic/entered", test_entered); g_test_add_func("/basic/in_coroutine", test_in_coroutine); g_test_add_func("/basic/order", test_order); if (g_test_perf()) { -- cgit v1.2.3 From fe121b9d3c4258e41f7efa4976bf79151b2d5dbb Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 27 Sep 2016 16:18:36 +0100 Subject: linux-aio: fix re-entrant completion processing Commit 0ed93d84edabc7656f5c998ae1a346fe8b94ca54 ("linux-aio: process completions from ioq_submit()") added an optimization that processes completions each time ioq_submit() returns with requests in flight. This commit introduces a "Co-routine re-entered recursively" error which can be triggered with -drive format=qcow2,aio=native. Fam Zheng , Kevin Wolf , and I debugged the following backtrace: (gdb) bt #0 0x00007ffff0a046f5 in raise () at /lib64/libc.so.6 #1 0x00007ffff0a062fa in abort () at /lib64/libc.so.6 #2 0x0000555555ac0013 in qemu_coroutine_enter (co=0x5555583464d0) at util/qemu-coroutine.c:113 #3 0x0000555555a4b663 in qemu_laio_process_completions (s=s@entry=0x555557e2f7f0) at block/linux-aio.c:218 #4 0x0000555555a4b874 in ioq_submit (s=s@entry=0x555557e2f7f0) at block/linux-aio.c:331 #5 0x0000555555a4ba12 in laio_do_submit (fd=fd@entry=13, laiocb=laiocb@entry=0x555559d38ae0, offset=offset@entry=2932727808, type=type@entry=1) at block/linux-aio.c:383 #6 0x0000555555a4bbd3 in laio_co_submit (bs=, s=0x555557e2f7f0, fd=13, offset=2932727808, qiov=0x555559d38e20, type=1) at block/linux-aio.c:402 #7 0x0000555555a4fd23 in bdrv_driver_preadv (bs=bs@entry=0x55555663bcb0, offset=offset@entry=2932727808, bytes=bytes@entry=8192, qiov=qiov@entry=0x555559d38e20, flags=0) at block/io.c:804 #8 0x0000555555a52b34 in bdrv_aligned_preadv (bs=bs@entry=0x55555663bcb0, req=req@entry=0x555559d38d20, offset=offset@entry=2932727808, bytes=bytes@entry=8192, align=align@entry=512, qiov=qiov@entry=0x555559d38e20, flags=0) at block/io.c:1041 #9 0x0000555555a52db8 in bdrv_co_preadv (child=, offset=2932727808, bytes=8192, qiov=qiov@entry=0x555559d38e20, flags=flags@entry=0) at block/io.c:1133 #10 0x0000555555a29629 in qcow2_co_preadv (bs=0x555556635890, offset=6178725888, bytes=8192, qiov=0x555557527840, flags=) at block/qcow2.c:1509 #11 0x0000555555a4fd23 in bdrv_driver_preadv (bs=bs@entry=0x555556635890, offset=offset@entry=6178725888, bytes=bytes@entry=8192, qiov=qiov@entry=0x555557527840, flags=0) at block/io.c:804 #12 0x0000555555a52b34 in bdrv_aligned_preadv (bs=bs@entry=0x555556635890, req=req@entry=0x555559d39000, offset=offset@entry=6178725888, bytes=bytes@entry=8192, align=align@entry=1, qiov=qiov@entry=0x555557527840, flags=0) at block/io.c:1041 #13 0x0000555555a52db8 in bdrv_co_preadv (child=, offset=offset@entry=6178725888, bytes=bytes@entry=8192, qiov=qiov@entry=0x555557527840, flags=flags@entry=0) at block/io.c:1133 #14 0x0000555555a4515a in blk_co_preadv (blk=0x5555566356d0, offset=6178725888, bytes=8192, qiov=0x555557527840, flags=0) at block/block-backend.c:783 #15 0x0000555555a45266 in blk_aio_read_entry (opaque=0x5555577025e0) at block/block-backend.c:991 #16 0x0000555555ac0cfa in coroutine_trampoline (i0=, i1=) at util/coroutine-ucontext.c:78 It turned out that re-entrant ioq_submit() and completion processing between three requests caused this error. The following check is not sufficient to prevent recursively entering coroutines: if (laiocb->co != qemu_coroutine_self()) { qemu_coroutine_enter(laiocb->co); } As the following coroutine backtrace shows, not just the current coroutine (self) can be entered. There might also be other coroutines that are currently entered and transferred control due to the qcow2 lock (CoMutex): (gdb) qemu coroutine 0x5555583464d0 #0 0x0000555555ac0c90 in qemu_coroutine_switch (from_=from_@entry=0x5555583464d0, to_=to_@entry=0x5555572f9890, action=action@entry=COROUTINE_ENTER) at util/coroutine-ucontext.c:175 #1 0x0000555555abfe54 in qemu_coroutine_enter (co=0x5555572f9890) at util/qemu-coroutine.c:117 #2 0x0000555555ac031c in qemu_co_queue_run_restart (co=co@entry=0x5555583462c0) at util/qemu-coroutine-lock.c:60 #3 0x0000555555abfe5e in qemu_coroutine_enter (co=0x5555583462c0) at util/qemu-coroutine.c:119 #4 0x0000555555a4b663 in qemu_laio_process_completions (s=s@entry=0x555557e2f7f0) at block/linux-aio.c:218 #5 0x0000555555a4b874 in ioq_submit (s=s@entry=0x555557e2f7f0) at block/linux-aio.c:331 #6 0x0000555555a4ba12 in laio_do_submit (fd=fd@entry=13, laiocb=laiocb@entry=0x55555a338b40, offset=offset@entry=2911477760, type=type@entry=1) at block/linux-aio.c:383 #7 0x0000555555a4bbd3 in laio_co_submit (bs=, s=0x555557e2f7f0, fd=13, offset=2911477760, qiov=0x55555a338e80, type=1) at block/linux-aio.c:402 #8 0x0000555555a4fd23 in bdrv_driver_preadv (bs=bs@entry=0x55555663bcb0, offset=offset@entry=2911477760, bytes=bytes@entry=8192, qiov=qiov@entry=0x55555a338e80, flags=0) at block/io.c:804 #9 0x0000555555a52b34 in bdrv_aligned_preadv (bs=bs@entry=0x55555663bcb0, req=req@entry=0x55555a338d80, offset=offset@entry=2911477760, bytes=bytes@entry=8192, align=align@entry=512, qiov=qiov@entry=0x55555a338e80, flags=0) at block/io.c:1041 #10 0x0000555555a52db8 in bdrv_co_preadv (child=, offset=2911477760, bytes=8192, qiov=qiov@entry=0x55555a338e80, flags=flags@entry=0) at block/io.c:1133 #11 0x0000555555a29629 in qcow2_co_preadv (bs=0x555556635890, offset=6157475840, bytes=8192, qiov=0x5555575df720, flags=) at block/qcow2.c:1509 #12 0x0000555555a4fd23 in bdrv_driver_preadv (bs=bs@entry=0x555556635890, offset=offset@entry=6157475840, bytes=bytes@entry=8192, qiov=qiov@entry=0x5555575df720, flags=0) at block/io.c:804 #13 0x0000555555a52b34 in bdrv_aligned_preadv (bs=bs@entry=0x555556635890, req=req@entry=0x55555a339060, offset=offset@entry=6157475840, bytes=bytes@entry=8192, align=align@entry=1, qiov=qiov@entry=0x5555575df720, flags=0) at block/io.c:1041 #14 0x0000555555a52db8 in bdrv_co_preadv (child=, offset=offset@entry=6157475840, bytes=bytes@entry=8192, qiov=qiov@entry=0x5555575df720, flags=flags@entry=0) at block/io.c:1133 #15 0x0000555555a4515a in blk_co_preadv (blk=0x5555566356d0, offset=6157475840, bytes=8192, qiov=0x5555575df720, flags=0) at block/block-backend.c:783 #16 0x0000555555a45266 in blk_aio_read_entry (opaque=0x555557231aa0) at block/block-backend.c:991 #17 0x0000555555ac0cfa in coroutine_trampoline (i0=, i1=) at util/coroutine-ucontext.c:78 Use the new qemu_coroutine_entered() function instead of comparing against qemu_coroutine_self(). This is correct because: 1. If a coroutine is not entered then it must have yielded to wait for I/O completion. It is therefore safe to enter. 2. If a coroutine is entered then it must be in ioq_submit()/qemu_laio_process_completions() because otherwise it would be yielded while waiting for I/O completion. Therefore it will check laio->ret and return from ioq_submit() instead of yielding, i.e. it's guaranteed not to hang. Reported-by: Fam Zheng Tested-by: Fam Zheng Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng Message-id: 1474989516-18255-4-git-send-email-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- block/linux-aio.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/block/linux-aio.c b/block/linux-aio.c index d4e19d444c..1685ec29a3 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -94,9 +94,12 @@ static void qemu_laio_process_completion(struct qemu_laiocb *laiocb) laiocb->ret = ret; if (laiocb->co) { - /* Jump and continue completion for foreign requests, don't do - * anything for current request, it will be completed shortly. */ - if (laiocb->co != qemu_coroutine_self()) { + /* If the coroutine is already entered it must be in ioq_submit() and + * will notice laio->ret has been filled in when it eventually runs + * later. Coroutines cannot be entered recursively so avoid doing + * that! + */ + if (!qemu_coroutine_entered(laiocb->co)) { qemu_coroutine_enter(laiocb->co); } } else { -- cgit v1.2.3