qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
@ 2016-01-18 11:42 Denis V. Lunev
  2016-01-19  8:53 ` Xiao Guangrong
  2016-01-20  4:59 ` Eric Blake
  0 siblings, 2 replies; 6+ messages in thread
From: Denis V. Lunev @ 2016-01-18 11:42 UTC (permalink / raw)
  Cc: Vladimir Sementsov-Ogievskiy, Xiao Guangrong, Michael S. Tsirkin,
	qemu-devel, Markus Armbruster, Stefan Hajnoczi, Igor Mammedov,
	Denis V. Lunev

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 ballon 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
patch
 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>
---
The patch is submitted start a discussion. It may be technically correct,
but for us the situation is a bit shady.

 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 b3038b2..613b4d5 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 if should be 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'
           }
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
  2016-01-18 11:42 [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon Denis V. Lunev
@ 2016-01-19  8:53 ` Xiao Guangrong
  2016-01-20 11:36   ` Vladimir Sementsov-Ogievskiy
  2016-01-20  4:59 ` Eric Blake
  1 sibling, 1 reply; 6+ messages in thread
From: Xiao Guangrong @ 2016-01-19  8:53 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Vladimir Sementsov-Ogievskiy, Michael S. Tsirkin, qemu-devel,
	Markus Armbruster, Stefan Hajnoczi, Igor Mammedov



On 01/18/2016 07:42 PM, Denis V. Lunev 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 ballon almost
> immediately inflates balloon and effectively kill the guest as
> qemu counts nvdimm as part of the ram.
>

It looks good me.

However, it is not related to this patch, why not use the 'total memory' reported
by guest instead? It is more precise as a) BIOS and other components will occupy
available memory and b) guest may limit the memory size it can use...

> Counting dimm devices as part of the ram for ballooning was started from
> patch
>   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>
> ---
> The patch is submitted start a discussion. It may be technically correct,
> but for us the situation is a bit shady.
>
>   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;
> +                }

Can we use "object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)" to filter out
NVDIMM device?

>                   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 b3038b2..613b4d5 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 if should be 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'

What is it used for?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
  2016-01-18 11:42 [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon Denis V. Lunev
  2016-01-19  8:53 ` Xiao Guangrong
@ 2016-01-20  4:59 ` Eric Blake
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Blake @ 2016-01-20  4:59 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: Vladimir Sementsov-Ogievskiy, Xiao Guangrong, Michael S. Tsirkin,
	qemu-devel, Markus Armbruster, Stefan Hajnoczi, Igor Mammedov

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On 01/18/2016 04:42 AM, Denis V. Lunev 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 ballon almost

s/ballon/balloon/

> 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
> patch
>  virtio-balloon: Fix balloon not working correctly when hotplug memory

Useful to mention the commit id: 463756d03

> 
> 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>

I'm just doing an interface review.

> ---
> The patch is submitted start a discussion. It may be technically correct,
> but for us the situation is a bit shady.
> 

> +++ 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 if should be counted in current ram size (since 2.6)

Output-only, so you are okay adding a new non-optional field.  However,
is "should be counted" the right wording, or would it be better as "is
counted", given that the user doesn't have any way to change whether a
device gets counted or not, but is merely learning the state previously
chosen by the device.

As for whether it makes sense technically, I'll leave that to discussion.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
  2016-01-19  8:53 ` Xiao Guangrong
@ 2016-01-20 11:36   ` Vladimir Sementsov-Ogievskiy
  2016-01-20 15:41     ` Xiao Guangrong
  2016-01-25 15:24     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 2 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2016-01-20 11:36 UTC (permalink / raw)
  To: Xiao Guangrong, Denis V. Lunev
  Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel,
	Stefan Hajnoczi, Igor Mammedov

On 19.01.2016 11:53, Xiao Guangrong wrote:
>
>
> On 01/18/2016 07:42 PM, Denis V. Lunev 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 ballon almost
>> immediately inflates balloon and effectively kill the guest as
>> qemu counts nvdimm as part of the ram.
>>
>
> It looks good me.
>
> However, it is not related to this patch, why not use the 'total 
> memory' reported
> by guest instead? It is more precise as a) BIOS and other components 
> will occupy
> available memory and b) guest may limit the memory size it can use...

What do you mean? fix virtio_balloon_set_config instead of 
get_current_ram_size ? Or what?

