|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-devel] [PATCH v9 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap |
Date: | Thu, 28 Dec 2017 14:16:06 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
28.12.2017 08:24, Fam Zheng wrote:
On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden> --- include/block/dirty-bitmap.h | 3 +++ block/dirty-bitmap.c | 25 ++++++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 93d4336505..caf1f3d861 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -92,5 +92,8 @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp); +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp);#endifdiff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index d83da077d5..fe27ddfb83 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -327,15 +327,11 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, * The merged parent will be un-frozen, but not explicitly re-enabled. * Called with BQL taken.Maybe update the comment as s/Called with BQL taken/Called within bdrv_dirty_bitmap_lock..unlock/ and ...
we have the following comment: /* 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 only requires * dirty_bitmap_mutex. */ QemuMutex dirty_bitmap_mutex;(I don't understand well the logic, why is it so. Paolo introduced the lock, but didn't update some functions..)
so, actually, here we need both BQL and mutex.
*/ -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, - BdrvDirtyBitmap *parent, - Error **errp) +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, + BdrvDirtyBitmap *parent, + Error **errp) { - BdrvDirtyBitmap *successor; - - qemu_mutex_lock(parent->mutex); - - successor = parent->successor; + BdrvDirtyBitmap *successor = parent->successor;if (!successor) {error_setg(errp, "Cannot reclaim a successor when none is present"); @@ -349,9 +345,20 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, bdrv_release_dirty_bitmap_locked(bs, successor); parent->successor = NULL;+ return parent;+} +... move the "Called with BQL taken" comment here?
and here BQL. Ok, I'll add/fix comments.
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *parent, + Error **errp) +{ + BdrvDirtyBitmap *ret; + + qemu_mutex_lock(parent->mutex); + ret = bdrv_reclaim_dirty_bitmap_locked(bs, parent, errp); qemu_mutex_unlock(parent->mutex);- return parent;+ return ret; }/**-- 2.11.1
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |