On 16.12.22 07:22, Chenyi Qiang wrote:
vmem->bitmap indexes the memory region of the virtio-mem backend at a granularity of block_size. To calculate the index of target section offset, the block_size should be divided instead of the bitmap_size.
I'm curious, what's the user-visible effect and how did you identify this issue?
IIUC, we could end up our search for a plugged/unplugged block "too late", such that we miss to process blocks.
That would be the case if the bitmap_size < block_size, which should effectively always happen ...
unplug_all and migration would be affected, which is why a simple test case without a guest reboot/migration wouldn't run into it.
Fixes: 2044969f0b ("virtio-mem: Implement RamDiscardManager interface") Signed-off-by: Chenyi Qiang <chenyi.qi...@intel.com> --- hw/virtio/virtio-mem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index ed170def48..e19ee817fe 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -235,7 +235,7 @@ static int virtio_mem_for_each_plugged_section(const VirtIOMEM *vmem, uint64_t offset, size; int ret = 0;- first_bit = s->offset_within_region / vmem->bitmap_size;+ first_bit = s->offset_within_region / vmem->block_size; first_bit = find_next_bit(vmem->bitmap, vmem->bitmap_size, first_bit); while (first_bit < vmem->bitmap_size) { MemoryRegionSection tmp = *s; @@ -267,7 +267,7 @@ static int virtio_mem_for_each_unplugged_section(const VirtIOMEM *vmem, uint64_t offset, size; int ret = 0;- first_bit = s->offset_within_region / vmem->bitmap_size;+ first_bit = s->offset_within_region / vmem->block_size; first_bit = find_next_zero_bit(vmem->bitmap, vmem->bitmap_size, first_bit); while (first_bit < vmem->bitmap_size) { MemoryRegionSection tmp = *s;
Looks correct to me Reviewed-by: David Hildenbrand <da...@redhat.com> Thanks! -- Thanks, David / dhildenb
