From nobody Mon Feb 9 17:36:55 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 1492690264580439.0127695202618; Thu, 20 Apr 2017 05:11:04 -0700 (PDT) Received: from localhost ([::1]:53582 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d1AvO-0006a4-NU for importer@patchew.org; Thu, 20 Apr 2017 08:11:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d1Amq-0007gi-5W for qemu-devel@nongnu.org; Thu, 20 Apr 2017 08:02:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d1Ami-00034o-Ri for qemu-devel@nongnu.org; Thu, 20 Apr 2017 08:02:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43880) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d1Am4-0002bA-GC; Thu, 20 Apr 2017 08:01:24 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 6A4A1C7074; Thu, 20 Apr 2017 12:01:23 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-117-9.ams2.redhat.com [10.36.117.9]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v3KC0w46002037; Thu, 20 Apr 2017 08:01:22 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6A4A1C7074 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6A4A1C7074 From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 20 Apr 2017 14:00:57 +0200 Message-Id: <20170420120058.28404-17-pbonzini@redhat.com> In-Reply-To: <20170420120058.28404-1-pbonzini@redhat.com> References: <20170420120058.28404-1-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 20 Apr 2017 12:01:23 +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 16/17] 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: qemu-block@nongnu.org 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" Signed-off-by: Paolo Bonzini --- block/dirty-bitmap.c | 74 ++++++++++++++++++++++++++++++++++++++++= +--- block/mirror.c | 11 +++++-- include/block/block_int.h | 4 +-- include/block/dirty-bitmap.h | 23 +++++++++++--- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index e13718e..b854077 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 = */ @@ -69,6 +70,16 @@ static inline void bdrv_dirty_bitmaps_unlock(BlockDriver= State *bs) qemu_mutex_unlock(&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) { @@ -116,6 +127,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); @@ -141,18 +153,22 @@ 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, +int bdrv_dirty_bitmap_get_meta_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector, int nb_sectors) { @@ -169,11 +185,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) @@ -400,7 +431,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDrive= rState *bs) return list; } =20 -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, +/* Called within bdrv_dirty_bitmap_lock..unlock */ +int bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector) { if (bitmap) { @@ -410,6 +442,18 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitm= ap *bitmap, } } =20 +int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, + int64_t sector) +{ + bool dirty; + + bdrv_dirty_bitmap_lock(bitmap); + dirty =3D bdrv_get_dirty_locked(bs, bitmap, sector); + bdrv_dirty_bitmap_unlock(bitmap); + + return dirty; +} + /** * Chooses a default granularity based on the existing cluster size, * but clamped between [4K, 64K]. Defaults to 64K in the case that there @@ -474,23 +518,42 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *ite= r) return hbitmap_iter_next(&iter->hbi); } =20 +/* 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); +} + 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_set(bitmap->bitmap, cur_sector, nr_sectors); + hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); } =20 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t cur_sector, int64_t nr_sectors) { - assert(bdrv_dirty_bitmap_enabled(bitmap)); - hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); + bdrv_dirty_bitmap_lock(bitmap); + bdrv_reset_dirty_bitmap_locked(bitmap, cur_sector, nr_sectors); + bdrv_dirty_bitmap_unlock(bitmap); } =20 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 { @@ -499,6 +562,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 dc227a2..6a5b0f8 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -344,10 +344,12 @@ static uint64_t coroutine_fn mirror_iteration(MirrorB= lockJob *s) =20 sector_num =3D bdrv_dirty_iter_next(s->dbi); if (sector_num < 0) { + bdrv_dirty_bitmap_lock(s->dirty_bitmap); bdrv_set_dirty_iter(s->dbi, 0); sector_num =3D bdrv_dirty_iter_next(s->dbi); 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; @@ -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 03db2cf..c264ead 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -598,8 +598,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..b6fc35b 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -45,6 +45,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 +55,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 +70,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 --=20 2.9.3