From nobody Wed Oct 29 17:10:21 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 152665039491326.9038511296219; Fri, 18 May 2018 06:33:14 -0700 (PDT) Received: from localhost ([::1]:38943 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fJfVR-00018W-VI for importer@patchew.org; Fri, 18 May 2018 09:33:14 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59494) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fJfKQ-0008Hv-GL for qemu-devel@nongnu.org; Fri, 18 May 2018 09:21:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fJfKN-0005wm-E0 for qemu-devel@nongnu.org; Fri, 18 May 2018 09:21:50 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47720 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fJfKH-0005rO-82; Fri, 18 May 2018 09:21:41 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D982EC12A1; Fri, 18 May 2018 13:21:40 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-149.ams2.redhat.com [10.36.117.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8CD982024CBF; Fri, 18 May 2018 13:21:39 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 18 May 2018 15:20:46 +0200 Message-Id: <20180518132114.4070-13-kwolf@redhat.com> In-Reply-To: <20180518132114.4070-1-kwolf@redhat.com> References: <20180518132114.4070-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 18 May 2018 13:21:40 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 18 May 2018 13:21:40 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 12/40] job: Move defer_to_main_loop to Job X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-devel@nongnu.org, jsnow@redhat.com, jcody@redhat.com, armbru@redhat.com, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Move the defer_to_main_loop functionality from BlockJob to Job. The code can be simplified because we can use job->aio_context in job_defer_to_main_loop_bh() now, instead of having to access the BlockDriverState. Probably taking the data->aio_context lock in addition was already unnecessary in the old code because we didn't actually make use of anything protected by the old AioContext except getting the new AioContext, in case it changed between scheduling the BH and running it. But it's certainly unnecessary now that the BDS isn't accessed at all any more. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: John Snow --- include/block/blockjob.h | 5 ---- include/block/blockjob_int.h | 19 --------------- include/qemu/job.h | 20 ++++++++++++++++ block/backup.c | 7 +++--- block/commit.c | 11 +++++---- block/mirror.c | 15 ++++++------ block/stream.c | 14 +++++------ blockjob.c | 57 ++++------------------------------------= ---- job.c | 32 +++++++++++++++++++++++++ tests/test-bdrv-drain.c | 7 +++--- tests/test-blockjob-txn.c | 13 +++++----- tests/test-blockjob.c | 7 +++--- 12 files changed, 97 insertions(+), 110 deletions(-) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 1e708f468a..2a9e865e31 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -92,11 +92,6 @@ typedef struct BlockJob { */ bool ready; =20 - /** - * Set to true when the job has deferred work to the main loop. - */ - bool deferred_to_main_loop; - /** Status that is published by the query-block-jobs QMP API */ BlockDeviceIoStatus iostatus; =20 diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index d64f30e6b0..0c2f8de381 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -233,23 +233,4 @@ void block_job_event_ready(BlockJob *job); BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_= err, int is_read, int error); =20 -typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); - -/** - * block_job_defer_to_main_loop: - * @job: The job - * @fn: The function to run in the main loop - * @opaque: The opaque value that is passed to @fn - * - * This function must be called by the main job coroutine just before it - * returns. @fn is executed in the main loop with the BlockDriverState - * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), = and - * anything that uses bdrv_drain_all() in the main loop. - * - * The @job AioContext is held while @fn executes. - */ -void block_job_defer_to_main_loop(BlockJob *job, - BlockJobDeferToMainLoopFn *fn, - void *opaque); - #endif diff --git a/include/qemu/job.h b/include/qemu/job.h index 01e083f97a..933e0ab328 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -58,6 +58,9 @@ typedef struct Job { */ bool cancelled; =20 + /** Set to true when the job has deferred work to the main loop. */ + bool deferred_to_main_loop; + /** Element of the list of jobs */ QLIST_ENTRY(Job) job_list; } Job; @@ -131,6 +134,23 @@ Job *job_get(const char *id); */ int job_apply_verb(Job *job, JobVerb verb, Error **errp); =20 +typedef void JobDeferToMainLoopFn(Job *job, void *opaque); + +/** + * @job: The job + * @fn: The function to run in the main loop + * @opaque: The opaque value that is passed to @fn + * + * This function must be called by the main job coroutine just before it + * returns. @fn is executed in the main loop with the job AioContext acqu= ired. + * + * Block jobs must call bdrv_unref(), bdrv_close(), and anything that uses + * bdrv_drain_all() in the main loop. + * + * The @job AioContext is held while @fn executes. + */ +void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue); + /* TODO To be removed from the public interface */ void job_state_transition(Job *job, JobStatus s1); =20 diff --git a/block/backup.c b/block/backup.c index ef0aa0e24e..22dd368c90 100644 --- a/block/backup.c +++ b/block/backup.c @@ -317,11 +317,12 @@ typedef struct { int ret; } BackupCompleteData; =20 -static void backup_complete(BlockJob *job, void *opaque) +static void backup_complete(Job *job, void *opaque) { + BlockJob *bjob =3D container_of(job, BlockJob, job); BackupCompleteData *data =3D opaque; =20 - block_job_completed(job, data->ret); + block_job_completed(bjob, data->ret); g_free(data); } =20 @@ -519,7 +520,7 @@ static void coroutine_fn backup_run(void *opaque) =20 data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&job->common, backup_complete, data); + job_defer_to_main_loop(&job->common.job, backup_complete, data); } =20 static const BlockJobDriver backup_job_driver =3D { diff --git a/block/commit.c b/block/commit.c index 85baea8f92..d326766e4d 100644 --- a/block/commit.c +++ b/block/commit.c @@ -72,9 +72,10 @@ typedef struct { int ret; } CommitCompleteData; =20 -static void commit_complete(BlockJob *job, void *opaque) +static void commit_complete(Job *job, void *opaque) { - CommitBlockJob *s =3D container_of(job, CommitBlockJob, common); + CommitBlockJob *s =3D container_of(job, CommitBlockJob, common.job); + BlockJob *bjob =3D &s->common; CommitCompleteData *data =3D opaque; BlockDriverState *top =3D blk_bs(s->top); BlockDriverState *base =3D blk_bs(s->base); @@ -90,7 +91,7 @@ static void commit_complete(BlockJob *job, void *opaque) * the normal backing chain can be restored. */ blk_unref(s->base); =20 - if (!job_is_cancelled(&s->common.job) && ret =3D=3D 0) { + if (!job_is_cancelled(job) && ret =3D=3D 0) { /* success */ ret =3D bdrv_drop_intermediate(s->commit_top_bs, base, s->backing_file_str); @@ -114,7 +115,7 @@ static void commit_complete(BlockJob *job, void *opaque) * block_job_finish_sync()), block_job_completed() won't free it and * therefore the blockers on the intermediate nodes remain. This would * cause bdrv_set_backing_hd() to fail. */ - block_job_remove_all_bdrv(job); + block_job_remove_all_bdrv(bjob); =20 block_job_completed(&s->common, ret); g_free(data); @@ -211,7 +212,7 @@ out: =20 data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&s->common, commit_complete, data); + job_defer_to_main_loop(&s->common.job, commit_complete, data); } =20 static const BlockJobDriver commit_job_driver =3D { diff --git a/block/mirror.c b/block/mirror.c index 424072ed00..90d4ac9cb6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -484,9 +484,10 @@ typedef struct { int ret; } MirrorExitData; =20 -static void mirror_exit(BlockJob *job, void *opaque) +static void mirror_exit(Job *job, void *opaque) { - MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); + MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common.job); + BlockJob *bjob =3D &s->common; MirrorExitData *data =3D opaque; AioContext *replace_aio_context =3D NULL; BlockDriverState *src =3D s->source; @@ -568,7 +569,7 @@ static void mirror_exit(BlockJob *job, void *opaque) * the blockers on the intermediate nodes so that the resulting state = is * valid. Also give up permissions on mirror_top_bs->backing, which mi= ght * block the removal. */ - block_job_remove_all_bdrv(job); + block_job_remove_all_bdrv(bjob); bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abo= rt); @@ -576,9 +577,9 @@ static void mirror_exit(BlockJob *job, void *opaque) /* We just changed the BDS the job BB refers to (with either or both o= f the * bdrv_replace_node() calls), so switch the BB back so the cleanup do= es * the right thing. We don't need any permissions any more now. */ - blk_remove_bs(job->blk); - blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); - blk_insert_bs(job->blk, mirror_top_bs, &error_abort); + blk_remove_bs(bjob->blk); + blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); + blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort); =20 block_job_completed(&s->common, data->ret); =20 @@ -901,7 +902,7 @@ immediate_exit: if (need_drain) { bdrv_drained_begin(bs); } - block_job_defer_to_main_loop(&s->common, mirror_exit, data); + job_defer_to_main_loop(&s->common.job, mirror_exit, data); } =20 static void mirror_complete(BlockJob *job, Error **errp) diff --git a/block/stream.c b/block/stream.c index 22c71ae100..0bba81678c 100644 --- a/block/stream.c +++ b/block/stream.c @@ -58,16 +58,16 @@ typedef struct { int ret; } StreamCompleteData; =20 -static void stream_complete(BlockJob *job, void *opaque) +static void stream_complete(Job *job, void *opaque) { - StreamBlockJob *s =3D container_of(job, StreamBlockJob, common); + StreamBlockJob *s =3D container_of(job, StreamBlockJob, common.job); + BlockJob *bjob =3D &s->common; StreamCompleteData *data =3D opaque; - BlockDriverState *bs =3D blk_bs(job->blk); + BlockDriverState *bs =3D blk_bs(bjob->blk); BlockDriverState *base =3D s->base; Error *local_err =3D NULL; =20 - if (!job_is_cancelled(&s->common.job) && bs->backing && - data->ret =3D=3D 0) { + if (!job_is_cancelled(job) && bs->backing && data->ret =3D=3D 0) { const char *base_id =3D NULL, *base_fmt =3D NULL; if (base) { base_id =3D s->backing_file_str; @@ -88,7 +88,7 @@ out: /* Reopen the image back in read-only mode if necessary */ if (s->bs_flags !=3D bdrv_get_flags(bs)) { /* Give up write permissions before making it read-only */ - blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); + blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); bdrv_reopen(bs, s->bs_flags, NULL); } =20 @@ -205,7 +205,7 @@ out: /* Modify backing chain and close BDSes in main loop */ data =3D g_malloc(sizeof(*data)); data->ret =3D ret; - block_job_defer_to_main_loop(&s->common, stream_complete, data); + job_defer_to_main_loop(&s->common.job, stream_complete, data); } =20 static const BlockJobDriver stream_job_driver =3D { diff --git a/blockjob.c b/blockjob.c index 0a0b1c41dd..3ede511da0 100644 --- a/blockjob.c +++ b/blockjob.c @@ -360,7 +360,7 @@ static void block_job_decommission(BlockJob *job) job->completed =3D true; job->busy =3D false; job->paused =3D false; - job->deferred_to_main_loop =3D true; + job->job.deferred_to_main_loop =3D true; block_job_txn_del_job(job); job_state_transition(&job->job, JOB_STATUS_NULL); job_unref(&job->job); @@ -515,7 +515,7 @@ static int block_job_finish_sync(BlockJob *job, /* block_job_drain calls block_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->completed) { + while (!job->job.deferred_to_main_loop && !job->completed) { block_job_drain(job); } while (!job->completed) { @@ -729,7 +729,7 @@ void block_job_cancel(BlockJob *job, bool force) block_job_cancel_async(job, force); if (!block_job_started(job)) { block_job_completed(job, -ECANCELED); - } else if (job->deferred_to_main_loop) { + } else if (job->job.deferred_to_main_loop) { block_job_completed_txn_abort(job); } else { block_job_enter(job); @@ -1045,7 +1045,7 @@ static void block_job_enter_cond(BlockJob *job, bool(= *fn)(BlockJob *job)) if (!block_job_started(job)) { return; } - if (job->deferred_to_main_loop) { + if (job->job.deferred_to_main_loop) { return; } =20 @@ -1060,7 +1060,7 @@ static void block_job_enter_cond(BlockJob *job, bool(= *fn)(BlockJob *job)) return; } =20 - assert(!job->deferred_to_main_loop); + assert(!job->job.deferred_to_main_loop); timer_del(&job->sleep_timer); job->busy =3D true; block_job_unlock(); @@ -1166,50 +1166,3 @@ BlockErrorAction block_job_error_action(BlockJob *jo= b, BlockdevOnError on_err, } return action; } - -typedef struct { - BlockJob *job; - AioContext *aio_context; - BlockJobDeferToMainLoopFn *fn; - void *opaque; -} BlockJobDeferToMainLoopData; - -static void block_job_defer_to_main_loop_bh(void *opaque) -{ - BlockJobDeferToMainLoopData *data =3D opaque; - AioContext *aio_context; - - /* 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 =3D blk_get_aio_context(data->job->blk); - if (aio_context !=3D data->aio_context) { - aio_context_acquire(aio_context); - } - - data->fn(data->job, data->opaque); - - if (aio_context !=3D data->aio_context) { - aio_context_release(aio_context); - } - - aio_context_release(data->aio_context); - - g_free(data); -} - -void block_job_defer_to_main_loop(BlockJob *job, - BlockJobDeferToMainLoopFn *fn, - void *opaque) -{ - BlockJobDeferToMainLoopData *data =3D g_malloc(sizeof(*data)); - data->job =3D job; - data->aio_context =3D blk_get_aio_context(job->blk); - data->fn =3D fn; - data->opaque =3D opaque; - job->deferred_to_main_loop =3D true; - - aio_bh_schedule_oneshot(qemu_get_aio_context(), - block_job_defer_to_main_loop_bh, data); -} diff --git a/job.c b/job.c index 7ff1f66273..2e0cc2e7a8 100644 --- a/job.c +++ b/job.c @@ -28,6 +28,7 @@ #include "qapi/error.h" #include "qemu/job.h" #include "qemu/id.h" +#include "qemu/main-loop.h" #include "trace-root.h" =20 static QLIST_HEAD(, Job) jobs =3D QLIST_HEAD_INITIALIZER(jobs); @@ -170,3 +171,34 @@ void job_unref(Job *job) g_free(job); } } + +typedef struct { + Job *job; + JobDeferToMainLoopFn *fn; + void *opaque; +} JobDeferToMainLoopData; + +static void job_defer_to_main_loop_bh(void *opaque) +{ + JobDeferToMainLoopData *data =3D opaque; + Job *job =3D data->job; + AioContext *aio_context =3D job->aio_context; + + aio_context_acquire(aio_context); + data->fn(data->job, data->opaque); + aio_context_release(aio_context); + + g_free(data); +} + +void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue) +{ + JobDeferToMainLoopData *data =3D g_malloc(sizeof(*data)); + data->job =3D job; + data->fn =3D fn; + data->opaque =3D opaque; + job->deferred_to_main_loop =3D true; + + aio_bh_schedule_oneshot(qemu_get_aio_context(), + job_defer_to_main_loop_bh, data); +} diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index f9e37d479c..4f8cba8377 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -496,9 +496,10 @@ typedef struct TestBlockJob { bool should_complete; } TestBlockJob; =20 -static void test_job_completed(BlockJob *job, void *opaque) +static void test_job_completed(Job *job, void *opaque) { - block_job_completed(job, 0); + BlockJob *bjob =3D container_of(job, BlockJob, job); + block_job_completed(bjob, 0); } =20 static void coroutine_fn test_job_start(void *opaque) @@ -510,7 +511,7 @@ static void coroutine_fn test_job_start(void *opaque) block_job_sleep_ns(&s->common, 100000); } =20 - block_job_defer_to_main_loop(&s->common, test_job_completed, NULL); + job_defer_to_main_loop(&s->common.job, test_job_completed, NULL); } =20 static void test_job_complete(BlockJob *job, Error **errp) diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 26b4bbb230..c03f9662d8 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -24,16 +24,17 @@ typedef struct { int *result; } TestBlockJob; =20 -static void test_block_job_complete(BlockJob *job, void *opaque) +static void test_block_job_complete(Job *job, void *opaque) { - BlockDriverState *bs =3D blk_bs(job->blk); + BlockJob *bjob =3D container_of(job, BlockJob, job); + BlockDriverState *bs =3D blk_bs(bjob->blk); int rc =3D (intptr_t)opaque; =20 - if (job_is_cancelled(&job->job)) { + if (job_is_cancelled(job)) { rc =3D -ECANCELED; } =20 - block_job_completed(job, rc); + block_job_completed(bjob, rc); bdrv_unref(bs); } =20 @@ -54,8 +55,8 @@ static void coroutine_fn test_block_job_run(void *opaque) } } =20 - block_job_defer_to_main_loop(job, test_block_job_complete, - (void *)(intptr_t)s->rc); + job_defer_to_main_loop(&job->job, test_block_job_complete, + (void *)(intptr_t)s->rc); } =20 typedef struct { diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index fa31481537..5f43bd72a4 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -161,11 +161,12 @@ typedef struct CancelJob { bool completed; } CancelJob; =20 -static void cancel_job_completed(BlockJob *job, void *opaque) +static void cancel_job_completed(Job *job, void *opaque) { + BlockJob *bjob =3D container_of(job, BlockJob, job); CancelJob *s =3D opaque; s->completed =3D true; - block_job_completed(job, 0); + block_job_completed(bjob, 0); } =20 static void cancel_job_complete(BlockJob *job, Error **errp) @@ -191,7 +192,7 @@ static void coroutine_fn cancel_job_start(void *opaque) } =20 defer: - block_job_defer_to_main_loop(&s->common, cancel_job_completed, s); + job_defer_to_main_loop(&s->common.job, cancel_job_completed, s); } =20 static const BlockJobDriver test_cancel_driver =3D { --=20 2.13.6