| Message ID | 1453740631-23259-1-git-send-email-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 25 Jan 2016 19:50:31 +0300 "Denis V. Lunev" <den@openvz.org> wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > NVDIMM for now is planned to use as a backing store for DAX filesystem > in the guest and thus this memory is excluded from guest memory management > and LRUs. > > In this case libvirt running QEMU along with configured balloon almost > immediately inflates balloon and effectively kill the guest as > qemu counts nvdimm as part of the ram. Isn't issue in ballooning impl. and not of pc-dimm/nvdimm, so make ballooning code to distinguish between kinds of memory rather than adding not related fields to PCDIMMDeviceClass why don't just move get_current_ram_size() into the sole user virtio-balloon.c and ignore NVDIMMs when counting ram in get_current_ram_size(), that would be much less intrusive patch. > > Counting dimm devices as part of the ram for ballooning was started from > commit 463756d03: > virtio-balloon: Fix balloon not working correctly when hotplug memory > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Xiao Guangrong <guangrong.xiao@linux.intel.com> > CC: "Michael S. Tsirkin" <mst@redhat.com> > CC: Igor Mammedov <imammedo@redhat.com> > CC: Eric Blake <eblake@redhat.com> > CC: Markus Armbruster <armbru@redhat.com> > --- > v2: > - some rewordings, thanks to Eric Blake > > hw/mem/nvdimm.c | 4 ++++ > hw/mem/pc-dimm.c | 7 ++++++- > include/hw/mem/pc-dimm.h | 1 + > qapi-schema.json | 5 ++++- > 4 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index 4fd397f..4f4d29a 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -27,9 +27,13 @@ > static void nvdimm_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > + PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc); > > /* nvdimm hotplug has not been supported yet. */ > dc->hotpluggable = false; > + > + /* ballooning is not supported */ > + ddc->in_ram = false; > } > > static TypeInfo nvdimm_info = { > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index d5cdab2..e0f869d 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -164,6 +164,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) > MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1); > PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1); > DeviceClass *dc = DEVICE_GET_CLASS(obj); > + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj); > PCDIMMDevice *dimm = PC_DIMM(obj); > > if (dev->id) { > @@ -172,6 +173,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) > } > di->hotplugged = dev->hotplugged; > di->hotpluggable = dc->hotpluggable; > + di->in_ram = ddc->in_ram; > di->addr = dimm->addr; > di->slot = dimm->slot; > di->node = dimm->node; > @@ -205,7 +207,9 @@ ram_addr_t get_current_ram_size(void) > if (value) { > switch (value->type) { > case MEMORY_DEVICE_INFO_KIND_DIMM: > - size += value->u.dimm->size; > + if (value->u.dimm->in_ram) { > + size += value->u.dimm->size; > + } > break; > default: > break; > @@ -444,6 +448,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) > dc->props = pc_dimm_properties; > dc->desc = "DIMM memory module"; > > + ddc->in_ram = true; > ddc->get_memory_region = pc_dimm_get_memory_region; > } > > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h > index d83bf30..3bcb505 100644 > --- a/include/hw/mem/pc-dimm.h > +++ b/include/hw/mem/pc-dimm.h > @@ -65,6 +65,7 @@ typedef struct PCDIMMDevice { > typedef struct PCDIMMDeviceClass { > /* private */ > DeviceClass parent_class; > + bool in_ram; > > /* public */ > MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm); > diff --git a/qapi-schema.json b/qapi-schema.json > index 2e960db..3cafa2b 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3922,6 +3922,8 @@ > # > # @hotpluggable: true if device if could be added/removed while machine is running > # > +# @in-ram: true if device is counted in current ram size (since 2.6) > +# > # Since: 2.1 > ## > { 'struct': 'PCDIMMDeviceInfo', > @@ -3932,7 +3934,8 @@ > 'node': 'int', > 'memdev': 'str', > 'hotplugged': 'bool', > - 'hotpluggable': 'bool' > + 'hotpluggable': 'bool', > + 'in-ram': 'bool' > } > } >
On 26.01.2016 13:38, Igor Mammedov wrote: > On Mon, 25 Jan 2016 19:50:31 +0300 > "Denis V. Lunev" <den@openvz.org> wrote: > >> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >> NVDIMM for now is planned to use as a backing store for DAX filesystem >> in the guest and thus this memory is excluded from guest memory management >> and LRUs. >> >> In this case libvirt running QEMU along with configured balloon almost >> immediately inflates balloon and effectively kill the guest as >> qemu counts nvdimm as part of the ram. > Isn't issue in ballooning impl. and not of pc-dimm/nvdimm, > so make ballooning code to distinguish between kinds of memory > rather than adding not related fields to PCDIMMDeviceClass > > why don't just move get_current_ram_size() into the sole user > virtio-balloon.c and ignore NVDIMMs when counting ram in > get_current_ram_size(), > that would be much less intrusive patch. Ok, will do. > >> Counting dimm devices as part of the ram for ballooning was started from >> commit 463756d03: >> virtio-balloon: Fix balloon not working correctly when hotplug memory >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com> >> CC: "Michael S. Tsirkin" <mst@redhat.com> >> CC: Igor Mammedov <imammedo@redhat.com> >> CC: Eric Blake <eblake@redhat.com> >> CC: Markus Armbruster <armbru@redhat.com> >> --- >> v2: >> - some rewordings, thanks to Eric Blake >> >> hw/mem/nvdimm.c | 4 ++++ >> hw/mem/pc-dimm.c | 7 ++++++- >> include/hw/mem/pc-dimm.h | 1 + >> qapi-schema.json | 5 ++++- >> 4 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c >> index 4fd397f..4f4d29a 100644 >> --- a/hw/mem/nvdimm.c >> +++ b/hw/mem/nvdimm.c >> @@ -27,9 +27,13 @@ >> static void nvdimm_class_init(ObjectClass *oc, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(oc); >> + PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc); >> >> /* nvdimm hotplug has not been supported yet. */ >> dc->hotpluggable = false; >> + >> + /* ballooning is not supported */ >> + ddc->in_ram = false; >> } >> >> static TypeInfo nvdimm_info = { >> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c >> index d5cdab2..e0f869d 100644 >> --- a/hw/mem/pc-dimm.c >> +++ b/hw/mem/pc-dimm.c >> @@ -164,6 +164,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) >> MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1); >> PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1); >> DeviceClass *dc = DEVICE_GET_CLASS(obj); >> + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj); >> PCDIMMDevice *dimm = PC_DIMM(obj); >> >> if (dev->id) { >> @@ -172,6 +173,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) >> } >> di->hotplugged = dev->hotplugged; >> di->hotpluggable = dc->hotpluggable; >> + di->in_ram = ddc->in_ram; >> di->addr = dimm->addr; >> di->slot = dimm->slot; >> di->node = dimm->node; >> @@ -205,7 +207,9 @@ ram_addr_t get_current_ram_size(void) >> if (value) { >> switch (value->type) { >> case MEMORY_DEVICE_INFO_KIND_DIMM: >> - size += value->u.dimm->size; >> + if (value->u.dimm->in_ram) { >> + size += value->u.dimm->size; >> + } >> break; >> default: >> break; >> @@ -444,6 +448,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) >> dc->props = pc_dimm_properties; >> dc->desc = "DIMM memory module"; >> >> + ddc->in_ram = true; >> ddc->get_memory_region = pc_dimm_get_memory_region; >> } >> >> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h >> index d83bf30..3bcb505 100644 >> --- a/include/hw/mem/pc-dimm.h >> +++ b/include/hw/mem/pc-dimm.h >> @@ -65,6 +65,7 @@ typedef struct PCDIMMDevice { >> typedef struct PCDIMMDeviceClass { >> /* private */ >> DeviceClass parent_class; >> + bool in_ram; >> >> /* public */ >> MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm); >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 2e960db..3cafa2b 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -3922,6 +3922,8 @@ >> # >> # @hotpluggable: true if device if could be added/removed while machine is running >> # >> +# @in-ram: true if device is counted in current ram size (since 2.6) >> +# >> # Since: 2.1 >> ## >> { 'struct': 'PCDIMMDeviceInfo', >> @@ -3932,7 +3934,8 @@ >> 'node': 'int', >> 'memdev': 'str', >> 'hotplugged': 'bool', >> - 'hotpluggable': 'bool' >> + 'hotpluggable': 'bool', >> + 'in-ram': 'bool' >> } >> } >>
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 4fd397f..4f4d29a 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -27,9 +27,13 @@ static void nvdimm_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); + PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc); /* nvdimm hotplug has not been supported yet. */ dc->hotpluggable = false; + + /* ballooning is not supported */ + ddc->in_ram = false; } static TypeInfo nvdimm_info = { diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index d5cdab2..e0f869d 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -164,6 +164,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1); PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1); DeviceClass *dc = DEVICE_GET_CLASS(obj); + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj); PCDIMMDevice *dimm = PC_DIMM(obj); if (dev->id) { @@ -172,6 +173,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) } di->hotplugged = dev->hotplugged; di->hotpluggable = dc->hotpluggable; + di->in_ram = ddc->in_ram; di->addr = dimm->addr; di->slot = dimm->slot; di->node = dimm->node; @@ -205,7 +207,9 @@ ram_addr_t get_current_ram_size(void) if (value) { switch (value->type) { case MEMORY_DEVICE_INFO_KIND_DIMM: - size += value->u.dimm->size; + if (value->u.dimm->in_ram) { + size += value->u.dimm->size; + } break; default: break; @@ -444,6 +448,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) dc->props = pc_dimm_properties; dc->desc = "DIMM memory module"; + ddc->in_ram = true; ddc->get_memory_region = pc_dimm_get_memory_region; } diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index d83bf30..3bcb505 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -65,6 +65,7 @@ typedef struct PCDIMMDevice { typedef struct PCDIMMDeviceClass { /* private */ DeviceClass parent_class; + bool in_ram; /* public */ MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm); diff --git a/qapi-schema.json b/qapi-schema.json index 2e960db..3cafa2b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3922,6 +3922,8 @@ # # @hotpluggable: true if device if could be added/removed while machine is running # +# @in-ram: true if device is counted in current ram size (since 2.6) +# # Since: 2.1 ## { 'struct': 'PCDIMMDeviceInfo', @@ -3932,7 +3934,8 @@ 'node': 'int', 'memdev': 'str', 'hotplugged': 'bool', - 'hotpluggable': 'bool' + 'hotpluggable': 'bool', + 'in-ram': 'bool' } }