From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35451) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLr3x-0002mc-CJ for qemu-devel@nongnu.org; Wed, 20 Jan 2016 06:36:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLr3u-0007NV-52 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 06:36:33 -0500 Received: from mx2.parallels.com ([199.115.105.18]:50073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLr3t-0007Kz-S1 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 06:36:30 -0500 Message-ID: <569F712E.3030106@virtuozzo.com> Date: Wed, 20 Jan 2016 14:36:14 +0300 From: Vladimir Sementsov-Ogievskiy MIME-Version: 1.0 References: <1453117322-30191-1-git-send-email-den@openvz.org> <569DF99C.3040700@linux.intel.com> In-Reply-To: <569DF99C.3040700@linux.intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] nvdimm: disable balloon List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guangrong , "Denis V. Lunev" Cc: "Michael S. Tsirkin" , Markus Armbruster , qemu-devel@nongnu.org, 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 >> >> 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 >> Signed-off-by: Denis V. Lunev >> CC: Stefan Hajnoczi >> CC: Xiao Guangrong >> CC: "Michael S. Tsirkin" >> CC: Igor Mammedov >> CC: Eric Blake >> CC: Markus Armbruster >> --- >> 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.