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

[Alexander Ivanov]

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 
---
  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, 
_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

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

[Denis V. Lunev]

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 
---
  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, _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.

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

[Alexander Ivanov]

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 
---
 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, _size);
 if (cluster_off <= 0) {
 goto end;
-- 
2.40.1