Reply: [PATCH v2] virtio-balloon: optimize the virtio-balloon on the ARM platform

[Yangming via <qemu-devel@nongnu.org>]

> On 09.03.23 07:14, Yangming wrote:
> >> On 08.03.23 01:42, Michael S. Tsirkin wrote:
> >>> On Wed, Mar 01, 2023 at 06:38:13AM +0000, Yangming wrote:
> >>>> Optimize the virtio-balloon feature on the ARM platform by adding a
> >>>> variable to keep track of the current hot-plugged pc-dimm size,
> >>>> instead of traversing the virtual machine's memory modules to count
> >>>> the current RAM size during the balloon inflation or deflation
> >>>> process. This variable can be updated only when plugging or
> >>>> unplugging the device, which will result in an increase of
> >>>> approximately 60% efficiency of balloon process on the ARM platform.
> >>>>
> >>>> We tested the total amount of time required for the balloon
> >>>> inflation
> >> process on ARM:
> >>>> inflate the balloon to 64GB of a 128GB guest under stress.
> >>>> Before: 102 seconds
> >>>> After: 42 seconds
> >>>>
> >>>> Signed-off-by: Qi Xi <xiqi2@huawei.com>
> >>>> Signed-off-by: Ming Yang yangming73@huawei.com
> >>>> ---
> >>>> Refactor the code by adding comments and removing unnecessary
> code.
> >>>>
> >>>>    hw/mem/pc-dimm.c           |  7 +++++++
> >>>>    hw/virtio/virtio-balloon.c | 33 +++++----------------------------
> >>>>    include/hw/boards.h        |  2 ++
> >>>>    3 files changed, 14 insertions(+), 28 deletions(-)
> >>>>
> >>>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index
> >>>> 50ef83215c..3f2734a267 100644
> >>>> --- a/hw/mem/pc-dimm.c
> >>>> +++ b/hw/mem/pc-dimm.c
> >>>> @@ -81,6 +81,10 @@ void pc_dimm_plug(PCDIMMDevice *dimm,
> >> MachineState
> >>>> *machine)
> >>>>
> >>>>        memory_device_plug(MEMORY_DEVICE(dimm), machine);
> >>>>        vmstate_register_ram(vmstate_mr, DEVICE(dimm));
> >>>> +    /* count only "real" DIMMs, not NVDIMMs */
> >>>> +    if (!object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM)) {
> >>>> +        machine->device_memory->dimm_size += vmstate_mr->size;
> >>>> +    }
> >>>>    }
> >>>>
> >>>>    void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState
> *machine)
> >>>
> >>> vmstate_mr->size is Int128 you are not supposed to do math on it.
> >>>
> >>> And generally poking at this struct is a bad idea.
> >>>
> >>> I think memory_region_size will do what you want but not 100% sure.
> >>> Maybe you need to look at the flatview ...
> >>
> >> Good point, we should use memory_region_size().
> >>
> >> --
> >> Thanks,
> >>
> >> David / dhildenb
> >
> > Thanks for the suggestion. The problem will be fixed in the upcoming third
> version by using 'memory_region_size()'.
> >
> > By the way, we found that the size of the object is aligned with
> 'qemu_host_page_size' before allocating the memory (see details in
> 'qemu_ram_alloc_internal()' from softmmu/physmem.c). This means that
> the actual allocated memory may differ from the size defined in the Object.
> As a result, in 'get_current_ram_size()', the original method of counting hot-
> plugged memory from the Object may not be accurate. Now, we count the
> size from the memory region, which is a proper way to get the actual size of
> memory allocated.
> 
> You'd have to have a memory backend size that is not aligned to the host
> page size.
> 
> qemu-system-x86_64 -nographic -nodefaults -object memory-backend-
> ram,id=tmp,size=1K
> 
> While that seems to be possible, it's something very odd to happen (I cannot
> think of a reasonable use case).
> 
> 
> 
> --
> Thanks,
> 
> David / dhildenb

Yes, I agree. Besides, a newer version[PATCH v3] has been sent. I am wondering could you please have a look at that？

Best
Qi