From nobody Wed Oct 29 09:08:30 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 1524585097174311.5688813414521; Tue, 24 Apr 2018 08:51:37 -0700 (PDT) Received: from localhost ([::1]:59239 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fB0EC-00031D-BH for importer@patchew.org; Tue, 24 Apr 2018 11:51:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fAzpD-00081b-JU for qemu-devel@nongnu.org; Tue, 24 Apr 2018 11:25:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fAzpB-00088D-5E for qemu-devel@nongnu.org; Tue, 24 Apr 2018 11:25:47 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:34842 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 1fAzp6-000817-4h; Tue, 24 Apr 2018 11:25:40 -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 A81CE738E0; Tue, 24 Apr 2018 15:25:39 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-100.ams2.redhat.com [10.36.117.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 97C2B2026990; Tue, 24 Apr 2018 15:25:38 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 24 Apr 2018 17:24:55 +0200 Message-Id: <20180424152515.25664-14-kwolf@redhat.com> In-Reply-To: <20180424152515.25664-1-kwolf@redhat.com> References: <20180424152515.25664-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]); Tue, 24 Apr 2018 15:25:39 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 24 Apr 2018 15:25:39 +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] [RFC PATCH 13/33] job: Add reference counting 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, jcody@redhat.com, jsnow@redhat.com, qemu-devel@nongnu.org, 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" This moves reference counting from BlockJob to Job. In order to keep calling the BlockJob cleanup code when the job is deleted via job_unref(), introduce a new JobDriver.free callback. Every block job must use block_job_free() for this callback, this is asserted in block_job_create(). Signed-off-by: Kevin Wolf --- include/block/blockjob.h | 21 ------------------- include/block/blockjob_int.h | 7 +++++++ include/qemu/job.h | 19 ++++++++++++++++-- block/backup.c | 1 + block/commit.c | 1 + block/mirror.c | 2 ++ block/stream.c | 1 + blockjob.c | 48 +++++++++++++++++++---------------------= ---- job.c | 22 ++++++++++++++++---- qemu-img.c | 4 ++-- tests/test-bdrv-drain.c | 1 + tests/test-blockjob-txn.c | 1 + tests/test-blockjob.c | 6 ++++-- 13 files changed, 76 insertions(+), 58 deletions(-) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 96e5f67ed7..5e32e719b6 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -132,9 +132,6 @@ typedef struct BlockJob { /** The opaque value that is passed to the completion function. */ void *opaque; =20 - /** Reference count of the block job */ - int refcnt; - /** True when job has reported completion by calling block_job_complet= ed. */ bool completed; =20 @@ -400,24 +397,6 @@ void block_job_iostatus_reset(BlockJob *job); BlockJobTxn *block_job_txn_new(void); =20 /** - * block_job_ref: - * - * Add a reference to BlockJob refcnt, it will be decreased with - * block_job_unref, and then be freed if it comes to be the last - * reference. - */ -void block_job_ref(BlockJob *job); - -/** - * block_job_unref: - * - * Release a reference that was previously acquired with block_job_ref - * or block_job_create. If it's the last reference to the object, it will = be - * freed. - */ -void block_job_unref(BlockJob *job); - -/** * block_job_txn_unref: * * Release a reference that was previously acquired with block_job_txn_add= _job diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 699a6d1487..4e95fa9f24 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -144,6 +144,13 @@ void *block_job_create(const char *job_id, const Block= JobDriver *driver, BlockCompletionFunc *cb, void *opaque, Error **errp= ); =20 /** + * block_job_free: + * Callback to be used for JobDriver.free in all block jobs. Frees block j= ob + * specific resources in @job. + */ +void block_job_free(Job *job); + +/** * block_job_sleep_ns: * @job: The job that calls the function. * @ns: How many nanoseconds to stop for. diff --git a/include/qemu/job.h b/include/qemu/job.h index c22054a7be..f502c30620 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -41,6 +41,9 @@ typedef struct Job { /** The type of this job. */ const JobDriver *driver; =20 + /** Reference count of the block job */ + int refcnt; + /** Current state; See @JobStatus for details. */ JobStatus status; =20 @@ -57,6 +60,9 @@ struct JobDriver { =20 /** Enum describing the operation */ JobType job_type; + + /** Called when the job is freed */ + void (*free)(Job *job); }; =20 =20 @@ -69,8 +75,17 @@ struct JobDriver { */ void *job_create(const char *job_id, const JobDriver *driver, Error **errp= ); =20 -/** Frees the @job object. */ -void job_delete(Job *job); +/** + * Add a reference to Job refcnt, it will be decreased with job_unref, and= then + * be freed if it comes to be the last reference. + */ +void job_ref(Job *job); + +/** + * Release a reference that was previously acquired with job_ref() or + * job_create(). If it's the last reference to the object, it will be free= d. + */ +void job_unref(Job *job); =20 /** Returns the JobType of a given Job. */ JobType job_type(Job *job); diff --git a/block/backup.c b/block/backup.c index 780c2bde90..569a118448 100644 --- a/block/backup.c +++ b/block/backup.c @@ -526,6 +526,7 @@ static const BlockJobDriver backup_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(BackupBlockJob), .job_type =3D JOB_TYPE_BACKUP, + .free =3D block_job_free, }, .start =3D backup_run, .commit =3D backup_commit, diff --git a/block/commit.c b/block/commit.c index 32d29c890e..925c96abe7 100644 --- a/block/commit.c +++ b/block/commit.c @@ -218,6 +218,7 @@ static const BlockJobDriver commit_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(CommitBlockJob), .job_type =3D JOB_TYPE_COMMIT, + .free =3D block_job_free, }, .start =3D commit_run, }; diff --git a/block/mirror.c b/block/mirror.c index 2c014a2b96..6ff8a370a5 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -985,6 +985,7 @@ static const BlockJobDriver mirror_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(MirrorBlockJob), .job_type =3D JOB_TYPE_MIRROR, + .free =3D block_job_free, }, .start =3D mirror_run, .complete =3D mirror_complete, @@ -997,6 +998,7 @@ static const BlockJobDriver commit_active_job_driver = =3D { .job_driver =3D { .instance_size =3D sizeof(MirrorBlockJob), .job_type =3D JOB_TYPE_COMMIT, + .free =3D block_job_free, }, .start =3D mirror_run, .complete =3D mirror_complete, diff --git a/block/stream.c b/block/stream.c index cb723f190a..7273d2213c 100644 --- a/block/stream.c +++ b/block/stream.c @@ -212,6 +212,7 @@ static const BlockJobDriver stream_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(StreamBlockJob), .job_type =3D JOB_TYPE_STREAM, + .free =3D block_job_free, }, .start =3D stream_run, }; diff --git a/blockjob.c b/blockjob.c index 2c995d4b34..34bc431696 100644 --- a/blockjob.c +++ b/blockjob.c @@ -177,31 +177,25 @@ static void block_job_resume(BlockJob *job) block_job_enter(job); } =20 -void block_job_ref(BlockJob *job) -{ - ++job->refcnt; -} - static void block_job_attached_aio_context(AioContext *new_context, void *opaque); static void block_job_detach_aio_context(void *opaque); =20 -void block_job_unref(BlockJob *job) +void block_job_free(Job *job) { - if (--job->refcnt =3D=3D 0) { - assert(job->job.status =3D=3D JOB_STATUS_NULL); - assert(!job->txn); - BlockDriverState *bs =3D blk_bs(job->blk); - bs->job =3D NULL; - block_job_remove_all_bdrv(job); - blk_remove_aio_context_notifier(job->blk, - block_job_attached_aio_context, - block_job_detach_aio_context, job); - blk_unref(job->blk); - error_free(job->blocker); - assert(!timer_pending(&job->sleep_timer)); - job_delete(&job->job); - } + BlockJob *bjob =3D container_of(job, BlockJob, job); + BlockDriverState *bs =3D blk_bs(bjob->blk); + + assert(!bjob->txn); + + bs->job =3D NULL; + block_job_remove_all_bdrv(bjob); + blk_remove_aio_context_notifier(bjob->blk, + block_job_attached_aio_context, + block_job_detach_aio_context, bjob); + blk_unref(bjob->blk); + error_free(bjob->blocker); + assert(!timer_pending(&bjob->sleep_timer)); } =20 static void block_job_attached_aio_context(AioContext *new_context, @@ -232,7 +226,7 @@ static void block_job_detach_aio_context(void *opaque) BlockJob *job =3D opaque; =20 /* In case the job terminates during aio_poll()... */ - block_job_ref(job); + job_ref(&job->job); =20 block_job_pause(job); =20 @@ -240,7 +234,7 @@ static void block_job_detach_aio_context(void *opaque) block_job_drain(job); } =20 - block_job_unref(job); + job_unref(&job->job); } =20 static char *child_job_get_parent_desc(BdrvChild *c) @@ -354,7 +348,7 @@ static void block_job_decommission(BlockJob *job) job->deferred_to_main_loop =3D true; block_job_txn_del_job(job); job_state_transition(&job->job, JOB_STATUS_NULL); - block_job_unref(job); + job_unref(&job->job); } =20 static void block_job_do_dismiss(BlockJob *job) @@ -493,14 +487,14 @@ static int block_job_finish_sync(BlockJob *job, =20 assert(blk_bs(job->blk)->job =3D=3D job); =20 - block_job_ref(job); + job_ref(&job->job); =20 if (finish) { finish(job, &local_err); } if (local_err) { error_propagate(errp, local_err); - block_job_unref(job); + job_unref(&job->job); return -EBUSY; } /* block_job_drain calls block_job_enter, and it should be enough to @@ -513,7 +507,7 @@ static int block_job_finish_sync(BlockJob *job, aio_poll(qemu_get_aio_context(), true); } ret =3D (job->cancelled && job->ret =3D=3D 0) ? -ECANCELED : job->ret; - block_job_unref(job); + job_unref(&job->job); return ret; } =20 @@ -900,6 +894,7 @@ void *block_job_create(const char *job_id, const BlockJ= obDriver *driver, } =20 assert(is_block_job(&job->job)); + assert(job->job.driver->free =3D=3D &block_job_free); =20 job->driver =3D driver; job->blk =3D blk; @@ -908,7 +903,6 @@ void *block_job_create(const char *job_id, const BlockJ= obDriver *driver, job->busy =3D false; job->paused =3D true; job->pause_count =3D 1; - job->refcnt =3D 1; job->auto_finalize =3D !(flags & BLOCK_JOB_MANUAL_FINALIZE); job->auto_dismiss =3D !(flags & BLOCK_JOB_MANUAL_DISMISS); aio_timer_init(qemu_get_aio_context(), &job->sleep_timer, diff --git a/job.c b/job.c index 791776d751..bcf5d5c288 100644 --- a/job.c +++ b/job.c @@ -135,6 +135,7 @@ void *job_create(const char *job_id, const JobDriver *d= river, Error **errp) job =3D g_malloc0(driver->instance_size); job->driver =3D driver; job->id =3D g_strdup(job_id); + job->refcnt =3D 1; =20 job_state_transition(job, JOB_STATUS_CREATED); =20 @@ -143,10 +144,23 @@ void *job_create(const char *job_id, const JobDriver = *driver, Error **errp) return job; } =20 -void job_delete(Job *job) +void job_ref(Job *job) { - QLIST_REMOVE(job, job_list); + ++job->refcnt; +} + +void job_unref(Job *job) +{ + if (--job->refcnt =3D=3D 0) { + assert(job->status =3D=3D JOB_STATUS_NULL); =20 - g_free(job->id); - g_free(job); + if (job->driver->free) { + job->driver->free(job); + } + + QLIST_REMOVE(job, job_list); + + g_free(job->id); + g_free(job); + } } diff --git a/qemu-img.c b/qemu-img.c index 855fa52514..62a52edc34 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -862,7 +862,7 @@ static void run_block_job(BlockJob *job, Error **errp) int ret =3D 0; =20 aio_context_acquire(aio_context); - block_job_ref(job); + job_ref(&job->job); do { aio_poll(aio_context, true); qemu_progress_print(job->len ? @@ -874,7 +874,7 @@ static void run_block_job(BlockJob *job, Error **errp) } else { ret =3D job->ret; } - block_job_unref(job); + job_unref(&job->job); aio_context_release(aio_context); =20 /* publish completion progress only when success */ diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index fe9f412b39..f9e37d479c 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -522,6 +522,7 @@ static void test_job_complete(BlockJob *job, Error **er= rp) BlockJobDriver test_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(TestBlockJob), + .free =3D block_job_free, }, .start =3D test_job_start, .complete =3D test_job_complete, diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 48b12d1744..b49b28ca27 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -76,6 +76,7 @@ static void test_block_job_cb(void *opaque, int ret) static const BlockJobDriver test_block_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(TestBlockJob), + .free =3D block_job_free, }, .start =3D test_block_job_run, }; diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 6ccd585dee..e24fc3f140 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -19,6 +19,7 @@ static const BlockJobDriver test_block_job_driver =3D { .job_driver =3D { .instance_size =3D sizeof(BlockJob), + .free =3D block_job_free, }, }; =20 @@ -196,6 +197,7 @@ static void coroutine_fn cancel_job_start(void *opaque) static const BlockJobDriver test_cancel_driver =3D { .job_driver =3D { .instance_size =3D sizeof(CancelJob), + .free =3D block_job_free, }, .start =3D cancel_job_start, .complete =3D cancel_job_complete, @@ -210,7 +212,7 @@ static CancelJob *create_common(BlockJob **pjob) blk =3D create_blk(NULL); job =3D mk_job(blk, "Steve", &test_cancel_driver, true, BLOCK_JOB_MANUAL_FINALIZE | BLOCK_JOB_MANUAL_DISMISS); - block_job_ref(job); + job_ref(&job->job); assert(job->job.status =3D=3D JOB_STATUS_CREATED); s =3D container_of(job, CancelJob, common); s->blk =3D blk; @@ -231,7 +233,7 @@ static void cancel_common(CancelJob *s) block_job_dismiss(&dummy, &error_abort); } assert(job->job.status =3D=3D JOB_STATUS_NULL); - block_job_unref(job); + job_unref(&job->job); destroy_blk(blk); } =20 --=20 2.13.6