| Message ID | 1454660341-45244-6-git-send-email-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 5 Feb 2016 11:19:01 +0300 Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > For now there are only two dimm's: pc-dimm and nvdimm. This patch is > actually needed to disable ballooning on nvdimm. But, to avoid future > bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if > someone adds new dimm which should be balloon-able, then this ability > should be explicitly specified here. > > Why ballooning for nvdimm should be disabled for now: > > 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. > > 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> > --- > hw/virtio/virtio-balloon.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index b9c1964..0415e07 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -18,6 +18,7 @@ > #include "qemu-common.h" > #include "hw/virtio/virtio.h" > #include "hw/i386/pc.h" > +#include "hw/mem/nvdimm.h" Is this include still needed? > #include "cpu.h" > #include "sysemu/balloon.h" > #include "hw/virtio/virtio-balloon.h" > @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void) > pc_dimm_build_list(qdev_get_machine(), &list); > for (item = list; item; item = g_slist_next(item)) { > PCDIMMDevice *dimm = item->data; > - size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL); > + if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) { > + size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, > + NULL); > + } > } > g_slist_free(list); >
On 05.02.2016 15:58, Igor Mammedov wrote: > On Fri, 5 Feb 2016 11:19:01 +0300 > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > >> For now there are only two dimm's: pc-dimm and nvdimm. This patch is >> actually needed to disable ballooning on nvdimm. But, to avoid future >> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if >> someone adds new dimm which should be balloon-able, then this ability >> should be explicitly specified here. >> >> Why ballooning for nvdimm should be disabled for now: >> >> 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. >> >> 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> >> --- >> hw/virtio/virtio-balloon.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c >> index b9c1964..0415e07 100644 >> --- a/hw/virtio/virtio-balloon.c >> +++ b/hw/virtio/virtio-balloon.c >> @@ -18,6 +18,7 @@ >> #include "qemu-common.h" >> #include "hw/virtio/virtio.h" >> #include "hw/i386/pc.h" >> +#include "hw/mem/nvdimm.h" > Is this include still needed? Yes, for TYPE_PC_DIMM > >> #include "cpu.h" >> #include "sysemu/balloon.h" >> #include "hw/virtio/virtio-balloon.h" >> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void) >> pc_dimm_build_list(qdev_get_machine(), &list); >> for (item = list; item; item = g_slist_next(item)) { >> PCDIMMDevice *dimm = item->data; >> - size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL); >> + if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) { >> + size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, >> + NULL); >> + } >> } >> g_slist_free(list); >>
On Fri, 5 Feb 2016 17:09:58 +0300 Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > On 05.02.2016 15:58, Igor Mammedov wrote: > > On Fri, 5 Feb 2016 11:19:01 +0300 > > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > > > >> For now there are only two dimm's: pc-dimm and nvdimm. This patch is > >> actually needed to disable ballooning on nvdimm. But, to avoid future > >> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if > >> someone adds new dimm which should be balloon-able, then this ability > >> should be explicitly specified here. > >> > >> Why ballooning for nvdimm should be disabled for now: > >> > >> 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. > >> > >> 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> > >> --- > >> hw/virtio/virtio-balloon.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > >> index b9c1964..0415e07 100644 > >> --- a/hw/virtio/virtio-balloon.c > >> +++ b/hw/virtio/virtio-balloon.c > >> @@ -18,6 +18,7 @@ > >> #include "qemu-common.h" > >> #include "hw/virtio/virtio.h" > >> #include "hw/i386/pc.h" > >> +#include "hw/mem/nvdimm.h" > > Is this include still needed? > > Yes, for TYPE_PC_DIMM then should it be pc-dimm.h > > > > >> #include "cpu.h" > >> #include "sysemu/balloon.h" > >> #include "hw/virtio/virtio-balloon.h" > >> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void) > >> pc_dimm_build_list(qdev_get_machine(), &list); > >> for (item = list; item; item = g_slist_next(item)) { > >> PCDIMMDevice *dimm = item->data; > >> - size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL); > >> + if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) { > >> + size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, > >> + NULL); > >> + } > >> } > >> g_slist_free(list); > >> > >
On 05.02.2016 17:55, Igor Mammedov wrote: > On Fri, 5 Feb 2016 17:09:58 +0300 > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > >> On 05.02.2016 15:58, Igor Mammedov wrote: >>> On Fri, 5 Feb 2016 11:19:01 +0300 >>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: >>> >>>> For now there are only two dimm's: pc-dimm and nvdimm. This patch is >>>> actually needed to disable ballooning on nvdimm. But, to avoid future >>>> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if >>>> someone adds new dimm which should be balloon-able, then this ability >>>> should be explicitly specified here. >>>> >>>> Why ballooning for nvdimm should be disabled for now: >>>> >>>> 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. >>>> >>>> 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> >>>> --- >>>> hw/virtio/virtio-balloon.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c >>>> index b9c1964..0415e07 100644 >>>> --- a/hw/virtio/virtio-balloon.c >>>> +++ b/hw/virtio/virtio-balloon.c >>>> @@ -18,6 +18,7 @@ >>>> #include "qemu-common.h" >>>> #include "hw/virtio/virtio.h" >>>> #include "hw/i386/pc.h" >>>> +#include "hw/mem/nvdimm.h" >>> Is this include still needed? >> Yes, for TYPE_PC_DIMM > then should it be pc-dimm.h oh, yes, I'm wrong. > >>> >>>> #include "cpu.h" >>>> #include "sysemu/balloon.h" >>>> #include "hw/virtio/virtio-balloon.h" >>>> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void) >>>> pc_dimm_build_list(qdev_get_machine(), &list); >>>> for (item = list; item; item = g_slist_next(item)) { >>>> PCDIMMDevice *dimm = item->data; >>>> - size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL); >>>> + if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) { >>>> + size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, >>>> + NULL); >>>> + } >>>> } >>>> g_slist_free(list); >>>> >>
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index b9c1964..0415e07 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -18,6 +18,7 @@ #include "qemu-common.h" #include "hw/virtio/virtio.h" #include "hw/i386/pc.h" +#include "hw/mem/nvdimm.h" #include "cpu.h" #include "sysemu/balloon.h" #include "hw/virtio/virtio-balloon.h" @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void) pc_dimm_build_list(qdev_get_machine(), &list); for (item = list; item; item = g_slist_next(item)) { PCDIMMDevice *dimm = item->data; - size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL); + if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) { + size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, + NULL); + } } g_slist_free(list);