qemu-block
[Top][All Lists]
Advanced

[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
> 
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]