Commit b64bd51e authored by Paolo Bonzini's avatar Paolo Bonzini Committed by Fam Zheng
Browse files

block: protect modification of dirty bitmaps with a mutex 



Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
Message-Id: <20170605123908.18777-17-pbonzini@redhat.com>
Signed-off-by: default avatarFam Zheng <famz@redhat.com>
parent c0bad499

block/dirty-bitmap.c

+60 −8
Original line number Diff line number Diff line
@@ -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(BlockDriverState *bs) 
    qemu_mutex_unlock(&bs->dirty_bitmap_mutex);

}



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(BlockDriverState *bs, 
        return NULL;

    }

    bitmap = g_new0(BdrvDirtyBitmap, 1);

    bitmap->mutex = &bs->dirty_bitmap_mutex;

    bitmap->bitmap = hbitmap_alloc(bitmap_size, ctz32(sector_granularity));

    bitmap->size = bitmap_size;

    bitmap->name = g_strdup(name);
@@ -134,18 +146,22 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, 
                                   int chunk_size)

{

    assert(!bitmap->meta);

    qemu_mutex_lock(bitmap->mutex);

    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,

                                       chunk_size * BITS_PER_BYTE);

    qemu_mutex_unlock(bitmap->mutex);

}



void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)

{

    assert(bitmap->meta);

    qemu_mutex_lock(bitmap->mutex);

    hbitmap_free_meta(bitmap->bitmap);

    bitmap->meta = NULL;

    qemu_mutex_unlock(bitmap->mutex);

}



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)

{
@@ -162,11 +178,26 @@ int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, 
    return false;

}



int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,

                               BdrvDirtyBitmap *bitmap, int64_t sector,

                               int nb_sectors)

{

    bool dirty;



    qemu_mutex_lock(bitmap->mutex);

    dirty = bdrv_dirty_bitmap_get_meta_locked(bs, bitmap, sector, nb_sectors);

    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);

}



int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
@@ -393,7 +424,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) 
    return list;

}



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) {
@@ -467,23 +499,42 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter) 
    return hbitmap_iter_next(&iter->hbi);

}



void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,

/* 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_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,

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);

}



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, HBitmap **out) 
                                       hbitmap_granularity(backup));

        *out = backup;

    }

    bdrv_dirty_bitmap_unlock(bitmap);

}



void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)

block/mirror.c

+8 −3
Original line number Diff line number Diff line
@@ -342,6 +342,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) 
    int max_io_sectors = MAX((s->buf_size >> BDRV_SECTOR_BITS) / MAX_IN_FLIGHT,

                             MAX_IO_SECTORS);



    bdrv_dirty_bitmap_lock(s->dirty_bitmap);

    sector_num = 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(MirrorBlockJob *s) 
        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));

        assert(sector_num >= 0);

    }

    bdrv_dirty_bitmap_unlock(s->dirty_bitmap);



    first_chunk = 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(MirrorBlockJob *s) 


    /* 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_BITS)) {

        int64_t next_dirty;

        int64_t next_sector = sector_num + nb_chunks * sectors_per_chunk;

        int64_t next_chunk = next_sector / sectors_per_chunk;

        if (next_sector >= 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(MirrorBlockJob *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,

    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_chunks);

    while (nb_chunks > 0 && sector_num < end) {

        int64_t ret;

include/block/block_int.h

+2 −2
Original line number Diff line number Diff line
@@ -611,8 +611,8 @@ struct BlockDriverState { 


    /* 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;

include/block/dirty-bitmap.h

+18 −7
Original line number Diff line number Diff line
@@ -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 sector,

                                      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(BdrvDirtyBitmap *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);



uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,

                                              uint64_t start, uint64_t count);
@@ -72,4 +68,19 @@ void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap, 
                                          bool finish);

void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);



/* 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

migration/block.c

+6 −4
Original line number Diff line number Diff line
@@ -525,14 +525,15 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *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 = total_sectors - sector;

            } else {

                nr_sectors = 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);



            blk = g_new(BlkMigBlock, 1);

            blk->buf = g_malloc(BLOCK_SIZE);
@@ -568,9 +569,10 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, 


            sector += nr_sectors;

            bmds->cur_dirty = sector;



            break;

        }



        bdrv_dirty_bitmap_unlock(bmds->dirty_bitmap);

        sector += BDRV_SECTORS_PER_DIRTY_CHUNK;

        bmds->cur_dirty = sector;

    }