>
>> Counting dimm devices as part of the ram for ballooning was started from
>> patch
>>   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>
>> ---
>> The patch is submitted start a discussion. It may be technically 
>> correct,
>> but for us the situation is a bit shady.
>>
>>   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 @@ ch(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;
>> +                }
>
> Can we use "object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)" to filter out
> NVDIMM device?

Hm, this will be ok too. and simpler.. If it ok for you, let's just 
check it here.

>
>>                   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 b3038b2..613b4d5 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 if should be 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'
>
> What is it used for?

only for check in ram_addr_t get_current_ram_size()


-- 
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
  2016-01-20 11:36   ` Vladimir Sementsov-Ogievskiy
@ 2016-01-20 15:41     ` Xiao Guangrong
  2016-01-25 15:24     ` Vladimir Sementsov-Ogievskiy
  1 sibling, 0 replies; 6+ messages in thread
From: Xiao Guangrong @ 2016-01-20 15:41 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, Denis V. Lunev
  Cc: qemu-devel, Igor Mammedov, Markus Armbruster, Stefan Hajnoczi,
	Michael S. Tsirkin



On 01/20/2016 07:36 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 19.01.2016 11:53, Xiao Guangrong wrote:
>>
>>
>> On 01/18/2016 07:42 PM, Denis V. Lunev 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 ballon almost
>>> immediately inflates balloon and effectively kill the guest as
>>> qemu counts nvdimm as part of the ram.
>>>
>>
>> It looks good me.
>>
>> However, it is not related to this patch, why not use the 'total memory' reported
>> by guest instead? It is more precise as a) BIOS and other components will occupy
>> available memory and b) guest may limit the memory size it can use...
>
> What do you mean? fix virtio_balloon_set_config instead of get_current_ram_size ? Or what?

No. I just thrown the question and hoped some one can figure it out, it is not about
this patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon
  2016-01-20 11:36   ` Vladimir Sementsov-Ogievskiy
  2016-01-20 15:41     ` Xiao Guangrong
@ 2016-01-25 15:24     ` Vladimir Sementsov-Ogievskiy
  1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2016-01-25 15:24 UTC (permalink / raw)
  To: Xiao Guangrong, Denis V. Lunev
  Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel,
	Stefan Hajnoczi, Igor Mammedov

On 20.01.2016 14:36, Vladimir Sementsov-Ogievskiy wrote:
> On 19.01.2016 11:53, Xiao Guangrong wrote:
>>
>>
>> On 01/18/2016 07:42 PM, Denis V. Lunev 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 ballon almost
>>> immediately inflates balloon and effectively kill the guest as
>>> qemu counts nvdimm as part of the ram.
>>>
>>
>> It looks good me.
>>
>> However, it is not related to this patch, why not use the 'total 
>> memory' reported
>> by guest instead? It is more precise as a) BIOS and other components 
>> will occupy
>> available memory and b) guest may limit the memory size it can use...
>
> What do you mean? fix virtio_balloon_set_config instead of 
> get_current_ram_size ? Or what?
>
>>
>>> Counting dimm devices as part of the ram for ballooning was started 
>>> from
>>> patch
>>>   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>
>>> ---
>>> The patch is submitted start a discussion. It may be technically 
>>> correct,
>>> but for us the situation is a bit shady.
>>>
>>>   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 @@ ch(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;
>>> +                }
>>
>> Can we use "object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)" to filter out
>> NVDIMM device?
>
> Hm, this will be ok too. and simpler.. If it ok for you, let's just 
> check it here.


No, we cant, there is no access to dev here. only info. But, if not use 
qmp_pc_dimm_device_list, with it dev -> info transition, and write 
simple pc_dimm_device_list it would be possible.

>
>>
>>>                   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 b3038b2..613b4d5 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 if should be 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'
>>
>> What is it used for?
>
> only for check in ram_addr_t get_current_ram_size()
>
>


-- 
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-01-25 15:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-18 11:42 [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon Denis V. Lunev
2016-01-19  8:53 ` Xiao Guangrong
2016-01-20 11:36   ` Vladimir Sementsov-Ogievskiy
2016-01-20 15:41     ` Xiao Guangrong
2016-01-25 15:24     ` Vladimir Sementsov-Ogievskiy
2016-01-20  4:59 ` Eric Blake

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).