[Qemu-devel] [PATCH v3 1/2] block: Introduce coroutine lock to dirty bitmap

[Fam Zheng]

Typically, what a dirty bit consumer does is 1) get the next dirty
sectors; 2) do something with the sectors; 3) clear the dirty bits; 4)
goto 1). This works as long as 2) is simple and atomic in the coroutine
sense.  Anything sophisticated requires either moving 3) before 2) or
using locks, because the dirty bits may get cleared in the middle when
the coroutine yield.

This will be the case for mirror.c in following patches, so introduce
CoMutex in BdrvDirtyBitmap to allowing blocking the producer.

Also mark all involved dirty bitmap functions as coroutine_fn.

Signed-off-by: Fam Zheng <address@hidden>
---
 block.c                   | 26 +++++++++++++++++++++-----
 include/block/block.h     |  6 ++++--
 include/block/block_int.h |  4 +++-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index e9f40dc..34a4109 100644
--- a/block.c
+++ b/block.c
@@ -69,6 +69,7 @@ struct BdrvDirtyBitmap {
     int64_t size;               /* Size of the bitmap (Number of sectors) */
     bool disabled;              /* Bitmap is read-only */
     QLIST_ENTRY(BdrvDirtyBitmap) list;
+    CoMutex lock;
 };
 
 #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress 
*/
@@ -3173,6 +3174,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
     bitmap->size = bitmap_size;
     bitmap->name = g_strdup(name);
     bitmap->disabled = false;
+    qemu_co_mutex_init(&bitmap->lock);
     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
     return bitmap;
 }
@@ -3385,11 +3387,24 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, 
HBitmapIter *hbi)
     hbitmap_iter_init(hbi, bitmap->bitmap, 0);
 }
 
-void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                           int64_t cur_sector, int nr_sectors)
+void coroutine_fn bdrv_lock_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    qemu_co_mutex_lock(&bitmap->lock);
+}
+
+void coroutine_fn bdrv_unlock_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    qemu_co_mutex_unlock(&bitmap->lock);
+}
+
+void coroutine_fn bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+                                        int64_t cur_sector,
+                                        int nr_sectors)
 {
     assert(bdrv_dirty_bitmap_enabled(bitmap));
+    bdrv_lock_dirty_bitmap(bitmap);
     hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+    bdrv_unlock_dirty_bitmap(bitmap);
 }
 
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
@@ -3405,15 +3420,16 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
     hbitmap_reset_all(bitmap->bitmap);
 }
 
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
-                    int nr_sectors)
+void coroutine_fn bdrv_set_dirty(BlockDriverState *bs,
+                                 int64_t cur_sector,
+                                 int nr_sectors)
 {
     BdrvDirtyBitmap *bitmap;
     QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
         if (!bdrv_dirty_bitmap_enabled(bitmap)) {
             continue;
         }
-        hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+        bdrv_set_dirty_bitmap(bitmap, cur_sector, nr_sectors);
     }
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index 610db92..592f317 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -488,9 +488,11 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap 
*bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
 DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
+void coroutine_fn bdrv_lock_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void coroutine_fn bdrv_unlock_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
-void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                           int64_t cur_sector, int nr_sectors);
+void coroutine_fn bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+                                        int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                              int64_t cur_sector, int nr_sectors);
 void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3ceeb5a..e17712c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -672,7 +672,9 @@ bool blk_dev_is_tray_open(BlockBackend *blk);
 bool blk_dev_is_medium_locked(BlockBackend *blk);
 void blk_dev_resize_cb(BlockBackend *blk);
 
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+void coroutine_fn bdrv_set_dirty(BlockDriverState *bs,
+                                 int64_t cur_sector,
+                                 int nr_sectors);
 bool bdrv_requests_pending(BlockDriverState *bs);
 
 #endif /* BLOCK_INT_H */
-- 
2.4.3