From nobody Sat Feb 7 05:52:33 2026 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1496666695621899.8685844845418; Mon, 5 Jun 2017 05:44:55 -0700 (PDT) Received: from localhost ([::1]:33155 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dHrNO-0004m5-3w for importer@patchew.org; Mon, 05 Jun 2017 08:44:54 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dHrI9-0008WC-SY for qemu-devel@nongnu.org; Mon, 05 Jun 2017 08:39:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dHrI8-000627-3f for qemu-devel@nongnu.org; Mon, 05 Jun 2017 08:39:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37014) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dHrI7-00060c-RW for qemu-devel@nongnu.org; Mon, 05 Jun 2017 08:39:28 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DFBFD80467 for ; Mon, 5 Jun 2017 12:39:26 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-117-97.ams2.redhat.com [10.36.117.97]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v55Cd83P010640; Mon, 5 Jun 2017 08:39:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DFBFD80467 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com DFBFD80467 From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 5 Jun 2017 14:39:03 +0200 Message-Id: <20170605123908.18777-15-pbonzini@redhat.com> In-Reply-To: <20170605123908.18777-1-pbonzini@redhat.com> References: <20170605123908.18777-1-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 05 Jun 2017 12:39:27 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 14/19] block: introduce dirty_bitmap_mutex 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: famz@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" It protects only the list of dirty bitmaps; in the next patch we will also protect their content. Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/dirty-bitmap.c | 44 +++++++++++++++++++++++++++++++++++++++++++- block/mirror.c | 3 ++- blockdev.c | 44 +++++++------------------------------------- include/block/block_int.h | 5 +++++ migration/block.c | 6 ------ 5 files changed, 57 insertions(+), 45 deletions(-) diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 519737c8d3..fa78109365 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -52,6 +52,17 @@ struct BdrvDirtyBitmapIter { BdrvDirtyBitmap *bitmap; }; =20 +static inline void bdrv_dirty_bitmaps_lock(BlockDriverState *bs) +{ + qemu_mutex_lock(&bs->dirty_bitmap_mutex); +} + +static inline void bdrv_dirty_bitmaps_unlock(BlockDriverState *bs) +{ + qemu_mutex_unlock(&bs->dirty_bitmap_mutex); +} + +/* Called with BQL or dirty_bitmap lock taken. */ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *= name) { BdrvDirtyBitmap *bm; @@ -65,6 +76,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState = *bs, const char *name) return NULL; } =20 +/* Called with BQL taken. */ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); @@ -72,6 +84,7 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap) bitmap->name =3D NULL; } =20 +/* Called with BQL taken. */ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, uint32_t granularity, const char *name, @@ -100,7 +113,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverSt= ate *bs, bitmap->size =3D bitmap_size; bitmap->name =3D g_strdup(name); bitmap->disabled =3D false; + bdrv_dirty_bitmaps_lock(bs); QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); + bdrv_dirty_bitmaps_unlock(bs); return bitmap; } =20 @@ -164,16 +179,19 @@ const char *bdrv_dirty_bitmap_name(const BdrvDirtyBit= map *bitmap) return bitmap->name; } =20 +/* Called with BQL taken. */ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap) { return bitmap->successor; } =20 +/* Called with BQL taken. */ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap) { return !(bitmap->disabled || bitmap->successor); } =20 +/* Called with BQL taken. */ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap) { if (bdrv_dirty_bitmap_frozen(bitmap)) { @@ -188,6 +206,7 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBit= map *bitmap) /** * Create a successor bitmap destined to replace this bitmap after an oper= ation. * Requires that the bitmap is not frozen and has no successor. + * Called with BQL taken. */ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, Error **er= rp) @@ -220,6 +239,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState= *bs, /** * For a bitmap with a successor, yield our name to the successor, * delete the old bitmap, and return a handle to the new bitmap. + * Called with BQL taken. */ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, @@ -247,6 +267,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriver= State *bs, * In cases of failure where we can no longer safely delete the parent, * we may wish to re-join the parent and child/successor. * The merged parent will be un-frozen, but not explicitly re-enabled. + * Called with BQL taken. */ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *parent, @@ -271,25 +292,30 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDrive= rState *bs, =20 /** * Truncates _all_ bitmaps attached to a BDS. + * Called with BQL taken. */ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) { BdrvDirtyBitmap *bitmap; uint64_t size =3D bdrv_nb_sectors(bs); =20 + bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); assert(!bitmap->active_iterators); hbitmap_truncate(bitmap->bitmap, size); bitmap->size =3D size; } + bdrv_dirty_bitmaps_unlock(bs); } =20 +/* Called with BQL taken. */ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, bool only_named) { BdrvDirtyBitmap *bm, *next; + bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) { if ((!bitmap || bm =3D=3D bitmap) && (!only_named || bm->name)) { assert(!bm->active_iterators); @@ -301,15 +327,19 @@ static void bdrv_do_release_matching_dirty_bitmap(Blo= ckDriverState *bs, g_free(bm); =20 if (bitmap) { - return; + goto out; } } } if (bitmap) { abort(); } + +out: + bdrv_dirty_bitmaps_unlock(bs); } =20 +/* Called with BQL taken. */ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitm= ap) { bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false); @@ -318,18 +348,21 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, = BdrvDirtyBitmap *bitmap) /** * Release all named dirty bitmaps attached to a BDS (for use in bdrv_clos= e()). * There must not be any frozen bitmaps attached. + * Called with BQL taken. */ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) { bdrv_do_release_matching_dirty_bitmap(bs, NULL, true); } =20 +/* Called with BQL taken. */ void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); bitmap->disabled =3D true; } =20 +/* Called with BQL taken. */ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); @@ -342,6 +375,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDrive= rState *bs) BlockDirtyInfoList *list =3D NULL; BlockDirtyInfoList **plist =3D &list; =20 + bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { BlockDirtyInfo *info =3D g_new0(BlockDirtyInfo, 1); BlockDirtyInfoList *entry =3D g_new0(BlockDirtyInfoList, 1); @@ -354,6 +388,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDrive= rState *bs) *plist =3D entry; plist =3D &entry->next; } + bdrv_dirty_bitmaps_unlock(bs); =20 return list; } @@ -508,12 +543,19 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur= _sector, int64_t nr_sectors) { BdrvDirtyBitmap *bitmap; + + if (QLIST_EMPTY(&bs->dirty_bitmaps)) { + return; + } + + bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { if (!bdrv_dirty_bitmap_enabled(bitmap)) { continue; } hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors); } + bdrv_dirty_bitmaps_unlock(bs); } =20 /** diff --git a/block/mirror.c b/block/mirror.c index a2a970301c..88ae882c46 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -506,6 +506,8 @@ static void mirror_exit(BlockJob *job, void *opaque) BlockDriverState *mirror_top_bs =3D s->mirror_top_bs; Error *local_err =3D NULL; =20 + bdrv_release_dirty_bitmap(src, s->dirty_bitmap); + /* Make sure that the source BDS doesn't go away before we called * block_job_completed(). */ bdrv_ref(src); @@ -904,7 +906,6 @@ immediate_exit: g_free(s->cow_bitmap); g_free(s->in_flight_bitmap); bdrv_dirty_iter_free(s->dbi); - bdrv_release_dirty_bitmap(bs, s->dirty_bitmap); =20 data =3D g_malloc(sizeof(*data)); data->ret =3D ret; diff --git a/blockdev.c b/blockdev.c index 335fbccf5c..6e7c8a678c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1362,12 +1362,10 @@ out_aio_context: static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node, const char *name, BlockDriverState **pbs, - AioContext **paio, Error **errp) { BlockDriverState *bs; BdrvDirtyBitmap *bitmap; - AioContext *aio_context; =20 if (!node) { error_setg(errp, "Node cannot be NULL"); @@ -1383,29 +1381,17 @@ static BdrvDirtyBitmap *block_dirty_bitmap_lookup(c= onst char *node, return NULL; } =20 - aio_context =3D bdrv_get_aio_context(bs); - aio_context_acquire(aio_context); - bitmap =3D bdrv_find_dirty_bitmap(bs, name); if (!bitmap) { error_setg(errp, "Dirty bitmap '%s' not found", name); - goto fail; + return NULL; } =20 if (pbs) { *pbs =3D bs; } - if (paio) { - *paio =3D aio_context; - } else { - aio_context_release(aio_context); - } =20 return bitmap; - - fail: - aio_context_release(aio_context); - return NULL; } =20 /* New and old BlockDriverState structs for atomic group operations */ @@ -2021,7 +2007,6 @@ static void block_dirty_bitmap_clear_prepare(BlkActio= nState *common, state->bitmap =3D block_dirty_bitmap_lookup(action->node, action->name, &state->bs, - &state->aio_context, errp); if (!state->bitmap) { return; @@ -2729,7 +2714,6 @@ void qmp_block_dirty_bitmap_add(const char *node, con= st char *name, bool has_granularity, uint32_t granularity, Error **errp) { - AioContext *aio_context; BlockDriverState *bs; =20 if (!name || name[0] =3D=3D '\0') { @@ -2742,14 +2726,11 @@ void qmp_block_dirty_bitmap_add(const char *node, c= onst char *name, return; } =20 - aio_context =3D bdrv_get_aio_context(bs); - aio_context_acquire(aio_context); - if (has_granularity) { if (granularity < 512 || !is_power_of_2(granularity)) { error_setg(errp, "Granularity must be power of 2 " "and at least 512"); - goto out; + return; } } else { /* Default to cluster size, if available: */ @@ -2757,19 +2738,15 @@ void qmp_block_dirty_bitmap_add(const char *node, c= onst char *name, } =20 bdrv_create_dirty_bitmap(bs, granularity, name, errp); - - out: - aio_context_release(aio_context); } =20 void qmp_block_dirty_bitmap_remove(const char *node, const char *name, Error **errp) { - AioContext *aio_context; BlockDriverState *bs; BdrvDirtyBitmap *bitmap; =20 - bitmap =3D block_dirty_bitmap_lookup(node, name, &bs, &aio_context, er= rp); + bitmap =3D block_dirty_bitmap_lookup(node, name, &bs, errp); if (!bitmap || !bs) { return; } @@ -2778,13 +2755,10 @@ void qmp_block_dirty_bitmap_remove(const char *node= , const char *name, error_setg(errp, "Bitmap '%s' is currently frozen and cannot be removed", name); - goto out; + return; } bdrv_dirty_bitmap_make_anon(bitmap); bdrv_release_dirty_bitmap(bs, bitmap); - - out: - aio_context_release(aio_context); } =20 /** @@ -2794,11 +2768,10 @@ void qmp_block_dirty_bitmap_remove(const char *node= , const char *name, void qmp_block_dirty_bitmap_clear(const char *node, const char *name, Error **errp) { - AioContext *aio_context; BdrvDirtyBitmap *bitmap; BlockDriverState *bs; =20 - bitmap =3D block_dirty_bitmap_lookup(node, name, &bs, &aio_context, er= rp); + bitmap =3D block_dirty_bitmap_lookup(node, name, &bs, errp); if (!bitmap || !bs) { return; } @@ -2807,18 +2780,15 @@ void qmp_block_dirty_bitmap_clear(const char *node,= const char *name, error_setg(errp, "Bitmap '%s' is currently frozen and cannot be modified= ", name); - goto out; + return; } else if (!bdrv_dirty_bitmap_enabled(bitmap)) { error_setg(errp, "Bitmap '%s' is currently disabled and cannot be cleare= d", name); - goto out; + return; } =20 bdrv_clear_dirty_bitmap(bitmap, NULL); - - out: - aio_context_release(aio_context); } =20 void hmp_drive_del(Monitor *mon, const QDict *qdict) diff --git a/include/block/block_int.h b/include/block/block_int.h index 31fb3640f1..c834b866a7 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -609,6 +609,11 @@ struct BlockDriverState { uint64_t write_threshold_offset; NotifierWithReturn write_threshold_notifier; =20 + /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex. + * Reading from the list can be done with either the BQL or the + * dirty_bitmap_mutex. Modifying a bitmap requires the AioContext + * lock. */ + QemuMutex dirty_bitmap_mutex; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; =20 /* Offset after the highest byte written to */ diff --git a/migration/block.c b/migration/block.c index 13f90d3f17..9e9f0316be 100644 --- a/migration/block.c +++ b/migration/block.c @@ -345,10 +345,8 @@ static int set_dirty_tracking(void) int ret; =20 QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { - aio_context_acquire(blk_get_aio_context(bmds->blk)); bmds->dirty_bitmap =3D bdrv_create_dirty_bitmap(blk_bs(bmds->blk), BLOCK_SIZE, NULL, NU= LL); - aio_context_release(blk_get_aio_context(bmds->blk)); if (!bmds->dirty_bitmap) { ret =3D -errno; goto fail; @@ -359,9 +357,7 @@ static int set_dirty_tracking(void) fail: QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { if (bmds->dirty_bitmap) { - aio_context_acquire(blk_get_aio_context(bmds->blk)); bdrv_release_dirty_bitmap(blk_bs(bmds->blk), bmds->dirty_bitma= p); - aio_context_release(blk_get_aio_context(bmds->blk)); } } return ret; @@ -374,9 +370,7 @@ static void unset_dirty_tracking(void) BlkMigDevState *bmds; =20 QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { - aio_context_acquire(blk_get_aio_context(bmds->blk)); bdrv_release_dirty_bitmap(blk_bs(bmds->blk), bmds->dirty_bitmap); - aio_context_release(blk_get_aio_context(bmds->blk)); } } =20 --=20 2.13.0