[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 15/17] block: introduce dirty_bitma
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 15/17] block: introduce dirty_bitmap_mutex |
Date: |
Thu, 4 May 2017 11:57:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 04/05/2017 09:55, Fam Zheng wrote:
> On Thu, 04/20 14:00, Paolo Bonzini wrote:
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 519737c..e13718e 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -52,6 +52,24 @@ struct BdrvDirtyBitmapIter {
>> BdrvDirtyBitmap *bitmap;
>> };
>>
>> +static QemuMutex dirty_bitmap_mutex;
>> +
>> +static void __attribute__((__constructor__))
>> bdrv_dirty_bitmaps_init_lock(void)
>> +{
>> + qemu_mutex_init(&dirty_bitmap_mutex);
>> +}
>> +
>> +static inline void bdrv_dirty_bitmaps_lock(BlockDriverState *bs)
>> +{
>> + qemu_mutex_lock(&dirty_bitmap_mutex);
>> +}
>> +
>> +static inline void bdrv_dirty_bitmaps_unlock(BlockDriverState *bs)
>> +{
>> + qemu_mutex_unlock(&dirty_bitmap_mutex);
>> +}
>
> Why a global lock instead of a per-BDS one? The contention can be heavy if a
> block job is made to run on a different thread than the one processing guest
> I/O.
Yes, I'll introduce bs->dirty_bitmap_mutex in this patch already.
>> @@ -508,12 +550,19 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t
>> cur_sector,
>> int64_t nr_sectors)
>> {
>> BdrvDirtyBitmap *bitmap;
>> +
>> + if (QLIST_EMPTY(&bs->dirty_bitmaps)) {
>> + return;
>> + }
>
> Should this check be protected by lock/unlock? Or simply removed?
The check avoids taking the lock in the common case of having no dirty
bitmap.
Paolo
>> +
>> + bdrv_dirty_bitmaps_lock(bs);
>> QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
>> if (!bdrv_dirty_bitmap_enabled(bitmap)) {
>> continue;
>> }
>> hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
>> }
>> + bdrv_dirty_bitmaps_unlock(bs);
>> }
>>
>> /**
>
> Fam
>
>