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

[Yangming]

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

Thanks,

XiQi