[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 04/14] job: Use AIO_WAIT_WHILE() in job_finish_s
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [PATCH 04/14] job: Use AIO_WAIT_WHILE() in job_finish_sync() |
Date: |
Tue, 11 Sep 2018 16:17:23 +0800 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Fri, 09/07 18:15, Kevin Wolf wrote:
> job_finish_sync() needs to release the AioContext lock of the job before
> calling aio_poll(). Otherwise, callbacks called by aio_poll() would
> possibly take the lock a second time and run into a deadlock with a
> nested AIO_WAIT_WHILE() call.
>
> Also, job_drain() without aio_poll() isn't necessarily enough to make
> progress on a job, it could depend on bottom halves to be executed.
>
> Combine both open-coded while loops into a single AIO_WAIT_WHILE() call
> that solves both of these problems.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> job.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/job.c b/job.c
> index 9ad0b7476a..8480eda188 100644
> --- a/job.c
> +++ b/job.c
> @@ -29,6 +29,7 @@
> #include "qemu/job.h"
> #include "qemu/id.h"
> #include "qemu/main-loop.h"
> +#include "block/aio-wait.h"
> #include "trace-root.h"
> #include "qapi/qapi-events-job.h"
>
> @@ -998,6 +999,7 @@ void job_defer_to_main_loop(Job *job,
> JobDeferToMainLoopFn *fn, void *opaque)
> int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error
> **errp)
> {
> Error *local_err = NULL;
> + AioWait dummy_wait = {};
> int ret;
>
> job_ref(job);
> @@ -1010,14 +1012,10 @@ int job_finish_sync(Job *job, void (*finish)(Job *,
> Error **errp), Error **errp)
> job_unref(job);
> return -EBUSY;
> }
> - /* job_drain calls job_enter, and it should be enough to induce progress
> - * until the job completes or moves to the main thread. */
> - while (!job->deferred_to_main_loop && !job_is_completed(job)) {
> - job_drain(job);
> - }
> - while (!job_is_completed(job)) {
> - aio_poll(qemu_get_aio_context(), true);
> - }
> +
> + AIO_WAIT_WHILE(&dummy_wait, job->aio_context,
> + (job_drain(job), !job_is_completed(job)));
The condition expression would read more elegant if job_drain() returns
progress.
Reviewed-by: Fam Zheng <address@hidden>
> +
> ret = (job_is_cancelled(job) && job->ret == 0) ? -ECANCELED : job->ret;
> job_unref(job);
> return ret;
> --
> 2.13.6
>
- [Qemu-block] [PATCH 00/14] Fix some jobs/drain/aio_poll related hangs, Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 03/14] test-blockjob: Acquire AioContext around job_finish_sync(), Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 01/14] blockjob: Wake up BDS when job becomes idle, Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 04/14] job: Use AIO_WAIT_WHILE() in job_finish_sync(), Kevin Wolf, 2018/09/07
- Re: [Qemu-block] [PATCH 04/14] job: Use AIO_WAIT_WHILE() in job_finish_sync(),
Fam Zheng <=
- [Qemu-block] [PATCH 02/14] test-bdrv-drain: Drain with block jobs in an I/O thread, Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 07/14] aio-wait: Increase num_waiters even in home thread, Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 05/14] test-bdrv-drain: Test AIO_WAIT_WHILE() in completion callback, Kevin Wolf, 2018/09/07
- [Qemu-block] [PATCH 06/14] block: Add missing locking in bdrv_co_drain_bh_cb(), Kevin Wolf, 2018/09/07