From nobody Wed Nov 5 19:41:24 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.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 1496391991231152.4907750199642; Fri, 2 Jun 2017 01:26:31 -0700 (PDT) Received: from localhost ([::1]:48495 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dGhue-0006Kg-VU for importer@patchew.org; Fri, 02 Jun 2017 04:26:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54806) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dGhgy-0002UK-Dh for qemu-devel@nongnu.org; Fri, 02 Jun 2017 04:12:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dGhgw-0002YN-G6 for qemu-devel@nongnu.org; Fri, 02 Jun 2017 04:12:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53396) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dGhgw-0002Wu-5t for qemu-devel@nongnu.org; Fri, 02 Jun 2017 04:12:18 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3CF16142A7D; Fri, 2 Jun 2017 08:12:17 +0000 (UTC) Received: from lemon.redhat.com (ovpn-12-49.pek2.redhat.com [10.72.12.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id A3FDD60A99; Fri, 2 Jun 2017 08:12:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3CF16142A7D Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=famz@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3CF16142A7D From: Fam Zheng To: qemu-devel@nongnu.org Date: Fri, 2 Jun 2017 16:11:18 +0800 Message-Id: <20170602081120.3497-21-famz@redhat.com> In-Reply-To: <20170602081120.3497-1-famz@redhat.com> References: <20170602081120.3497-1-famz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 02 Jun 2017 08:12:17 +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] [PULL v2 20/22] block: protect modification of dirty bitmaps with a 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: peter.maydell@linaro.org, Stefan Hajnoczi 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" From: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini Message-Id: <20170525163225.29954-17-pbonzini@redhat.com> Signed-off-by: Fam Zheng --- block/dirty-bitmap.c | 70 ++++++++++++++++++++++++++++++++++++++--= ---- block/mirror.c | 11 +++++-- include/block/block_int.h | 4 +-- include/block/dirty-bitmap.h | 25 +++++++++++----- migration/block.c | 10 ++++--- 5 files changed, 95 insertions(+), 25 deletions(-) diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index fa78109..a04c6e4 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -37,6 +37,7 @@ * or enabled. A frozen bitmap can only abdicate() or reclaim(). */ struct BdrvDirtyBitmap { + QemuMutex *mutex; HBitmap *bitmap; /* Dirty sector bitmap implementation */ HBitmap *meta; /* Meta dirty bitmap */ BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status = */ @@ -62,6 +63,16 @@ static inline void bdrv_dirty_bitmaps_unlock(BlockDriver= State *bs) qemu_mutex_unlock(&bs->dirty_bitmap_mutex); } =20 +void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap) +{ + qemu_mutex_lock(bitmap->mutex); +} + +void bdrv_dirty_bitmap_unlock(BdrvDirtyBitmap *bitmap) +{ + qemu_mutex_unlock(bitmap->mutex); +} + /* Called with BQL or dirty_bitmap lock taken. */ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *= name) { @@ -109,6 +120,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverSt= ate *bs, return NULL; } bitmap =3D g_new0(BdrvDirtyBitmap, 1); + bitmap->mutex =3D &bs->dirty_bitmap_mutex; bitmap->bitmap =3D hbitmap_alloc(bitmap_size, ctz32(sector_granularity= )); bitmap->size =3D bitmap_size; bitmap->name =3D g_strdup(name); @@ -134,20 +146,24 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *b= itmap, int chunk_size) { assert(!bitmap->meta); + qemu_mutex_lock(bitmap->mutex); bitmap->meta =3D hbitmap_create_meta(bitmap->bitmap, chunk_size * BITS_PER_BYTE); + qemu_mutex_unlock(bitmap->mutex); } =20 void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap) { assert(bitmap->meta); + qemu_mutex_lock(bitmap->mutex); hbitmap_free_meta(bitmap->bitmap); bitmap->meta =3D NULL; + qemu_mutex_unlock(bitmap->mutex); } =20 -int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, int64_t sector, - int nb_sectors) +int bdrv_dirty_bitmap_get_meta_locked(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, int64_t sec= tor, + int nb_sectors) { uint64_t i; int sectors_per_bit =3D 1 << hbitmap_granularity(bitmap->meta); @@ -162,11 +178,26 @@ int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, return false; } =20 +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, int64_t sector, + int nb_sectors) +{ + bool dirty; + + qemu_mutex_lock(bitmap->mutex); + dirty =3D bdrv_dirty_bitmap_get_meta_locked(bs, bitmap, sector, nb_sec= tors); + qemu_mutex_unlock(bitmap->mutex); + + return dirty; +} + void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector, int nb_sectors) { + qemu_mutex_lock(bitmap->mutex); hbitmap_reset(bitmap->meta, sector, nb_sectors); + qemu_mutex_unlock(bitmap->mutex); } =20 int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap) @@ -393,8 +424,9 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDrive= rState *bs) return list; } =20 -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, - int64_t sector) +/* Called within bdrv_dirty_bitmap_lock..unlock */ +int bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, + int64_t sector) { if (bitmap) { return hbitmap_get(bitmap->bitmap, sector); @@ -467,23 +499,42 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *ite= r) return hbitmap_iter_next(&iter->hbi); } =20 -void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, - int64_t cur_sector, int64_t nr_sectors) +/* Called within bdrv_dirty_bitmap_lock..unlock */ +void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors) { assert(bdrv_dirty_bitmap_enabled(bitmap)); hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors); } =20 -void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, - int64_t cur_sector, int64_t nr_sectors) +void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors) +{ + bdrv_dirty_bitmap_lock(bitmap); + bdrv_set_dirty_bitmap_locked(bitmap, cur_sector, nr_sectors); + bdrv_dirty_bitmap_unlock(bitmap); +} + +/* Called within bdrv_dirty_bitmap_lock..unlock */ +void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors) { assert(bdrv_dirty_bitmap_enabled(bitmap)); hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); } =20 +void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors) +{ + bdrv_dirty_bitmap_lock(bitmap); + bdrv_reset_dirty_bitmap_locked(bitmap, cur_sector, nr_sectors); + bdrv_dirty_bitmap_unlock(bitmap); +} + void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out) { assert(bdrv_dirty_bitmap_enabled(bitmap)); + bdrv_dirty_bitmap_lock(bitmap); if (!out) { hbitmap_reset_all(bitmap->bitmap); } else { @@ -492,6 +543,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, H= Bitmap **out) hbitmap_granularity(backup)); *out =3D backup; } + bdrv_dirty_bitmap_unlock(bitmap); } =20 void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in) diff --git a/block/mirror.c b/block/mirror.c index 88ae882..19afcc6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -342,6 +342,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlo= ckJob *s) int max_io_sectors =3D MAX((s->buf_size >> BDRV_SECTOR_BITS) / MAX_IN_= FLIGHT, MAX_IO_SECTORS); =20 + bdrv_dirty_bitmap_lock(s->dirty_bitmap); sector_num =3D bdrv_dirty_iter_next(s->dbi); if (sector_num < 0) { bdrv_set_dirty_iter(s->dbi, 0); @@ -349,6 +350,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlo= ckJob *s) trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap)= ); assert(sector_num >=3D 0); } + bdrv_dirty_bitmap_unlock(s->dirty_bitmap); =20 first_chunk =3D sector_num / sectors_per_chunk; while (test_bit(first_chunk, s->in_flight_bitmap)) { @@ -360,12 +362,13 @@ static uint64_t coroutine_fn mirror_iteration(MirrorB= lockJob *s) =20 /* Find the number of consective dirty chunks following the first dirty * one, and wait for in flight requests in them. */ + bdrv_dirty_bitmap_lock(s->dirty_bitmap); while (nb_chunks * sectors_per_chunk < (s->buf_size >> BDRV_SECTOR_BIT= S)) { int64_t next_dirty; int64_t next_sector =3D sector_num + nb_chunks * sectors_per_chunk; int64_t next_chunk =3D next_sector / sectors_per_chunk; if (next_sector >=3D end || - !bdrv_get_dirty(source, s->dirty_bitmap, next_sector)) { + !bdrv_get_dirty_locked(source, s->dirty_bitmap, next_sector)) { break; } if (test_bit(next_chunk, s->in_flight_bitmap)) { @@ -386,8 +389,10 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBl= ockJob *s) * calling bdrv_get_block_status_above could yield - if some blocks are * marked dirty in this window, we need to know. */ - bdrv_reset_dirty_bitmap(s->dirty_bitmap, sector_num, - nb_chunks * sectors_per_chunk); + bdrv_reset_dirty_bitmap_locked(s->dirty_bitmap, sector_num, + nb_chunks * sectors_per_chunk); + bdrv_dirty_bitmap_unlock(s->dirty_bitmap); + bitmap_set(s->in_flight_bitmap, sector_num / sectors_per_chunk, nb_chu= nks); while (nb_chunks > 0 && sector_num < end) { int64_t ret; diff --git a/include/block/block_int.h b/include/block/block_int.h index c834b86..15c656d 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -611,8 +611,8 @@ struct BlockDriverState { =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. */ + * dirty_bitmap_mutex. Modifying a bitmap only requires + * dirty_bitmap_mutex. */ QemuMutex dirty_bitmap_mutex; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; =20 diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 9dea14b..ad6558a 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -36,8 +36,6 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap); const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap); int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap); DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap); -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, - int64_t sector); void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t cur_sector, int64_t nr_sectors); void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, @@ -45,6 +43,9 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector, int nb_sectors); +int bdrv_dirty_bitmap_get_meta_locked(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, int64_t sec= tor, + int nb_sectors); void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector, int nb_sectors); @@ -52,11 +53,6 @@ BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyB= itmap *bitmap); BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap, uint64_t first_sector); void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter); -int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); -void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); -int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); =20 uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitma= p, uint64_t start, uint64_t cou= nt); @@ -72,4 +68,19 @@ void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitma= p *bitmap, bool finish); void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap); =20 +/* Functions that require manual locking. */ +void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap); +void bdrv_dirty_bitmap_unlock(BdrvDirtyBitmap *bitmap); +int bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, + int64_t sector); +void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors); +void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, + int64_t cur_sector, int64_t nr_sectors= ); +int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); +int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); + #endif diff --git a/migration/block.c b/migration/block.c index 8fe484e..d102c84 100644 --- a/migration/block.c +++ b/migration/block.c @@ -529,14 +529,15 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigD= evState *bmds, } else { blk_mig_unlock(); } - if (bdrv_get_dirty(bs, bmds->dirty_bitmap, sector)) { - + bdrv_dirty_bitmap_lock(bmds->dirty_bitmap); + if (bdrv_get_dirty_locked(bs, bmds->dirty_bitmap, sector)) { if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) { nr_sectors =3D total_sectors - sector; } else { nr_sectors =3D BDRV_SECTORS_PER_DIRTY_CHUNK; } - bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors= ); + bdrv_reset_dirty_bitmap_locked(bmds->dirty_bitmap, sector, nr_= sectors); + bdrv_dirty_bitmap_unlock(bmds->dirty_bitmap); =20 blk =3D g_new(BlkMigBlock, 1); blk->buf =3D g_malloc(BLOCK_SIZE); @@ -572,9 +573,10 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDe= vState *bmds, =20 sector +=3D nr_sectors; bmds->cur_dirty =3D sector; - break; } + + bdrv_dirty_bitmap_unlock(bmds->dirty_bitmap); sector +=3D BDRV_SECTORS_PER_DIRTY_CHUNK; bmds->cur_dirty =3D sector; } --=20 2.9.4