| Message ID | 20231228101232.372142-14-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Add full dirty bitmap support | expand |
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.
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.
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;
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(-)