> 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 <xi...@huawei.com> > >>>> Signed-off-by: Ming Yang yangmin...@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