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

[David Hildenbrand <david@redhat.com>]

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