| Message ID | 264b6cc6a74945c3b5214fa4e8f099fe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-balloon: optimize the virtio-balloon on the ARM platform. | expand |
On Tue, Feb 28, 2023 at 03:26:56AM +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 > > > --- > > > hw/mem/pc-dimm.c | 2 ++ > > > hw/virtio/virtio-balloon.c | 33 +++++---------------------------- > > > include/hw/boards.h | 1 + > > > 3 files changed, 8 insertions(+), 28 deletions(-) > > > > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index > > > 50ef83215c..192fc7922c 100644 > > > --- a/hw/mem/pc-dimm.c > > > +++ b/hw/mem/pc-dimm.c > > > @@ -81,6 +81,7 @@ void pc_dimm_plug(PCDIMMDevice *dimm, > > MachineState > > > *machine) > > > > > > memory_device_plug(MEMORY_DEVICE(dimm), machine); > > > vmstate_register_ram(vmstate_mr, DEVICE(dimm)); > > > + machine->device_memory->dimm_size += vmstate_mr->size; > > > } > > > > > > void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) > > @@ > > > -90,6 +91,7 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, > > MachineState > > > *machine) > > > > > > memory_device_unplug(MEMORY_DEVICE(dimm), machine); > > > vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); > > > + machine->device_memory->dimm_size -= vmstate_mr->size; > > > } > > > > Ahh, missed that my previous comment was not addressed: we only want to > > track "real" DIMMs, not NVDIMMs. > > > > -- > > Thanks, > > > > David / dhildenb > > 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 > --- > hw/mem/pc-dimm.c | 8 ++++++++ > hw/virtio/virtio-balloon.c | 33 +++++---------------------------- > include/hw/boards.h | 1 + > 3 files changed, 14 insertions(+), 28 deletions(-) > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index 50ef83215c..2107615016 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)); > + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); > + if (!is_nvdimm) { > + machine->device_memory->dimm_size += vmstate_mr->size; > + } > } > > void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) > @@ -90,6 +94,10 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) > > memory_device_unplug(MEMORY_DEVICE(dimm), machine); > vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); > + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); > + if (!is_nvdimm) { > + machine->device_memory->dimm_size -= vmstate_mr->size; > + } > } > add comments here explaining why are nvdimms excluded? > static int pc_dimm_slot2bitmap(Object *obj, void *opaque) > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 746f07c4d2..80bbb59132 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -729,37 +729,14 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) > memcpy(config_data, &config, virtio_balloon_config_size(dev)); > } > > -static int build_dimm_list(Object *obj, void *opaque) > -{ > - GSList **list = opaque; > - > - if (object_dynamic_cast(obj, TYPE_PC_DIMM)) { > - DeviceState *dev = DEVICE(obj); > - if (dev->realized) { /* only realized DIMMs matter */ > - *list = g_slist_prepend(*list, dev); > - } > - } > - > - object_child_foreach(obj, build_dimm_list, opaque); > - return 0; > -} > - > static ram_addr_t get_current_ram_size(void) > { > - GSList *list = NULL, *item; > - ram_addr_t size = current_machine->ram_size; > - > - build_dimm_list(qdev_get_machine(), &list); > - for (item = list; item; item = g_slist_next(item)) { > - Object *obj = OBJECT(item->data); > - if (!strcmp(object_get_typename(obj), TYPE_PC_DIMM)) { > - size += object_property_get_int(obj, PC_DIMM_SIZE_PROP, > - &error_abort); > - } > + MachineState *machine = MACHINE(qdev_get_machine()); > + if (machine->device_memory != NULL) { just if (machine->device_memory) is equivalent and shorter > + return machine->ram_size + machine->device_memory->dimm_size; > + } else { > + return machine->ram_size; > } > - g_slist_free(list); > - > - return size; > } > > static bool virtio_balloon_page_poison_support(void *opaque) > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 6fbbfd56c8..551b4b419e 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -296,6 +296,7 @@ struct MachineClass { > typedef struct DeviceMemoryState { > hwaddr base; > MemoryRegion mr; > + ram_addr_t dimm_size; add a comment explaining what this is? > } DeviceMemoryState; > > /** > -- > 2.33.0 >
On 28.02.23 09:56, Michael S. Tsirkin wrote: > On Tue, Feb 28, 2023 at 03:26:56AM +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 >>>> --- >>>> hw/mem/pc-dimm.c | 2 ++ >>>> hw/virtio/virtio-balloon.c | 33 +++++---------------------------- >>>> include/hw/boards.h | 1 + >>>> 3 files changed, 8 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index >>>> 50ef83215c..192fc7922c 100644 >>>> --- a/hw/mem/pc-dimm.c >>>> +++ b/hw/mem/pc-dimm.c >>>> @@ -81,6 +81,7 @@ void pc_dimm_plug(PCDIMMDevice *dimm, >>> MachineState >>>> *machine) >>>> >>>> memory_device_plug(MEMORY_DEVICE(dimm), machine); >>>> vmstate_register_ram(vmstate_mr, DEVICE(dimm)); >>>> + machine->device_memory->dimm_size += vmstate_mr->size; >>>> } >>>> >>>> void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) >>> @@ >>>> -90,6 +91,7 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, >>> MachineState >>>> *machine) >>>> >>>> memory_device_unplug(MEMORY_DEVICE(dimm), machine); >>>> vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); >>>> + machine->device_memory->dimm_size -= vmstate_mr->size; >>>> } >>> >>> Ahh, missed that my previous comment was not addressed: we only want to >>> track "real" DIMMs, not NVDIMMs. >>> >>> -- >>> Thanks, >>> >>> David / dhildenb >> >> 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 >> --- >> hw/mem/pc-dimm.c | 8 ++++++++ >> hw/virtio/virtio-balloon.c | 33 +++++---------------------------- >> include/hw/boards.h | 1 + >> 3 files changed, 14 insertions(+), 28 deletions(-) >> >> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c >> index 50ef83215c..2107615016 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)); >> + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); >> + if (!is_nvdimm) { >> + machine->device_memory->dimm_size += vmstate_mr->size; >> + } >> } >> >> void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) >> @@ -90,6 +94,10 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) >> >> memory_device_unplug(MEMORY_DEVICE(dimm), machine); >> vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); >> + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); >> + if (!is_nvdimm) { >> + machine->device_memory->dimm_size -= vmstate_mr->size; >> + } >> } >> > > add comments here explaining why are nvdimms excluded? > I really prefer to avoid mixing declaration and initialization where it an be avoided. Further, the local variable can be easily avoided completely. if (!object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM)) With that and with the other comments addressed Acked-by: David Hildenbrand <david@redhat.com>
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 50ef83215c..2107615016 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)); + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); + if (!is_nvdimm) { + machine->device_memory->dimm_size += vmstate_mr->size; + } } void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) @@ -90,6 +94,10 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) memory_device_unplug(MEMORY_DEVICE(dimm), machine); vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); + bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM); + if (!is_nvdimm) { + machine->device_memory->dimm_size -= vmstate_mr->size; + } } static int pc_dimm_slot2bitmap(Object *obj, void *opaque) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 746f07c4d2..80bbb59132 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -729,37 +729,14 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) memcpy(config_data, &config, virtio_balloon_config_size(dev)); } -static int build_dimm_list(Object *obj, void *opaque) -{ - GSList **list = opaque; - - if (object_dynamic_cast(obj, TYPE_PC_DIMM)) { - DeviceState *dev = DEVICE(obj); - if (dev->realized) { /* only realized DIMMs matter */ - *list = g_slist_prepend(*list, dev); - } - } - - object_child_foreach(obj, build_dimm_list, opaque); - return 0; -} - static ram_addr_t get_current_ram_size(void) { - GSList *list = NULL, *item; - ram_addr_t size = current_machine->ram_size; - - build_dimm_list(qdev_get_machine(), &list); - for (item = list; item; item = g_slist_next(item)) { - Object *obj = OBJECT(item->data); - if (!strcmp(object_get_typename(obj), TYPE_PC_DIMM)) { - size += object_property_get_int(obj, PC_DIMM_SIZE_PROP, - &error_abort); - } + MachineState *machine = MACHINE(qdev_get_machine()); + if (machine->device_memory != NULL) { + return machine->ram_size + machine->device_memory->dimm_size; + } else { + return machine->ram_size; } - g_slist_free(list); - - return size; } static bool virtio_balloon_page_poison_support(void *opaque) diff --git a/include/hw/boards.h b/include/hw/boards.h index 6fbbfd56c8..551b4b419e 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -296,6 +296,7 @@ struct MachineClass { typedef struct DeviceMemoryState { hwaddr base; MemoryRegion mr; + ram_addr_t dimm_size; } DeviceMemoryState; /**