From 00a25291e6fbde6a174af07bc2cf2e9f0771c170 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 1 Mar 2019 16:35:18 +0000 Subject: [PATCH 1/7] MAINTAINERS: add missing support status fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds the "S:" line for areas of the codebase that currently lack a support status field. Note that there are a few more areas that are more abstract and do not correspond to a specific set of files. They have not been modified. Cc: Alex Bennée Signed-off-by: Stefan Hajnoczi Reviewed-by: Thomas Huth Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190301163518.20702-1-stefanha@redhat.com Message-Id: <20190301163518.20702-1-stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 074ad46d47..1ed8cf7ee7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -338,6 +338,7 @@ F: include/hw/tricore/ Multiarch Linux User Tests M: Alex Bennée +S: Maintained F: tests/tcg/multiarch/ Guest CPU Cores (KVM): @@ -2094,6 +2095,7 @@ F: qemu.sasl Coroutines M: Stefan Hajnoczi M: Kevin Wolf +S: Maintained F: util/*coroutine* F: include/qemu/coroutine* F: tests/test-coroutine.c @@ -2540,6 +2542,7 @@ F: .gitlab-ci.yml Guest Test Compilation Support M: Alex Bennée R: Philippe Mathieu-Daudé +S: Maintained F: tests/tcg/Makefile F: tests/tcg/Makefile.include L: qemu-devel@nongnu.org From bf4069fbd72ae11f6af82f9150d3396c8a95bb18 Mon Sep 17 00:00:00 2001 From: Anastasiia Rusakova Date: Thu, 7 Mar 2019 18:19:25 +0200 Subject: [PATCH 2/7] hw/block/virtio-blk: Clean req->dev repetitions Some functions sometimes uses req->dev even though a local variable VirtIOBlock* s = req->dev has already been defined. Updated places to use s everywhere in the file. Signed-off-by: Anastasiia Rusakova Message-id: 20190307161925.4158-1-rusakova.nastasia@icloud.com Message-Id: <20190307161925.4158-1-rusakova.nastasia@icloud.com> Signed-off-by: Stefan Hajnoczi --- hw/block/virtio-blk.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 0cc3c590b9..06e57a4d39 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -127,7 +127,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) } if (ret) { - int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type); + int p = virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type); bool is_read = !(p & VIRTIO_BLK_T_OUT); /* Note that memory may be dirtied on read failure. If the * virtio request is not completed here, as is the case for @@ -143,7 +143,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) } virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); - block_acct_done(blk_get_stats(req->dev->blk), &req->acct); + block_acct_done(blk_get_stats(s->blk), &req->acct); virtio_blk_free_request(req); } aio_context_release(blk_get_aio_context(s->conf.conf.blk)); @@ -260,9 +260,9 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req) { int status = VIRTIO_BLK_S_OK; struct virtio_scsi_inhdr *scsi = NULL; - VirtIODevice *vdev = VIRTIO_DEVICE(req->dev); - VirtQueueElement *elem = &req->elem; VirtIOBlock *blk = req->dev; + VirtIODevice *vdev = VIRTIO_DEVICE(blk); + VirtQueueElement *elem = &req->elem; #ifdef __linux__ int i; @@ -492,16 +492,18 @@ static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) static void virtio_blk_handle_flush(VirtIOBlockReq *req, MultiReqBuffer *mrb) { - block_acct_start(blk_get_stats(req->dev->blk), &req->acct, 0, + VirtIOBlock *s = req->dev; + + block_acct_start(blk_get_stats(s->blk), &req->acct, 0, BLOCK_ACCT_FLUSH); /* * Make sure all outstanding writes are posted to the backing device. */ if (mrb->is_write && mrb->num_reqs > 0) { - virtio_blk_submit_multireq(req->dev->blk, mrb); + virtio_blk_submit_multireq(s->blk, mrb); } - blk_aio_flush(req->dev->blk, virtio_blk_flush_complete, req); + blk_aio_flush(s->blk, virtio_blk_flush_complete, req); } static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, From 21c4d15b4708b7d30c450041a560df670f36cac8 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Wed, 6 Mar 2019 19:55:28 +0800 Subject: [PATCH 3/7] iothread: replace init_done_cond with a semaphore Only sending an init-done message using lock+cond seems an overkill to me. Replacing it with a simpler semaphore. Meanwhile, init the semaphore unconditionally, then we can destroy it unconditionally too in finalize which seems cleaner. Signed-off-by: Peter Xu Message-id: 20190306115532.23025-2-peterx@redhat.com Message-Id: <20190306115532.23025-2-peterx@redhat.com> Signed-off-by: Stefan Hajnoczi --- include/sysemu/iothread.h | 3 +-- iothread.c | 17 ++++------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h index 8a7ac2c528..50411ba54a 100644 --- a/include/sysemu/iothread.h +++ b/include/sysemu/iothread.h @@ -27,8 +27,7 @@ typedef struct { GMainContext *worker_context; GMainLoop *main_loop; GOnce once; - QemuMutex init_done_lock; - QemuCond init_done_cond; /* is thread initialization done? */ + QemuSemaphore init_done_sem; /* is thread init done? */ bool stopping; /* has iothread_stop() been called? */ bool running; /* should iothread_run() continue? */ int thread_id; diff --git a/iothread.c b/iothread.c index e615b7ae52..6e297e9ef1 100644 --- a/iothread.c +++ b/iothread.c @@ -55,10 +55,8 @@ static void *iothread_run(void *opaque) rcu_register_thread(); my_iothread = iothread; - qemu_mutex_lock(&iothread->init_done_lock); iothread->thread_id = qemu_get_thread_id(); - qemu_cond_signal(&iothread->init_done_cond); - qemu_mutex_unlock(&iothread->init_done_lock); + qemu_sem_post(&iothread->init_done_sem); while (iothread->running) { aio_poll(iothread->ctx, true); @@ -115,6 +113,7 @@ static void iothread_instance_init(Object *obj) iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT; iothread->thread_id = -1; + qemu_sem_init(&iothread->init_done_sem, 0); } static void iothread_instance_finalize(Object *obj) @@ -123,10 +122,6 @@ static void iothread_instance_finalize(Object *obj) iothread_stop(iothread); - if (iothread->thread_id != -1) { - qemu_cond_destroy(&iothread->init_done_cond); - qemu_mutex_destroy(&iothread->init_done_lock); - } /* * Before glib2 2.33.10, there is a glib2 bug that GSource context * pointer may not be cleared even if the context has already been @@ -145,6 +140,7 @@ static void iothread_instance_finalize(Object *obj) g_main_context_unref(iothread->worker_context); iothread->worker_context = NULL; } + qemu_sem_destroy(&iothread->init_done_sem); } static void iothread_complete(UserCreatable *obj, Error **errp) @@ -173,8 +169,6 @@ static void iothread_complete(UserCreatable *obj, Error **errp) return; } - qemu_mutex_init(&iothread->init_done_lock); - qemu_cond_init(&iothread->init_done_cond); iothread->once = (GOnce) G_ONCE_INIT; /* This assumes we are called from a thread with useful CPU affinity for us @@ -188,12 +182,9 @@ static void iothread_complete(UserCreatable *obj, Error **errp) g_free(name); /* Wait for initialization to complete */ - qemu_mutex_lock(&iothread->init_done_lock); while (iothread->thread_id == -1) { - qemu_cond_wait(&iothread->init_done_cond, - &iothread->init_done_lock); + qemu_sem_wait(&iothread->init_done_sem); } - qemu_mutex_unlock(&iothread->init_done_lock); } typedef struct { From b506e0f16c05b68b776752909e4451b54d461fd1 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Wed, 6 Mar 2019 19:55:29 +0800 Subject: [PATCH 4/7] iothread: create the gcontext unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In existing code we create the gcontext dynamically at the first access of the gcontext from caller. That can bring some complexity and potential races during using iothread. Since the context itself is not that big a resource, and we won't have millions of iothread, let's simply create the gcontext unconditionally. This will also be a preparation work further to move the thread context push operation earlier than before (now it's only pushed right before we want to start running the gmainloop). Removing the g_once since it's not necessary, while introducing a new run_gcontext boolean to show whether we want to run the gcontext. Reviewed-by: Marc-André Lureau Signed-off-by: Peter Xu Message-id: 20190306115532.23025-3-peterx@redhat.com Message-Id: <20190306115532.23025-3-peterx@redhat.com> Signed-off-by: Stefan Hajnoczi --- include/sysemu/iothread.h | 2 +- iothread.c | 43 +++++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h index 50411ba54a..5f6240d5cb 100644 --- a/include/sysemu/iothread.h +++ b/include/sysemu/iothread.h @@ -24,9 +24,9 @@ typedef struct { QemuThread thread; AioContext *ctx; + bool run_gcontext; /* whether we should run gcontext */ GMainContext *worker_context; GMainLoop *main_loop; - GOnce once; QemuSemaphore init_done_sem; /* is thread init done? */ bool stopping; /* has iothread_stop() been called? */ bool running; /* should iothread_run() continue? */ diff --git a/iothread.c b/iothread.c index 6e297e9ef1..6fa87876e0 100644 --- a/iothread.c +++ b/iothread.c @@ -65,7 +65,7 @@ static void *iothread_run(void *opaque) * We must check the running state again in case it was * changed in previous aio_poll() */ - if (iothread->running && atomic_read(&iothread->worker_context)) { + if (iothread->running && atomic_read(&iothread->run_gcontext)) { GMainLoop *loop; g_main_context_push_thread_default(iothread->worker_context); @@ -114,6 +114,8 @@ static void iothread_instance_init(Object *obj) iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT; iothread->thread_id = -1; qemu_sem_init(&iothread->init_done_sem, 0); + /* By default, we don't run gcontext */ + atomic_set(&iothread->run_gcontext, 0); } static void iothread_instance_finalize(Object *obj) @@ -143,6 +145,16 @@ static void iothread_instance_finalize(Object *obj) qemu_sem_destroy(&iothread->init_done_sem); } +static void iothread_init_gcontext(IOThread *iothread) +{ + GSource *source; + + iothread->worker_context = g_main_context_new(); + source = aio_get_g_source(iothread_get_aio_context(iothread)); + g_source_attach(source, iothread->worker_context); + g_source_unref(source); +} + static void iothread_complete(UserCreatable *obj, Error **errp) { Error *local_error = NULL; @@ -157,6 +169,12 @@ static void iothread_complete(UserCreatable *obj, Error **errp) return; } + /* + * Init one GMainContext for the iothread unconditionally, even if + * it's not used + */ + iothread_init_gcontext(iothread); + aio_context_set_poll_params(iothread->ctx, iothread->poll_max_ns, iothread->poll_grow, @@ -169,8 +187,6 @@ static void iothread_complete(UserCreatable *obj, Error **errp) return; } - iothread->once = (GOnce) G_ONCE_INIT; - /* This assumes we are called from a thread with useful CPU affinity for us * to inherit. */ @@ -333,27 +349,10 @@ IOThreadInfoList *qmp_query_iothreads(Error **errp) return head; } -static gpointer iothread_g_main_context_init(gpointer opaque) -{ - AioContext *ctx; - IOThread *iothread = opaque; - GSource *source; - - iothread->worker_context = g_main_context_new(); - - ctx = iothread_get_aio_context(iothread); - source = aio_get_g_source(ctx); - g_source_attach(source, iothread->worker_context); - g_source_unref(source); - - aio_notify(iothread->ctx); - return NULL; -} - GMainContext *iothread_get_g_main_context(IOThread *iothread) { - g_once(&iothread->once, iothread_g_main_context_init, iothread); - + atomic_set(&iothread->run_gcontext, 1); + aio_notify(iothread->ctx); return iothread->worker_context; } From 0bd2d233c628dcfcd2b24b7b7f59e8fcd2dcbae3 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Wed, 6 Mar 2019 19:55:30 +0800 Subject: [PATCH 5/7] iothread: create main loop unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we've have the gcontext always there, create the main loop altogether. The iothread_run() is even cleaner. Reviewed-by: Marc-André Lureau Signed-off-by: Peter Xu Message-id: 20190306115532.23025-4-peterx@redhat.com Message-Id: <20190306115532.23025-4-peterx@redhat.com> Signed-off-by: Stefan Hajnoczi --- iothread.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/iothread.c b/iothread.c index 6fa87876e0..9abdbace66 100644 --- a/iothread.c +++ b/iothread.c @@ -66,17 +66,8 @@ static void *iothread_run(void *opaque) * changed in previous aio_poll() */ if (iothread->running && atomic_read(&iothread->run_gcontext)) { - GMainLoop *loop; - g_main_context_push_thread_default(iothread->worker_context); - iothread->main_loop = - g_main_loop_new(iothread->worker_context, TRUE); - loop = iothread->main_loop; - g_main_loop_run(iothread->main_loop); - iothread->main_loop = NULL; - g_main_loop_unref(loop); - g_main_context_pop_thread_default(iothread->worker_context); } } @@ -141,6 +132,8 @@ static void iothread_instance_finalize(Object *obj) if (iothread->worker_context) { g_main_context_unref(iothread->worker_context); iothread->worker_context = NULL; + g_main_loop_unref(iothread->main_loop); + iothread->main_loop = NULL; } qemu_sem_destroy(&iothread->init_done_sem); } @@ -153,6 +146,7 @@ static void iothread_init_gcontext(IOThread *iothread) source = aio_get_g_source(iothread_get_aio_context(iothread)); g_source_attach(source, iothread->worker_context); g_source_unref(source); + iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE); } static void iothread_complete(UserCreatable *obj, Error **errp) From b60ec76a13b0d2cd06b4a681f025c180f4dd79a1 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Wed, 6 Mar 2019 19:55:31 +0800 Subject: [PATCH 6/7] iothread: push gcontext earlier in the thread_fn We were pushing the context until right before running the gmainloop. Now since we have everything unconditionally, we can move this earlier. One benefit is that now it's done even before init_done_sem, so as long as the iothread user calls iothread_create() and completes, we know that the thread stack is ready. Signed-off-by: Peter Xu Message-id: 20190306115532.23025-5-peterx@redhat.com Message-Id: <20190306115532.23025-5-peterx@redhat.com> [Tweaked comment wording as discussed with Peter Xu. --Stefan] Signed-off-by: Stefan Hajnoczi --- iothread.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/iothread.c b/iothread.c index 9abdbace66..ad64c757ac 100644 --- a/iothread.c +++ b/iothread.c @@ -53,7 +53,11 @@ static void *iothread_run(void *opaque) IOThread *iothread = opaque; rcu_register_thread(); - + /* + * g_main_context_push_thread_default() must be called before anything + * in this new thread uses glib. + */ + g_main_context_push_thread_default(iothread->worker_context); my_iothread = iothread; iothread->thread_id = qemu_get_thread_id(); qemu_sem_post(&iothread->init_done_sem); @@ -66,12 +70,11 @@ static void *iothread_run(void *opaque) * changed in previous aio_poll() */ if (iothread->running && atomic_read(&iothread->run_gcontext)) { - g_main_context_push_thread_default(iothread->worker_context); g_main_loop_run(iothread->main_loop); - g_main_context_pop_thread_default(iothread->worker_context); } } + g_main_context_pop_thread_default(iothread->worker_context); rcu_unregister_thread(); return NULL; } From 6ca206204fa773c8626d59caf2a5676d6cc35f52 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Wed, 6 Mar 2019 19:55:32 +0800 Subject: [PATCH 7/7] iothread: document about why we need explicit aio_poll() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After consulting Paolo I know why we'd better keep the explicit aio_poll() in iothread_run(). Document it directly into the code so that future readers will know the answer from day one. Signed-off-by: Peter Xu Reviewed-by: Marc-André Lureau Message-id: 20190306115532.23025-6-peterx@redhat.com Message-Id: <20190306115532.23025-6-peterx@redhat.com> Signed-off-by: Stefan Hajnoczi --- iothread.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/iothread.c b/iothread.c index ad64c757ac..7130be58e3 100644 --- a/iothread.c +++ b/iothread.c @@ -63,6 +63,15 @@ static void *iothread_run(void *opaque) qemu_sem_post(&iothread->init_done_sem); while (iothread->running) { + /* + * Note: from functional-wise the g_main_loop_run() below can + * already cover the aio_poll() events, but we can't run the + * main loop unconditionally because explicit aio_poll() here + * is faster than g_main_loop_run() when we do not need the + * gcontext at all (e.g., pure block layer iothreads). In + * other words, when we want to run the gcontext with the + * iothread we need to pay some performance for functionality. + */ aio_poll(iothread->ctx, true); /*