On Mon, Dec 16, 2013 at 8:32 AM, Edgar E. Iglesias <edgar.igles...@gmail.com
> wrote:
> On Fri, Dec 13, 2013 at 12:14:31PM +0100, Antonios Motakis wrote:
> > This option complements -mem-path. It implies -mem-prealloc. If
> specified,
> > the guest RAM is allocated as a shared memory object. If both -mem-path
> > and -mem-share are provided, the memory is allocated from the HugeTLBFS
> > supplied path, and then mmapped with MAP_SHARED.
>
> Hi,
>
> Interesting, I've got a similar use-case here where I've added a
> -mem-shared
> option. I've got a few comments/questions.
>
> Why do you imply -mem-prealloc? cant the user keep controlling that through
> -mem-prealloc?
>
> I'd prefer if -mem-share did not use shm_open but took a directory path as
> arg
> and created the backing files there. I'd also prefer if the files had
> deterministic names and where not unlinked after creation. I.e, let the
> user
> delete them when no longer needed.
>
> The reason for this is that it makes it easier to use apps that are not
> aware of shm or QEMU specifics to manipulate the memory backing. I
> understand
> that there might be issues (e.g filling up the disk, slow access over NFS
> etc)
> but these are at the choice of the user.
>
>
Currently -mem-path implies HugeTLBFS for the supplied path. Maybe we
should change
its behavior to do what you suggest:
-mem-path - specify a directory where to allocate the mmap-ed ram files
(and don't unlink them)
-mem-hugetlbfs - check mem-path directory for HugeTLBFS (do we need this
one?)
-mem-share - add MAP_SHARED to mmap
-mem-prealloc - preallocate the memory
How does this sound?
> Any thoughts around this?
>
> Best regards,
> Edgar
>
>
>
> >
> > Signed-off-by: Antonios Motakis <a.mota...@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikol...@virtualopensystems.com>
> > ---
> > exec.c | 72
> +++++++++++++++++++++++++++++++++++---------------
> > include/exec/cpu-all.h | 1 +
> > qemu-options.hx | 9 +++++++
> > vl.c | 5 ++++
> > 4 files changed, 66 insertions(+), 21 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index f4b9ef2..10720f1 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -883,6 +883,7 @@ void qemu_mutex_unlock_ramlist(void)
> > #include <sys/vfs.h>
> >
> > #define HUGETLBFS_MAGIC 0x958458f6
> > +#define MIN_HUGE_PAGE_SIZE (2*1024*1024)
> >
> > static long gethugepagesize(const char *path)
> > {
> > @@ -920,15 +921,24 @@ static void *file_ram_alloc(RAMBlock *block,
> > char *c;
> > void *area;
> > int fd;
> > + int flags;
> > unsigned long hpagesize;
> >
> > - hpagesize = gethugepagesize(path);
> > - if (!hpagesize) {
> > - return NULL;
> > - }
> > + if (path) {
> > + hpagesize = gethugepagesize(path);
> >
> > - if (memory < hpagesize) {
> > - return NULL;
> > + if (!hpagesize) {
> > + return NULL ;
> > + }
> > +
> > + if (memory < hpagesize) {
> > + return NULL ;
> > + }
> > + } else {
> > + if (memory < MIN_HUGE_PAGE_SIZE) {
> > + return NULL ;
> > + }
> > + hpagesize = MIN_HUGE_PAGE_SIZE;
> > }
> >
> > if (kvm_enabled() && !kvm_has_sync_mmu()) {
> > @@ -943,20 +953,37 @@ static void *file_ram_alloc(RAMBlock *block,
> > *c = '_';
> > }
> >
> > - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> > - sanitized_name);
> > - g_free(sanitized_name);
> > + if (path) {
> > +
> > + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
> > + sanitized_name);
> > + g_free(sanitized_name);
> >
> > - fd = mkstemp(filename);
> > - if (fd < 0) {
> > - perror("unable to create backing store for hugepages");
> > + fd = mkstemp(filename);
> > + if (fd < 0) {
> > + perror("unable to create backing store for huge");
> > + g_free(filename);
> > + return NULL ;
> > + }
> > + unlink(filename);
> > g_free(filename);
> > + memory = (memory + hpagesize - 1) & ~(hpagesize - 1);
> > + } else if (mem_share) {
> > + filename = g_strdup_printf("qemu_back_mem.%s.XXXXXX",
> sanitized_name);
> > + g_free(sanitized_name);
> > + fd = shm_open(filename, O_CREAT | O_RDWR | O_EXCL,
> > + S_IRWXU | S_IRWXG | S_IRWXO);
> > + if (fd < 0) {
> > + perror("unable to create backing store for shared memory");
> > + g_free(filename);
> > + return NULL ;
> > + }
> > + shm_unlink(filename);
> > + g_free(filename);
> > + } else {
> > + fprintf(stderr, "-mem-path or -mem-share required\n");
> > return NULL;
> > }
> > - unlink(filename);
> > - g_free(filename);
> > -
> > - memory = (memory+hpagesize-1) & ~(hpagesize-1);
> >
> > /*
> > * ftruncate is not supported by hugetlbfs in older
> > @@ -967,7 +994,8 @@ static void *file_ram_alloc(RAMBlock *block,
> > if (ftruncate(fd, memory))
> > perror("ftruncate");
> >
> > - area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> > + flags = mem_share ? MAP_SHARED : MAP_PRIVATE;
> > + area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0);
> > if (area == MAP_FAILED) {
> > perror("file_ram_alloc: can't mmap RAM pages");
> > close(fd);
> > @@ -1150,13 +1178,14 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t
> size, void *host,
> > new_block->host = host;
> > new_block->flags |= RAM_PREALLOC_MASK;
> > } else if (xen_enabled()) {
> > - if (mem_path) {
> > - fprintf(stderr, "-mem-path not supported with Xen\n");
> > + if (mem_path || mem_share) {
> > + fprintf(stderr,
> > + "-mem-path and -mem-share not supported with
> Xen\n");
> > exit(1);
> > }
> > xen_ram_alloc(new_block->offset, size, mr);
> > } else {
> > - if (mem_path) {
> > + if (mem_path || mem_share) {
> > if (phys_mem_alloc != qemu_anon_ram_alloc) {
> > /*
> > * file_ram_alloc() needs to allocate just like
> > @@ -1164,7 +1193,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t
> size, void *host,
> > * a hook there.
> > */
> > fprintf(stderr,
> > - "-mem-path not supported with this
> accelerator\n");
> > + "-mem-path and -mem-share "
> > + "not supported with this accelerator\n");
> > exit(1);
> > }
> > new_block->host = file_ram_alloc(new_block, size, mem_path);
> > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> > index b6998f0..05ad0ee 100644
> > --- a/include/exec/cpu-all.h
> > +++ b/include/exec/cpu-all.h
> > @@ -469,6 +469,7 @@ extern RAMList ram_list;
> >
> > extern const char *mem_path;
> > extern int mem_prealloc;
> > +extern int mem_share;
> >
> > /* Flags stored in the low bits of the TLB virtual address. These are
> > defined so that fast path ram access is all zeros. */
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index af34483..bd70777 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -237,6 +237,15 @@ STEXI
> > Preallocate memory when using -mem-path.
> > ETEXI
> >
> > +DEF("mem-share", 0, QEMU_OPTION_mem_share,
> > + "-mem-share share guest memory (implies -mem-prealloc)\n",
> > + QEMU_ARCH_ALL)
> > +STEXI
> > +@item -mem-share
> > +@findex -mem-share
> > +Allocate guest RAM as a shared memory object.
> > +ETEXI
> > +
> > DEF("k", HAS_ARG, QEMU_OPTION_k,
> > "-k language use keyboard layout (for example 'fr' for
> French)\n",
> > QEMU_ARCH_ALL)
> > diff --git a/vl.c b/vl.c
> > index b0399de..d2f7403 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -189,6 +189,7 @@ const char* keyboard_layout = NULL;
> > ram_addr_t ram_size;
> > const char *mem_path = NULL;
> > int mem_prealloc = 0; /* force preallocation of physical target memory
> */
> > +int mem_share = 0; /* allocate shared memory */
> > int nb_nics;
> > NICInfo nd_table[MAX_NICS];
> > int autostart;
> > @@ -3212,6 +3213,10 @@ int main(int argc, char **argv, char **envp)
> > case QEMU_OPTION_mem_prealloc:
> > mem_prealloc = 1;
> > break;
> > + case QEMU_OPTION_mem_share:
> > + mem_share = 1;
> > + mem_prealloc = 1;
> > + break;
> > case QEMU_OPTION_d:
> > log_mask = optarg;
> > break;
> > --
> > 1.8.3.2
> >
> >
>
