| Message ID | 988dc97c3b3e3c89066b3b3dc8c17b3fccc3c816.1402299637.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 9 Jun 2014 18:25:23 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > backends/Makefile.objs | 1 + > backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 108 insertions(+) > create mode 100644 backends/hostmem-file.c > > diff --git a/backends/Makefile.objs b/backends/Makefile.objs > index 7fb7acd..506a46c 100644 > --- a/backends/Makefile.objs > +++ b/backends/Makefile.objs > @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS) > common-obj-$(CONFIG_TPM) += tpm.o > > common-obj-y += hostmem.o hostmem-ram.o > +common-obj-$(CONFIG_LINUX) += hostmem-file.o > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > new file mode 100644 > index 0000000..b8df933 > --- /dev/null > +++ b/backends/hostmem-file.c > @@ -0,0 +1,107 @@ > +/* > + * QEMU Host Memory Backend for hugetlbfs > + * > + * Copyright (C) 2013 Red Hat Inc > + * > + * Authors: > + * Paolo Bonzini <pbonzini@redhat.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > +#include "sysemu/hostmem.h" > +#include "qom/object_interfaces.h" > + > +/* hostmem-file.c */ > +/** > + * @TYPE_MEMORY_BACKEND_FILE: > + * name of backend that uses mmap on a file descriptor > + */ > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" how about naming it after what it really is? "memory-backend-hugepage" Later we could split it into generic superclass mmap-ed "memory-backend-file" and have TPH specific code moved into this backend. > + > +#define MEMORY_BACKEND_FILE(obj) \ > + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE) > + > +typedef struct HostMemoryBackendFile HostMemoryBackendFile; > + > +struct HostMemoryBackendFile { > + HostMemoryBackend parent_obj; > + char *mem_path; > +}; > + > +static void > +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > +{ > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); > + > + if (!backend->size) { > + error_setg(errp, "can't create backend with size 0"); > + return; > + } > + if (!fb->mem_path) { > + error_setg(errp, "mem-path property not set"); > + return; > + } > +#ifndef CONFIG_LINUX > + error_setg(errp, "-mem-path not supported on this host"); Is it possible to not compile this backend on non linux host at all, instead of ifdefs. > +#else > + if (!memory_region_size(&backend->mr)) { > + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > + object_get_canonical_path(OBJECT(backend)), > + backend->size, > + fb->mem_path, errp); > + } > +#endif > +} > + > +static void > +file_backend_class_init(ObjectClass *oc, void *data) > +{ > + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc); > + > + bc->alloc = file_backend_memory_alloc; > +} > + > +static char *get_mem_path(Object *o, Error **errp) > +{ > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > + > + return g_strdup(fb->mem_path); > +} > + > +static void set_mem_path(Object *o, const char *str, Error **errp) > +{ > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > + > + if (memory_region_size(&backend->mr)) { > + error_setg(errp, "cannot change property value"); > + return; > + } > + if (fb->mem_path) { > + g_free(fb->mem_path); > + } > + fb->mem_path = g_strdup(str); > +} > + > +static void > +file_backend_instance_init(Object *o) > +{ > + object_property_add_str(o, "mem-path", get_mem_path, > + set_mem_path, NULL); s/"mem-path"/"path"/ > +} > + > +static const TypeInfo file_backend_info = { > + .name = TYPE_MEMORY_BACKEND_FILE, > + .parent = TYPE_MEMORY_BACKEND, > + .class_init = file_backend_class_init, > + .instance_init = file_backend_instance_init, > + .instance_size = sizeof(HostMemoryBackendFile), > +}; > + > +static void register_types(void) > +{ > + type_register_static(&file_backend_info); > +} > + > +type_init(register_types); > -- > 1.9.3 >
On Mon, Jun 09, 2014 at 01:32:46PM +0200, Igor Mammedov wrote: > On Mon, 9 Jun 2014 18:25:23 +0800 > Hu Tao <hutao@cn.fujitsu.com> wrote: > > > From: Paolo Bonzini <pbonzini@redhat.com> > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > backends/Makefile.objs | 1 + > > backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 108 insertions(+) > > create mode 100644 backends/hostmem-file.c > > > > diff --git a/backends/Makefile.objs b/backends/Makefile.objs > > index 7fb7acd..506a46c 100644 > > --- a/backends/Makefile.objs > > +++ b/backends/Makefile.objs > > @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS) > > common-obj-$(CONFIG_TPM) += tpm.o > > > > common-obj-y += hostmem.o hostmem-ram.o > > +common-obj-$(CONFIG_LINUX) += hostmem-file.o > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > > new file mode 100644 > > index 0000000..b8df933 > > --- /dev/null > > +++ b/backends/hostmem-file.c > > @@ -0,0 +1,107 @@ > > +/* > > + * QEMU Host Memory Backend for hugetlbfs > > + * > > + * Copyright (C) 2013 Red Hat Inc > > + * > > + * Authors: > > + * Paolo Bonzini <pbonzini@redhat.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > + * See the COPYING file in the top-level directory. > > + */ > > +#include "sysemu/hostmem.h" > > +#include "qom/object_interfaces.h" > > + > > +/* hostmem-file.c */ > > +/** > > + * @TYPE_MEMORY_BACKEND_FILE: > > + * name of backend that uses mmap on a file descriptor > > + */ > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > how about naming it after what it really is? "memory-backend-hugepage" > Later we could split it into generic superclass mmap-ed "memory-backend-file" > and have TPH specific code moved into this backend. What does this last sentence mean? THP is transparent huge pages, right? > > + > > +#define MEMORY_BACKEND_FILE(obj) \ > > + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE) > > + > > +typedef struct HostMemoryBackendFile HostMemoryBackendFile; > > + > > +struct HostMemoryBackendFile { > > + HostMemoryBackend parent_obj; > > + char *mem_path; > > +}; > > + > > +static void > > +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > > +{ > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); > > + > > + if (!backend->size) { > > + error_setg(errp, "can't create backend with size 0"); > > + return; > > + } > > + if (!fb->mem_path) { > > + error_setg(errp, "mem-path property not set"); > > + return; > > + } > > +#ifndef CONFIG_LINUX > > + error_setg(errp, "-mem-path not supported on this host"); > Is it possible to not compile this backend on non linux host at all, instead > of ifdefs. > > > +#else > > + if (!memory_region_size(&backend->mr)) { > > + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > > + object_get_canonical_path(OBJECT(backend)), > > + backend->size, > > + fb->mem_path, errp); > > + } > > +#endif > > +} > > + > > +static void > > +file_backend_class_init(ObjectClass *oc, void *data) > > +{ > > + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc); > > + > > + bc->alloc = file_backend_memory_alloc; > > +} > > + > > +static char *get_mem_path(Object *o, Error **errp) > > +{ > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > + > > + return g_strdup(fb->mem_path); > > +} > > + > > +static void set_mem_path(Object *o, const char *str, Error **errp) > > +{ > > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > + > > + if (memory_region_size(&backend->mr)) { > > + error_setg(errp, "cannot change property value"); > > + return; > > + } > > + if (fb->mem_path) { > > + g_free(fb->mem_path); > > + } > > + fb->mem_path = g_strdup(str); > > +} > > + > > +static void > > +file_backend_instance_init(Object *o) > > +{ > > + object_property_add_str(o, "mem-path", get_mem_path, > > + set_mem_path, NULL); > s/"mem-path"/"path"/ > > > > +} > > + > > +static const TypeInfo file_backend_info = { > > + .name = TYPE_MEMORY_BACKEND_FILE, > > + .parent = TYPE_MEMORY_BACKEND, > > + .class_init = file_backend_class_init, > > + .instance_init = file_backend_instance_init, > > + .instance_size = sizeof(HostMemoryBackendFile), > > +}; > > + > > +static void register_types(void) > > +{ > > + type_register_static(&file_backend_info); > > +} > > + > > +type_init(register_types); > > -- > > 1.9.3 > > > > > -- > Regards, > Igor
On Mon, 9 Jun 2014 14:35:53 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Jun 09, 2014 at 01:32:46PM +0200, Igor Mammedov wrote: > > On Mon, 9 Jun 2014 18:25:23 +0800 > > Hu Tao <hutao@cn.fujitsu.com> wrote: > > > > > From: Paolo Bonzini <pbonzini@redhat.com> > > > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > > --- > > > backends/Makefile.objs | 1 + > > > backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 108 insertions(+) > > > create mode 100644 backends/hostmem-file.c > > > > > > diff --git a/backends/Makefile.objs b/backends/Makefile.objs > > > index 7fb7acd..506a46c 100644 > > > --- a/backends/Makefile.objs > > > +++ b/backends/Makefile.objs > > > @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS) > > > common-obj-$(CONFIG_TPM) += tpm.o > > > > > > common-obj-y += hostmem.o hostmem-ram.o > > > +common-obj-$(CONFIG_LINUX) += hostmem-file.o > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > > > new file mode 100644 > > > index 0000000..b8df933 > > > --- /dev/null > > > +++ b/backends/hostmem-file.c > > > @@ -0,0 +1,107 @@ > > > +/* > > > + * QEMU Host Memory Backend for hugetlbfs > > > + * > > > + * Copyright (C) 2013 Red Hat Inc > > > + * > > > + * Authors: > > > + * Paolo Bonzini <pbonzini@redhat.com> > > > + * > > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > > + * See the COPYING file in the top-level directory. > > > + */ > > > +#include "sysemu/hostmem.h" > > > +#include "qom/object_interfaces.h" > > > + > > > +/* hostmem-file.c */ > > > +/** > > > + * @TYPE_MEMORY_BACKEND_FILE: > > > + * name of backend that uses mmap on a file descriptor > > > + */ > > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > > how about naming it after what it really is? "memory-backend-hugepage" > > Later we could split it into generic superclass mmap-ed "memory-backend-file" > > and have TPH specific code moved into this backend. > > What does this last sentence mean? 1. currently file_ram_alloc() uses TPH specific code, I suggest to keep name "memory-backend-file" free for now so that in case if there would be need in a generic file backend, we could introduce it without causing confusion with TPH backend. 2. There is not much point to build TPH backend for every host, we can exclude it safely from non linux builds, instead of building it and make it failing at runtime. > > THP is transparent huge pages, right? yes. > > > > > > + > > > +#define MEMORY_BACKEND_FILE(obj) \ > > > + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE) > > > + > > > +typedef struct HostMemoryBackendFile HostMemoryBackendFile; > > > + > > > +struct HostMemoryBackendFile { > > > + HostMemoryBackend parent_obj; > > > + char *mem_path; > > > +}; > > > + > > > +static void > > > +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > > > +{ > > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); > > > + > > > + if (!backend->size) { > > > + error_setg(errp, "can't create backend with size 0"); > > > + return; > > > + } > > > + if (!fb->mem_path) { > > > + error_setg(errp, "mem-path property not set"); > > > + return; > > > + } > > > +#ifndef CONFIG_LINUX > > > + error_setg(errp, "-mem-path not supported on this host"); > > Is it possible to not compile this backend on non linux host at all, instead > > of ifdefs. > > > > > +#else > > > + if (!memory_region_size(&backend->mr)) { > > > + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > > > + object_get_canonical_path(OBJECT(backend)), > > > + backend->size, > > > + fb->mem_path, errp); > > > + } > > > +#endif > > > +} > > > + > > > +static void > > > +file_backend_class_init(ObjectClass *oc, void *data) > > > +{ > > > + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc); > > > + > > > + bc->alloc = file_backend_memory_alloc; > > > +} > > > + > > > +static char *get_mem_path(Object *o, Error **errp) > > > +{ > > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > > + > > > + return g_strdup(fb->mem_path); > > > +} > > > + > > > +static void set_mem_path(Object *o, const char *str, Error **errp) > > > +{ > > > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > > + > > > + if (memory_region_size(&backend->mr)) { > > > + error_setg(errp, "cannot change property value"); > > > + return; > > > + } > > > + if (fb->mem_path) { > > > + g_free(fb->mem_path); > > > + } > > > + fb->mem_path = g_strdup(str); > > > +} > > > + > > > +static void > > > +file_backend_instance_init(Object *o) > > > +{ > > > + object_property_add_str(o, "mem-path", get_mem_path, > > > + set_mem_path, NULL); > > s/"mem-path"/"path"/ > > > > > > > +} > > > + > > > +static const TypeInfo file_backend_info = { > > > + .name = TYPE_MEMORY_BACKEND_FILE, > > > + .parent = TYPE_MEMORY_BACKEND, > > > + .class_init = file_backend_class_init, > > > + .instance_init = file_backend_instance_init, > > > + .instance_size = sizeof(HostMemoryBackendFile), > > > +}; > > > + > > > +static void register_types(void) > > > +{ > > > + type_register_static(&file_backend_info); > > > +} > > > + > > > +type_init(register_types); > > > -- > > > 1.9.3 > > > > > > > > > -- > > Regards, > > Igor >
On Mon, Jun 09, 2014 at 01:32:46PM +0200, Igor Mammedov wrote: > On Mon, 9 Jun 2014 18:25:23 +0800 > Hu Tao <hutao@cn.fujitsu.com> wrote: > > > From: Paolo Bonzini <pbonzini@redhat.com> > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > backends/Makefile.objs | 1 + > > backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 108 insertions(+) > > create mode 100644 backends/hostmem-file.c > > > > diff --git a/backends/Makefile.objs b/backends/Makefile.objs > > index 7fb7acd..506a46c 100644 > > --- a/backends/Makefile.objs > > +++ b/backends/Makefile.objs > > @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS) > > common-obj-$(CONFIG_TPM) += tpm.o > > > > common-obj-y += hostmem.o hostmem-ram.o > > +common-obj-$(CONFIG_LINUX) += hostmem-file.o > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > > new file mode 100644 > > index 0000000..b8df933 > > --- /dev/null > > +++ b/backends/hostmem-file.c > > @@ -0,0 +1,107 @@ > > +/* > > + * QEMU Host Memory Backend for hugetlbfs > > + * > > + * Copyright (C) 2013 Red Hat Inc > > + * > > + * Authors: > > + * Paolo Bonzini <pbonzini@redhat.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > + * See the COPYING file in the top-level directory. > > + */ > > +#include "sysemu/hostmem.h" > > +#include "qom/object_interfaces.h" > > + > > +/* hostmem-file.c */ > > +/** > > + * @TYPE_MEMORY_BACKEND_FILE: > > + * name of backend that uses mmap on a file descriptor > > + */ > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > how about naming it after what it really is? "memory-backend-hugepage" > Later we could split it into generic superclass mmap-ed "memory-backend-file" > and have TPH specific code moved into this backend. OK. > > > + > > +#define MEMORY_BACKEND_FILE(obj) \ > > + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE) > > + > > +typedef struct HostMemoryBackendFile HostMemoryBackendFile; > > + > > +struct HostMemoryBackendFile { > > + HostMemoryBackend parent_obj; > > + char *mem_path; > > +}; > > + > > +static void > > +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > > +{ > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); > > + > > + if (!backend->size) { > > + error_setg(errp, "can't create backend with size 0"); > > + return; > > + } > > + if (!fb->mem_path) { > > + error_setg(errp, "mem-path property not set"); > > + return; > > + } > > +#ifndef CONFIG_LINUX > > + error_setg(errp, "-mem-path not supported on this host"); > Is it possible to not compile this backend on non linux host at all, instead > of ifdefs. Good idea! > > > +#else > > + if (!memory_region_size(&backend->mr)) { > > + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > > + object_get_canonical_path(OBJECT(backend)), > > + backend->size, > > + fb->mem_path, errp); > > + } > > +#endif > > +} > > + > > +static void > > +file_backend_class_init(ObjectClass *oc, void *data) > > +{ > > + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc); > > + > > + bc->alloc = file_backend_memory_alloc; > > +} > > + > > +static char *get_mem_path(Object *o, Error **errp) > > +{ > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > + > > + return g_strdup(fb->mem_path); > > +} > > + > > +static void set_mem_path(Object *o, const char *str, Error **errp) > > +{ > > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > > + > > + if (memory_region_size(&backend->mr)) { > > + error_setg(errp, "cannot change property value"); > > + return; > > + } > > + if (fb->mem_path) { > > + g_free(fb->mem_path); > > + } > > + fb->mem_path = g_strdup(str); > > +} > > + > > +static void > > +file_backend_instance_init(Object *o) > > +{ > > + object_property_add_str(o, "mem-path", get_mem_path, > > + set_mem_path, NULL); > s/"mem-path"/"path"/ OK. > > > > +} > > + > > +static const TypeInfo file_backend_info = { > > + .name = TYPE_MEMORY_BACKEND_FILE, > > + .parent = TYPE_MEMORY_BACKEND, > > + .class_init = file_backend_class_init, > > + .instance_init = file_backend_instance_init, > > + .instance_size = sizeof(HostMemoryBackendFile), > > +}; > > + > > +static void register_types(void) > > +{ > > + type_register_static(&file_backend_info); > > +} > > + > > +type_init(register_types); > > -- > > 1.9.3 > > > > > -- > Regards, > Igor
> > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > > how about naming it after what it really is? "memory-backend-hugepage" > > Later we could split it into generic superclass mmap-ed > > "memory-backend-file" and have TPH specific code moved into this backend. > > OK. Actually I don't think there's anything hugepage-specific in this backend (except perhaps passing a path instead of a filename). It could be used with a tmpfs backing storage like /dev/shm. Paolo
On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: > > > > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > > > how about naming it after what it really is? "memory-backend-hugepage" > > > Later we could split it into generic superclass mmap-ed > > > "memory-backend-file" and have TPH specific code moved into this backend. > > > > OK. > > Actually I don't think there's anything hugepage-specific in this backend > (except perhaps passing a path instead of a filename). It could be used > with a tmpfs backing storage like /dev/shm. What's the point compared to memory-backend-ram? Igor suggested memory-backend-file be compiled only for Linux. Does this mean memory-backend-file shuold be compiled also for systems supporting tmpfs or like? Regards, Hu
Il 10/06/2014 10:30, Hu Tao ha scritto: > On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: >> >>>>> +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" >>>> how about naming it after what it really is? "memory-backend-hugepage" >>>> Later we could split it into generic superclass mmap-ed >>>> "memory-backend-file" and have TPH specific code moved into this backend. >>> >>> OK. >> >> Actually I don't think there's anything hugepage-specific in this backend >> (except perhaps passing a path instead of a filename). It could be used >> with a tmpfs backing storage like /dev/shm. > > What's the point compared to memory-backend-ram? That you can use shared memory, for example together with vhost-user. > Igor suggested memory-backend-file be compiled only for Linux. Does this mean > memory-backend-file shuold be compiled also for systems supporting tmpfs > or like? Yes, I think it should be compiled on all POSIX systems. But it can be done later. Paolo
On Tue, 10 Jun 2014 16:30:06 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote: > On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: > > > > > > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > > > > how about naming it after what it really is? "memory-backend-hugepage" > > > > Later we could split it into generic superclass mmap-ed > > > > "memory-backend-file" and have TPH specific code moved into this backend. > > > > > > OK. > > > > Actually I don't think there's anything hugepage-specific in this backend > > (except perhaps passing a path instead of a filename). It could be used > > with a tmpfs backing storage like /dev/shm. > > What's the point compared to memory-backend-ram? > > Igor suggested memory-backend-file be compiled only for Linux. Does this mean > memory-backend-file shuold be compiled also for systems supporting tmpfs > or like? I was too hasty with this suggestion, looking again at behind scenes file_ram_alloc(), for now it works only with THP /gethugepagesize()/ but it could be modified to run on non linux hosts as well and take /dev/shm or just any file on host as backing storage. > > Regards, > Hu
On Tue, Jun 10, 2014 at 10:56:42AM +0200, Paolo Bonzini wrote: > Il 10/06/2014 10:30, Hu Tao ha scritto: > >On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: > >> > >>>>>+#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > >>>>how about naming it after what it really is? "memory-backend-hugepage" > >>>>Later we could split it into generic superclass mmap-ed > >>>>"memory-backend-file" and have TPH specific code moved into this backend. > >>> > >>>OK. > >> > >>Actually I don't think there's anything hugepage-specific in this backend > >>(except perhaps passing a path instead of a filename). It could be used > >>with a tmpfs backing storage like /dev/shm. > > > >What's the point compared to memory-backend-ram? > > That you can use shared memory, for example together with vhost-user. > > >Igor suggested memory-backend-file be compiled only for Linux. Does this mean > >memory-backend-file shuold be compiled also for systems supporting tmpfs > >or like? > > Yes, I think it should be compiled on all POSIX systems. But it can > be done later. OK. I'll leave the patch as is. Hu
On Tue, Jun 10, 2014 at 11:07:35AM +0200, Igor Mammedov wrote: > On Tue, 10 Jun 2014 16:30:06 +0800 > Hu Tao <hutao@cn.fujitsu.com> wrote: > > > On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: > > > > > > > > > +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > > > > > how about naming it after what it really is? "memory-backend-hugepage" > > > > > Later we could split it into generic superclass mmap-ed > > > > > "memory-backend-file" and have TPH specific code moved into this backend. > > > > > > > > OK. > > > > > > Actually I don't think there's anything hugepage-specific in this backend > > > (except perhaps passing a path instead of a filename). It could be used > > > with a tmpfs backing storage like /dev/shm. > > > > What's the point compared to memory-backend-ram? > > > > Igor suggested memory-backend-file be compiled only for Linux. Does this mean > > memory-backend-file shuold be compiled also for systems supporting tmpfs > > or like? > I was too hasty with this suggestion, looking again at behind scenes > file_ram_alloc(), for now it works only with THP You mean Hugetlbfs I guess, not THP? > /gethugepagesize()/ but > it could be modified to run on non linux hosts as well and take /dev/shm or > just any file on host as backing storage. Yes, however there's a problem: on linux THP does not work with non anonymous memory at the moment. So using this feature would slow everything down as you get more TLB misses. That would be quite unexpected for users. Requiring hugetlbfs follows the principle of least surprise. > > > > > Regards, > > Hu > > > -- > Regards, > Igor
On Tue, Jun 10, 2014 at 10:56:42AM +0200, Paolo Bonzini wrote: > Il 10/06/2014 10:30, Hu Tao ha scritto: > >On Tue, Jun 10, 2014 at 01:09:32AM -0400, Paolo Bonzini wrote: > >> > >>>>>+#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" > >>>>how about naming it after what it really is? "memory-backend-hugepage" > >>>>Later we could split it into generic superclass mmap-ed > >>>>"memory-backend-file" and have TPH specific code moved into this backend. > >>> > >>>OK. > >> > >>Actually I don't think there's anything hugepage-specific in this backend > >>(except perhaps passing a path instead of a filename). It could be used > >>with a tmpfs backing storage like /dev/shm. > > > >What's the point compared to memory-backend-ram? > > That you can use shared memory, for example together with vhost-user. I don't think it's a good idea until THP supports shared memory. > >Igor suggested memory-backend-file be compiled only for Linux. Does this mean > >memory-backend-file shuold be compiled also for systems supporting tmpfs > >or like? > > Yes, I think it should be compiled on all POSIX systems. But it can be done > later. > > Paolo For example when someone actually requests this :). Which is unlikely to happen soon I think.
Il 10/06/2014 11:59, Michael S. Tsirkin ha scritto: > > >What's the point compared to memory-backend-ram? > > > > That you can use shared memory, for example together with vhost-user. > > I don't think it's a good idea until THP supports shared memory. Why? For example it would be useful for testing on machines that you don't have root for, and that do not have a hugetlbfs mount point. For example you could run the test case from the vhost-user's patches. THP is not a magic wand and you can get slowness from memory fragmentation at any time. We should not limit ourselves due to kernel bugs. Paolo
On Tue, Jun 10, 2014 at 01:12:04PM +0200, Paolo Bonzini wrote: > Il 10/06/2014 11:59, Michael S. Tsirkin ha scritto: > >> >What's the point compared to memory-backend-ram? > >> > >> That you can use shared memory, for example together with vhost-user. > > > >I don't think it's a good idea until THP supports shared memory. > > Why? For example it would be useful for testing on machines that you don't > have root for, and that do not have a hugetlbfs mount point. For example > you could run the test case from the vhost-user's patches. Sounds useful, I guess we could allow this when running under qtest. > THP is not a magic wand and you can get slowness from memory fragmentation > at any time. Right but there's a difference between "can get slowness when memory is overcommitted" and "will get slowness even on a mostly idle box". > We should not limit ourselves due to kernel bugs. > > Paolo Why not? Practically people do have to run this on some kernel, we should not use kernel in a way that it can't support well. Old firefox doing a ton of fsync commands and slowing the box to a crawl comes to mind as another example of this.
Il 10/06/2014 13:23, Michael S. Tsirkin ha scritto: > On Tue, Jun 10, 2014 at 01:12:04PM +0200, Paolo Bonzini wrote: >> Il 10/06/2014 11:59, Michael S. Tsirkin ha scritto: >>>>> What's the point compared to memory-backend-ram? >>>> >>>> That you can use shared memory, for example together with vhost-user. >>> >>> I don't think it's a good idea until THP supports shared memory. >> >> Why? For example it would be useful for testing on machines that you don't >> have root for, and that do not have a hugetlbfs mount point. For example >> you could run the test case from the vhost-user's patches. > > Sounds useful, I guess we could allow this when running under qtest. Or TCG, or Xen. At this point, why single out KVM? (Also, "--enable-kvm -mem-path /dev/shm" works on 2.0, and it would be a regression in 2.1). >> THP is not a magic wand and you can get slowness from memory fragmentation >> at any time. > > Right but there's a difference between "can get slowness when memory > is overcommitted" and "will get slowness even on a mostly idle box". I would like to see the slowness on a real-world benchmark though. I suspect in most scenarios it would not matter. >> We should not limit ourselves due to kernel bugs. > > Why not? Practically people do have to run this on some kernel, > we should not use kernel in a way that it can't support well. > Old firefox doing a ton of fsync commands and slowing > the box to a crawl comes to mind as another example of this. Yes, and firefox doesn't say "no sorry can't do it" when running on such a kernel (which is much worse than more expensive TLB misses). Paolo
On Tue, Jun 10, 2014 at 01:29:06PM +0200, Paolo Bonzini wrote: > Il 10/06/2014 13:23, Michael S. Tsirkin ha scritto: > >On Tue, Jun 10, 2014 at 01:12:04PM +0200, Paolo Bonzini wrote: > >>Il 10/06/2014 11:59, Michael S. Tsirkin ha scritto: > >>>>>What's the point compared to memory-backend-ram? > >>>> > >>>>That you can use shared memory, for example together with vhost-user. > >>> > >>>I don't think it's a good idea until THP supports shared memory. > >> > >>Why? For example it would be useful for testing on machines that you don't > >>have root for, and that do not have a hugetlbfs mount point. For example > >>you could run the test case from the vhost-user's patches. > > > >Sounds useful, I guess we could allow this when running under qtest. > > Or TCG, or Xen. At this point, why single out KVM? > > (Also, "--enable-kvm -mem-path /dev/shm" works on 2.0, and it would be a > regression in 2.1). It prints fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path); Correct? I guess I agree then, hopefully the warning is enough. Maybe add an extra warning that performance will suffer. > >>THP is not a magic wand and you can get slowness from memory fragmentation > >>at any time. > > > >Right but there's a difference between "can get slowness when memory > >is overcommitted" and "will get slowness even on a mostly idle box". > > I would like to see the slowness on a real-world benchmark though. I > suspect in most scenarios it would not matter. Weird. Things like kernel build time are known to be measureably improved by using THP. > >>We should not limit ourselves due to kernel bugs. > > > >Why not? Practically people do have to run this on some kernel, > >we should not use kernel in a way that it can't support well. > >Old firefox doing a ton of fsync commands and slowing > >the box to a crawl comes to mind as another example of this. > > Yes, and firefox doesn't say "no sorry can't do it" when running on such a > kernel (which is much worse than more expensive TLB misses). > > Paolo kernel can't speed up fsync. So IIRC instead, firefox switched to using renames instead of fsync. IMHO QEMU should do the same, look for a mechanism that kernel can support efficiently, instead of insisting on using a feature that it can't.
Il 10/06/2014 13:35, Michael S. Tsirkin ha scritto: >>>> Why? For example it would be useful for testing on machines that you don't >>>> have root for, and that do not have a hugetlbfs mount point. For example >>>> you could run the test case from the vhost-user's patches. >>> >>> Sounds useful, I guess we could allow this when running under qtest. >> >> Or TCG, or Xen. At this point, why single out KVM? >> >> (Also, "--enable-kvm -mem-path /dev/shm" works on 2.0, and it would be a >> regression in 2.1). > > It prints > fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path); > > Correct? Yes. > I guess I agree then, hopefully the warning is enough. > Maybe add an extra warning that performance will suffer. > >>>> THP is not a magic wand and you can get slowness from memory fragmentation >>>> at any time. >>> >>> Right but there's a difference between "can get slowness when memory >>> is overcommitted" and "will get slowness even on a mostly idle box". >> >> I would like to see the slowness on a real-world benchmark though. I >> suspect in most scenarios it would not matter. > > Weird. Things like kernel build time are known to be measureably > improved by using THP. Even measurable speedups in most scenarios would not matter. I don't care if a kernel compile takes 2 minutes vs. 110 seconds (for a 10% speedup), even though it's great that THP can speed up such a common task. Paolo
On Tue, Jun 10, 2014 at 01:43:27PM +0200, Paolo Bonzini wrote: > Il 10/06/2014 13:35, Michael S. Tsirkin ha scritto: > >>>>Why? For example it would be useful for testing on machines that you don't > >>>>have root for, and that do not have a hugetlbfs mount point. For example > >>>>you could run the test case from the vhost-user's patches. > >>> > >>>Sounds useful, I guess we could allow this when running under qtest. > >> > >>Or TCG, or Xen. At this point, why single out KVM? > >> > >>(Also, "--enable-kvm -mem-path /dev/shm" works on 2.0, and it would be a > >>regression in 2.1). > > > >It prints > > fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path); > > > >Correct? > > Yes. > > >I guess I agree then, hopefully the warning is enough. > >Maybe add an extra warning that performance will suffer. > > > >>>>THP is not a magic wand and you can get slowness from memory fragmentation > >>>>at any time. > >>> > >>>Right but there's a difference between "can get slowness when memory > >>>is overcommitted" and "will get slowness even on a mostly idle box". > >> > >>I would like to see the slowness on a real-world benchmark though. I > >>suspect in most scenarios it would not matter. > > > >Weird. Things like kernel build time are known to be measureably > >improved by using THP. > > Even measurable speedups in most scenarios would not matter. I don't care > if a kernel compile takes 2 minutes vs. 110 seconds (for a 10% speedup), > even though it's great that THP can speed up such a common task. > > Paolo True. But I am not sure why would such a user play with vhost-user at all. That one seems to mostly be about using aggressive polling to drive down guest to guest latency.
Il 10/06/2014 13:48, Michael S. Tsirkin ha scritto: >> Even measurable speedups in most scenarios would not matter. I don't care >> if a kernel compile takes 2 minutes vs. 110 seconds (for a 10% speedup), >> even though it's great that THP can speed up such a common task. > > True. But I am not sure why would such a user play with vhost-user at all. > That one seems to mostly be about using aggressive polling > to drive down guest to guest latency. But then there is so much more you have to do to get the performance you're looking for, including using GB hugepages which needs hugetlbfs anyway. Anyhow, since there is a warning and the behavior is the same as 2.0 the question is moot, I think. Renaming memory-backend-file to memory-backend-hugetlbfs would suggest that there is a regression, which is not the case. Paolo
diff --git a/backends/Makefile.objs b/backends/Makefile.objs index 7fb7acd..506a46c 100644 --- a/backends/Makefile.objs +++ b/backends/Makefile.objs @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS) common-obj-$(CONFIG_TPM) += tpm.o common-obj-y += hostmem.o hostmem-ram.o +common-obj-$(CONFIG_LINUX) += hostmem-file.o diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c new file mode 100644 index 0000000..b8df933 --- /dev/null +++ b/backends/hostmem-file.c @@ -0,0 +1,107 @@ +/* + * QEMU Host Memory Backend for hugetlbfs + * + * Copyright (C) 2013 Red Hat Inc + * + * Authors: + * Paolo Bonzini <pbonzini@redhat.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "sysemu/hostmem.h" +#include "qom/object_interfaces.h" + +/* hostmem-file.c */ +/** + * @TYPE_MEMORY_BACKEND_FILE: + * name of backend that uses mmap on a file descriptor + */ +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file" + +#define MEMORY_BACKEND_FILE(obj) \ + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE) + +typedef struct HostMemoryBackendFile HostMemoryBackendFile; + +struct HostMemoryBackendFile { + HostMemoryBackend parent_obj; + char *mem_path; +}; + +static void +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) +{ + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); + + if (!backend->size) { + error_setg(errp, "can't create backend with size 0"); + return; + } + if (!fb->mem_path) { + error_setg(errp, "mem-path property not set"); + return; + } +#ifndef CONFIG_LINUX + error_setg(errp, "-mem-path not supported on this host"); +#else + if (!memory_region_size(&backend->mr)) { + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), + object_get_canonical_path(OBJECT(backend)), + backend->size, + fb->mem_path, errp); + } +#endif +} + +static void +file_backend_class_init(ObjectClass *oc, void *data) +{ + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc); + + bc->alloc = file_backend_memory_alloc; +} + +static char *get_mem_path(Object *o, Error **errp) +{ + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); + + return g_strdup(fb->mem_path); +} + +static void set_mem_path(Object *o, const char *str, Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(o); + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); + + if (memory_region_size(&backend->mr)) { + error_setg(errp, "cannot change property value"); + return; + } + if (fb->mem_path) { + g_free(fb->mem_path); + } + fb->mem_path = g_strdup(str); +} + +static void +file_backend_instance_init(Object *o) +{ + object_property_add_str(o, "mem-path", get_mem_path, + set_mem_path, NULL); +} + +static const TypeInfo file_backend_info = { + .name = TYPE_MEMORY_BACKEND_FILE, + .parent = TYPE_MEMORY_BACKEND, + .class_init = file_backend_class_init, + .instance_init = file_backend_instance_init, + .instance_size = sizeof(HostMemoryBackendFile), +}; + +static void register_types(void) +{ + type_register_static(&file_backend_info); +} + +type_init(register_types);