[v4] parallels: Add full dirty bitmap support

[PATCH v4 13/21] parallels: Handle L1 entries equal to one

Posted by Alexander Ivanov 11 months ago

If all the bits in a dirty bitmap cluster are ones, the cluster shouldn't
be written. Instead the corresponding L1 entry should be set to 1.

Check if all bits in a memory region are ones and set 1 to L1 entries
corresponding clusters filled with ones.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels-ext.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 195b01b109..033ca3ec3a 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -354,7 +354,7 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
     offset = 0;
     while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
         uint64_t idx = offset / limit;
-        int64_t cluster_off, end, write_size;
+        int64_t cluster_off, end, write_size, first_zero;
 
         offset = QEMU_ALIGN_DOWN(offset, limit);
         end = MIN(bm_size, offset + limit);
@@ -367,6 +367,16 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
             memset(bm_buf + write_size, 0, s->cluster_size - write_size);
         }
 
+        first_zero = bdrv_dirty_bitmap_next_zero(bitmap, offset, bm_size);
+        if (first_zero < 0) {
+            goto end;
+        }
+        if (first_zero - offset >= s->cluster_size) {
+            l1_table[idx] = 1;
+            offset = end;
+            continue;
+        }
+
         cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
         if (cluster_off <= 0) {
             goto end;
-- 
2.40.1

Re: [PATCH v4 13/21] parallels: Handle L1 entries equal to one

Posted by Denis V. Lunev 10 months, 1 week ago

On 12/28/23 11:12, Alexander Ivanov wrote:
> If all the bits in a dirty bitmap cluster are ones, the cluster shouldn't
> be written. Instead the corresponding L1 entry should be set to 1.
>
> Check if all bits in a memory region are ones and set 1 to L1 entries
> corresponding clusters filled with ones.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   block/parallels-ext.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/block/parallels-ext.c b/block/parallels-ext.c
> index 195b01b109..033ca3ec3a 100644
> --- a/block/parallels-ext.c
> +++ b/block/parallels-ext.c
> @@ -354,7 +354,7 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
>       offset = 0;
>       while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
>           uint64_t idx = offset / limit;
> -        int64_t cluster_off, end, write_size;
> +        int64_t cluster_off, end, write_size, first_zero;
>   
>           offset = QEMU_ALIGN_DOWN(offset, limit);
>           end = MIN(bm_size, offset + limit);
> @@ -367,6 +367,16 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
>               memset(bm_buf + write_size, 0, s->cluster_size - write_size);
>           }
>   
> +        first_zero = bdrv_dirty_bitmap_next_zero(bitmap, offset, bm_size);
> +        if (first_zero < 0) {
> +            goto end;
> +        }
> +        if (first_zero - offset >= s->cluster_size) {
> +            l1_table[idx] = 1;
> +            offset = end;
> +            continue;
> +        }
> +
>           cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
>           if (cluster_off <= 0) {
>               goto end;
That is not enough. We should handle all-one and all-zeroes according
to the spec and all-zeroes would be much more common.

Re: [PATCH v4 13/21] parallels: Handle L1 entries equal to one

Posted by Alexander Ivanov 9 months ago


On 1/18/24 14:37, Denis V. Lunev wrote:
> On 12/28/23 11:12, Alexander Ivanov wrote:
>> If all the bits in a dirty bitmap cluster are ones, the cluster 
>> shouldn't
>> be written. Instead the corresponding L1 entry should be set to 1.
>>
>> Check if all bits in a memory region are ones and set 1 to L1 entries
>> corresponding clusters filled with ones.
>>
>> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>> ---
>>   block/parallels-ext.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/parallels-ext.c b/block/parallels-ext.c
>> index 195b01b109..033ca3ec3a 100644
>> --- a/block/parallels-ext.c
>> +++ b/block/parallels-ext.c
>> @@ -354,7 +354,7 @@ static void GRAPH_RDLOCK 
>> parallels_save_bitmap(BlockDriverState *bs,
>>       offset = 0;
>>       while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, 
>> bm_size)) >= 0) {
>>           uint64_t idx = offset / limit;
>> -        int64_t cluster_off, end, write_size;
>> +        int64_t cluster_off, end, write_size, first_zero;
>>             offset = QEMU_ALIGN_DOWN(offset, limit);
>>           end = MIN(bm_size, offset + limit);
>> @@ -367,6 +367,16 @@ static void GRAPH_RDLOCK 
>> parallels_save_bitmap(BlockDriverState *bs,
>>               memset(bm_buf + write_size, 0, s->cluster_size - 
>> write_size);
>>           }
>>   +        first_zero = bdrv_dirty_bitmap_next_zero(bitmap, offset, 
>> bm_size);
>> +        if (first_zero < 0) {
>> +            goto end;
>> +        }
>> +        if (first_zero - offset >= s->cluster_size) {
>> +            l1_table[idx] = 1;
>> +            offset = end;
>> +            continue;
>> +        }
>> +
>>           cluster_off = parallels_allocate_host_clusters(bs, 
>> &alloc_size);
>>           if (cluster_off <= 0) {
>>               goto end;
> That is not enough. We should handle all-one and all-zeroes according
> to the spec and all-zeroes would be much more common.
Buffer for extensions contains zeroes before handling (it was allocated with
qemu_blockalign0). We skip all  all-zeroes l1 entries and the stay zeroed.

-- 
Best regards,
Alexander Ivanov