[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loo
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loop() |
Date: |
Thu, 2 Oct 2014 15:58:53 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Oct 01, 2014 at 09:06:36PM +0200, Max Reitz wrote:
> On 01.10.2014 19:01, Stefan Hajnoczi wrote:
> >+typedef struct {
> >+ BlockJob *job;
> >+ QEMUBH *bh;
> >+ AioContext *aio_context;
> >+ BlockJobDeferToMainLoopFn *fn;
> >+ void *opaque;
> >+} BlockJobDeferToMainLoopData;
> >+
> >+static void block_job_defer_to_main_loop_bh(void *opaque)
> >+{
> >+ BlockJobDeferToMainLoopData *data = opaque;
> >+
> >+ qemu_bh_delete(data->bh);
> >+
> >+ aio_context_acquire(data->aio_context);
> >+ data->fn(data->job, data->opaque);
> >+ aio_context_release(data->aio_context);
> >+
> >+ g_free(data);
> >+}
> >+
> >+void block_job_defer_to_main_loop(BlockJob *job,
> >+ BlockJobDeferToMainLoopFn *fn,
> >+ void *opaque)
> >+{
> >+ BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
> >+ data->job = job;
> >+ data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
> >+ data->aio_context = bdrv_get_aio_context(job->bs);
> >+ data->fn = fn;
> >+ data->opaque = opaque;
> >+
> >+ qemu_bh_schedule(data->bh);
> >+}
>
> I'm not so sure whether it'd be possible to change the BDS's AIO context (in
> another thread) after the call to bdrv_get_aio_context() in
> block_job_defer_to_main_loop() and before block_job_defer_to_main_loop_bh()
> is run. If so, this might create race conditions.
>
> Other than that, this patch looks good.
Yes, I think you are correct.
The solution is a little tricky, we need to hold both AioContexts:
static void block_job_defer_to_main_loop_bh(void *opaque)
{
BlockJobDeferToMainLoopData *data = opaque;
AioContext aio_context;
qemu_bh_delete(data->bh);
/* Prevent race with block_job_defer_to_main_loop() */
aio_context_acquire(data->aio_context);
/* Fetch BDS AioContext again, in case it has changed */
aio_context = bdrv_get_aio_context(data->job->bs);
aio_context_acquire(aio_context);
data->fn(data->job, data->opaque);
aio_context_release(aio_context);
aio_context_release(data->aio_context);
g_free(data);
}
Tada! Because this executes in the main loop it is safe to do this - no lock
ordering problems. And because aio_context_acquire() is recursive we can lock
twice without problems.
Stefan
pgpusIHr3ByIw.pgp
Description: PGP signature
- [Qemu-devel] [PATCH 01/11] block: acquire AioContext in generic blockjob QMP commands, (continued)
- [Qemu-devel] [PATCH 01/11] block: acquire AioContext in generic blockjob QMP commands, Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 03/11] blockdev: acquire AioContext in blockdev_mark_auto_del(), Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 02/11] blockdev: acquire AioContext in do_qmp_query_block_jobs_one(), Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 04/11] blockdev: add note that block_job_cb() must be thread-safe, Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loop(), Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 06/11] block: add bdrv_drain(), Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 07/11] block: let backup blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 08/11] block: let stream blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/01
- [Qemu-devel] [PATCH 10/11] block: let commit blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/01