From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9hc-0001Yk-DY for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw9hb-0005fn-0z for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9ha-0005fb-QS for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:14 -0500 From: Stefan Hajnoczi Date: Tue, 10 Nov 2015 14:14:09 +0000 Message-Id: <1447164879-6756-15-git-send-email-stefanha@redhat.com> In-Reply-To: <1447164879-6756-1-git-send-email-stefanha@redhat.com> References: <1447164879-6756-1-git-send-email-stefanha@redhat.com> Subject: [Qemu-devel] [PULL 14/44] blockjob: Introduce reference count and fix reference to job->bs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: John Snow , Peter Maydell , Fam Zheng , Stefan Hajnoczi From: Fam Zheng Add reference count to block job, meanwhile move the ownership of the reference to job->bs from the caller (which is released in two completion callbacks) to the block job itself. It is necessary for block_job_complete_sync to work, because block job shouldn't live longer than its bs, as asserted in bdrv_delete. Now block_job_complete_sync can be simplified. Signed-off-by: Fam Zheng Signed-off-by: John Snow Message-id: 1446765200-3054-6-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- block/mirror.c | 2 +- blockdev.c | 28 ---------------------------- blockjob.c | 25 ++++++++++++++++--------- include/block/blockjob.h | 18 +++++++++++++++--- qemu-img.c | 3 --- 5 files changed, 32 insertions(+), 44 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b1252a1..47b32c5 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -741,7 +741,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp); if (!s->dirty_bitmap) { g_free(s->replaces); - block_job_release(bs); + block_job_unref(&s->common); return; } bdrv_set_enable_write_cache(s->target, true); diff --git a/blockdev.c b/blockdev.c index ec4a79c..299d3be 100644 --- a/blockdev.c +++ b/blockdev.c @@ -283,32 +283,6 @@ typedef struct { BlockDriverState *bs; } BDRVPutRefBH; -static void bdrv_put_ref_bh(void *opaque) -{ - BDRVPutRefBH *s = opaque; - - bdrv_unref(s->bs); - qemu_bh_delete(s->bh); - g_free(s); -} - -/* - * Release a BDS reference in a BH - * - * It is not safe to use bdrv_unref() from a callback function when the callers - * still need the BlockDriverState. In such cases we schedule a BH to release - * the reference. - */ -static void bdrv_put_ref_bh_schedule(BlockDriverState *bs) -{ - BDRVPutRefBH *s; - - s = g_new(BDRVPutRefBH, 1); - s->bh = qemu_bh_new(bdrv_put_ref_bh, s); - s->bs = bs; - qemu_bh_schedule(s->bh); -} - static int parse_block_error_action(const char *buf, bool is_read, Error **errp) { if (!strcmp(buf, "ignore")) { @@ -2546,8 +2520,6 @@ static void block_job_cb(void *opaque, int ret) } else { block_job_event_completed(bs->job, msg); } - - bdrv_put_ref_bh_schedule(bs); } void qmp_block_stream(const char *device, diff --git a/blockjob.c b/blockjob.c index c02fe59..ae9c5b2 100644 --- a/blockjob.c +++ b/blockjob.c @@ -60,6 +60,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, job->cb = cb; job->opaque = opaque; job->busy = true; + job->refcnt = 1; bs->job = job; /* Only set speed when necessary to avoid NotSupported error */ @@ -68,7 +69,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, block_job_set_speed(job, speed, &local_err); if (local_err) { - block_job_release(bs); + block_job_unref(job); error_propagate(errp, local_err); return NULL; } @@ -76,15 +77,21 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, return job; } -void block_job_release(BlockDriverState *bs) +void block_job_ref(BlockJob *job) { - BlockJob *job = bs->job; + ++job->refcnt; +} - bs->job = NULL; - bdrv_op_unblock_all(bs, job->blocker); - error_free(job->blocker); - g_free(job->id); - g_free(job); +void block_job_unref(BlockJob *job) +{ + if (--job->refcnt == 0) { + job->bs->job = NULL; + bdrv_op_unblock_all(job->bs, job->blocker); + bdrv_unref(job->bs); + error_free(job->blocker); + g_free(job->id); + g_free(job); + } } void block_job_completed(BlockJob *job, int ret) @@ -93,7 +100,7 @@ void block_job_completed(BlockJob *job, int ret) assert(bs->job == job); job->cb(job->opaque, ret); - block_job_release(bs); + block_job_unref(job); } void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 289b13f..b649a40 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -130,6 +130,9 @@ struct BlockJob { /** The opaque value that is passed to the completion function. */ void *opaque; + + /** Reference count of the block job */ + int refcnt; }; /** @@ -174,12 +177,21 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); void block_job_yield(BlockJob *job); /** - * block_job_release: + * block_job_ref: * @bs: The block device. * - * Release job resources when an error occurred or job completed. + * Grab a reference to the block job. Should be paired with block_job_unref. */ -void block_job_release(BlockDriverState *bs); +void block_job_ref(BlockJob *job); + +/** + * block_job_unref: + * @bs: The block device. + * + * Release reference to the block job and release resources if it is the last + * reference. + */ +void block_job_unref(BlockJob *job); /** * block_job_completed: diff --git a/qemu-img.c b/qemu-img.c index 3025776..510fdbd 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -645,9 +645,6 @@ static void common_block_job_cb(void *opaque, int ret) if (ret < 0) { error_setg_errno(cbi->errp, -ret, "Block job failed"); } - - /* Drop this block job's reference */ - bdrv_unref(cbi->bs); } static void run_block_job(BlockJob *job, Error **errp) -- 2.5.0