All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd
@ 2019-07-23 14:04 Stefano Garzarella
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load Stefano Garzarella
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-23 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, Paolo Bonzini,
	Richard Henderson

In order to reduce the memory footprint when PVH kernel and initrd
are used, we map them into memory instead of reading them.
In this way we can share them between multiple instances of QEMU.

v2:
- Patch 1: used g_mapped_file_new_from_fd() with 'writeble' set to 'true',
           since we can modify the mapped buffer. [Paolo, Peter]

These are the results using a PVH kernel and initrd (cpio):
- memory footprint (using smem) [MB]
        QEMU              before                   now
    # instances        USS      PSS            USS      PSS
         1           102.0M   105.8M         102.3M   106.2M
         2            94.6M   101.2M          72.3M    90.1M
         4            94.1M    98.0M          72.0M    81.5M
         8            94.0M    96.2M          71.8M    76.9M
        16            93.9M    95.1M          71.6M    74.3M

    Initrd size: 3.0M
    Kernel
        image size: 28M
        sections size [size -A -d vmlinux]:  18.9M

- boot time [ms]
                          before                   now
 qemu_init_end:           63.85                   55.91
 linux_start_kernel:      82.11 (+18.26)          74.51 (+18.60)
 linux_start_user:       169.94 (+87.83)         159.06 (+84.56)

QEMU command used:
./qemu-system-x86_64 -bios /path/to/seabios/out/bios.bin -no-hpet \
    -machine q35,accel=kvm,kernel_irqchip,nvdimm,sata=off,smbus=off,vmport=off \
    -cpu host -m 1G -smp 1 -vga none -display none -no-user-config -nodefaults \
    -kernel /path/to/vmlinux -initrd /path/to/rootfs.cpio \
    -append 'root=/dev/mem0 ro console=hvc0 pci=lastbus=0 nosmap'

Stefano Garzarella (2):
  elf-ops.h: Map into memory the ELF to load
  hw/i386/pc: Map into memory the initrd

 hw/i386/pc.c         | 15 ++++++++---
 include/hw/elf_ops.h | 64 ++++++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 33 deletions(-)

-- 
2.20.1



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

* [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load
  2019-07-23 14:04 [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Stefano Garzarella
@ 2019-07-23 14:04 ` Stefano Garzarella
  2019-07-23 14:33   ` Paolo Bonzini
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd Stefano Garzarella
  2019-07-23 17:37 ` [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Montes, Julio
  2 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-23 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, Paolo Bonzini,
	Richard Henderson

In order to reduce the memory footprint we map into memory
the ELF to load using g_mapped_file_new_from_fd() instead of
reading each sections. In this way we can share the ELF pages
between multiple instances of QEMU.

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
  - used g_mapped_file_new_from_fd() with 'writeble' set to 'true',
    since we can modify the mapped buffer. [Paolo, Peter]
---
 include/hw/elf_ops.h | 64 ++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 690f9238c8..d80e7ad20c 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -323,8 +323,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
     struct elfhdr ehdr;
     struct elf_phdr *phdr = NULL, *ph;
     int size, i, total_size;
-    elf_word mem_size, file_size;
+    elf_word mem_size, file_size, data_offset;
     uint64_t addr, low = (uint64_t)-1, high = 0;
+    GMappedFile *gmf = NULL;
     uint8_t *data = NULL;
     char label[128];
     int ret = ELF_LOAD_FAILED;
@@ -409,22 +410,31 @@ static int glue(load_elf, SZ)(const char *name, int fd,
         }
     }
 
+    /*
+     * Since we want to be able to modify the mapped buffer, we set the
+     * 'writeble' parameter to 'true'. Modifications to the buffer are not
+     * written back to the file.
+     */
+    gmf = g_mapped_file_new_from_fd(fd, true, NULL);
+    if (!gmf) {
+        goto fail;
+    }
+
     total_size = 0;
     for(i = 0; i < ehdr.e_phnum; i++) {
         ph = &phdr[i];
         if (ph->p_type == PT_LOAD) {
             mem_size = ph->p_memsz; /* Size of the ROM */
             file_size = ph->p_filesz; /* Size of the allocated data */
-            data = g_malloc0(file_size);
-            if (ph->p_filesz > 0) {
-                if (lseek(fd, ph->p_offset, SEEK_SET) < 0) {
-                    goto fail;
-                }
-                if (read(fd, data, file_size) != file_size) {
-                    goto fail;
-                }
+            data_offset = ph->p_offset; /* Offset where the data is located */
+
+            if (g_mapped_file_get_length(gmf) < file_size + data_offset) {
+                goto fail;
             }
 
+            data = (uint8_t *)g_mapped_file_get_contents(gmf);
+            data += data_offset;
+
             /* The ELF spec is somewhat vague about the purpose of the
              * physical address field. One common use in the embedded world
              * is that physical address field specifies the load address
@@ -513,17 +523,16 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                 *pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr;
             }
 
-            if (mem_size == 0) {
-                /* Some ELF files really do have segments of zero size;
-                 * just ignore them rather than trying to create empty
-                 * ROM blobs, because the zero-length blob can falsely
-                 * trigger the overlapping-ROM-blobs check.
-                 */
-                g_free(data);
-            } else {
+            /* Some ELF files really do have segments of zero size;
+             * just ignore them rather than trying to create empty
+             * ROM blobs, because the zero-length blob can falsely
+             * trigger the overlapping-ROM-blobs check.
+             */
+            if (mem_size != 0) {
                 if (load_rom) {
                     snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
-
+                    /* Increments the reference count to avoid the unmap */
+                    g_mapped_file_ref(gmf);
                     /* rom_add_elf_program() seize the ownership of 'data' */
                     rom_add_elf_program(label, data, file_size, mem_size,
                                         addr, as);
@@ -531,7 +540,6 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                     address_space_write(as ? as : &address_space_memory,
                                         addr, MEMTXATTRS_UNSPECIFIED,
                                         data, file_size);
-                    g_free(data);
                 }
             }
 
@@ -547,16 +555,15 @@ static int glue(load_elf, SZ)(const char *name, int fd,
             struct elf_note *nhdr = NULL;
 
             file_size = ph->p_filesz; /* Size of the range of ELF notes */
-            data = g_malloc0(file_size);
-            if (ph->p_filesz > 0) {
-                if (lseek(fd, ph->p_offset, SEEK_SET) < 0) {
-                    goto fail;
-                }
-                if (read(fd, data, file_size) != file_size) {
-                    goto fail;
-                }
+            data_offset = ph->p_offset; /* Offset where the notes are located */
+
+            if (g_mapped_file_get_length(gmf) < file_size + data_offset) {
+                goto fail;
             }
 
+            data = (uint8_t *)g_mapped_file_get_contents(gmf);
+            data += data_offset;
+
             /*
              * Search the ELF notes to find one with a type matching the
              * value passed in via 'translate_opaque'
@@ -570,7 +577,6 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                     sizeof(struct elf_note) == sizeof(struct elf64_note);
                 elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
             }
-            g_free(data);
             data = NULL;
         }
     }
@@ -582,7 +588,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
         *highaddr = (uint64_t)(elf_sword)high;
     return total_size;
  fail:
-    g_free(data);
+    g_mapped_file_unref(gmf);
     g_free(phdr);
     return ret;
 }
-- 
2.20.1



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

* [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd
  2019-07-23 14:04 [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Stefano Garzarella
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load Stefano Garzarella
@ 2019-07-23 14:04 ` Stefano Garzarella
  2019-07-23 14:30   ` Paolo Bonzini
  2019-07-23 17:37 ` [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Montes, Julio
  2 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-23 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, Paolo Bonzini,
	Richard Henderson

In order to reduce the memory footprint we map into memory
the initrd using g_mapped_file_new() instead of reading it.
In this way we can share the initrd pages between multiple
instances of QEMU.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 hw/i386/pc.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 549c437050..b139589777 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1241,17 +1241,20 @@ static void load_linux(PCMachineState *pcms,
 
             /* load initrd */
             if (initrd_filename) {
+                GMappedFile *gmf;
                 gsize initrd_size;
                 gchar *initrd_data;
                 GError *gerr = NULL;
 
-                if (!g_file_get_contents(initrd_filename, &initrd_data,
-                            &initrd_size, &gerr)) {
+                gmf = g_mapped_file_new(initrd_filename, false, &gerr);
+                if (!gmf) {
                     fprintf(stderr, "qemu: error reading initrd %s: %s\n",
                             initrd_filename, gerr->message);
                     exit(1);
                 }
 
+                initrd_data = g_mapped_file_get_contents(gmf);
+                initrd_size = g_mapped_file_get_length(gmf);
                 initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
                 if (initrd_size >= initrd_max) {
                     fprintf(stderr, "qemu: initrd is too large, cannot support."
@@ -1378,6 +1381,7 @@ static void load_linux(PCMachineState *pcms,
 
     /* load initrd */
     if (initrd_filename) {
+        GMappedFile *gmf;
         gsize initrd_size;
         gchar *initrd_data;
         GError *gerr = NULL;
@@ -1387,12 +1391,15 @@ static void load_linux(PCMachineState *pcms,
             exit(1);
         }
 
-        if (!g_file_get_contents(initrd_filename, &initrd_data,
-                                 &initrd_size, &gerr)) {
+        gmf = g_mapped_file_new(initrd_filename, false, &gerr);
+        if (!gmf) {
             fprintf(stderr, "qemu: error reading initrd %s: %s\n",
                     initrd_filename, gerr->message);
             exit(1);
         }
+
+        initrd_data = g_mapped_file_get_contents(gmf);
+        initrd_size = g_mapped_file_get_length(gmf);
         if (initrd_size >= initrd_max) {
             fprintf(stderr, "qemu: initrd is too large, cannot support."
                     "(max: %"PRIu32", need %"PRId64")\n",
-- 
2.20.1



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

* Re: [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd Stefano Garzarella
@ 2019-07-23 14:30   ` Paolo Bonzini
  2019-07-23 14:47     ` Stefano Garzarella
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2019-07-23 14:30 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, Richard Henderson

On 23/07/19 16:04, Stefano Garzarella wrote:
> In order to reduce the memory footprint we map into memory
> the initrd using g_mapped_file_new() instead of reading it.
> In this way we can share the initrd pages between multiple
> instances of QEMU.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Coverity is going to complain about the leaks.  Should we instead store
the initrd GMappedFile in PCMachineState, even if it is just for reference?

Paolo

> ---
>  hw/i386/pc.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 549c437050..b139589777 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1241,17 +1241,20 @@ static void load_linux(PCMachineState *pcms,
>  
>              /* load initrd */
>              if (initrd_filename) {
> +                GMappedFile *gmf;
>                  gsize initrd_size;
>                  gchar *initrd_data;
>                  GError *gerr = NULL;
>  
> -                if (!g_file_get_contents(initrd_filename, &initrd_data,
> -                            &initrd_size, &gerr)) {
> +                gmf = g_mapped_file_new(initrd_filename, false, &gerr);
> +                if (!gmf) {
>                      fprintf(stderr, "qemu: error reading initrd %s: %s\n",
>                              initrd_filename, gerr->message);
>                      exit(1);
>                  }
>  
> +                initrd_data = g_mapped_file_get_contents(gmf);
> +                initrd_size = g_mapped_file_get_length(gmf);
>                  initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
>                  if (initrd_size >= initrd_max) {
>                      fprintf(stderr, "qemu: initrd is too large, cannot support."
> @@ -1378,6 +1381,7 @@ static void load_linux(PCMachineState *pcms,
>  
>      /* load initrd */
>      if (initrd_filename) {
> +        GMappedFile *gmf;
>          gsize initrd_size;
>          gchar *initrd_data;
>          GError *gerr = NULL;
> @@ -1387,12 +1391,15 @@ static void load_linux(PCMachineState *pcms,
>              exit(1);
>          }
>  
> -        if (!g_file_get_contents(initrd_filename, &initrd_data,
> -                                 &initrd_size, &gerr)) {
> +        gmf = g_mapped_file_new(initrd_filename, false, &gerr);
> +        if (!gmf) {
>              fprintf(stderr, "qemu: error reading initrd %s: %s\n",
>                      initrd_filename, gerr->message);
>              exit(1);
>          }
> +
> +        initrd_data = g_mapped_file_get_contents(gmf);
> +        initrd_size = g_mapped_file_get_length(gmf);
>          if (initrd_size >= initrd_max) {
>              fprintf(stderr, "qemu: initrd is too large, cannot support."
>                      "(max: %"PRIu32", need %"PRId64")\n",
> 




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

* Re: [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load Stefano Garzarella
@ 2019-07-23 14:33   ` Paolo Bonzini
  2019-07-23 14:57     ` Stefano Garzarella
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2019-07-23 14:33 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, Richard Henderson

On 23/07/19 16:04, Stefano Garzarella wrote:
> +                    /* Increments the reference count to avoid the unmap */
> +                    g_mapped_file_ref(gmf);
>                      /* rom_add_elf_program() seize the ownership of 'data' */
>                      rom_add_elf_program(label, data, file_size, mem_size,
>                                          addr, as);

I'm a bit worried about rom_reset g_free'ing rom->data, which goes
against the comment on top of rom_free:

/* rom->data must be heap-allocated (do not use with
   rom_add_elf_program()) */


Since this is the only call to rom_add_elf_program, what about adding a
GMappedFile* field to struct Rom and passing it here instead of
data+file_size?

Then the g_mapped_file_ref can be in rom_add_elf_program, and you can
have a nice

static void rom_free_data(Rom *rom)
{
    if (rom->mapped_file) {
        g_mapped_file_unref(rom->mapped_file);
        rom->mapped_file = NULL;
    } else {
        g_free(rom->data);
    }
    rom->data = NULL;
}

that is called from both rom_free and rom_reset.

Thanks,

Paolo

> @@ -531,7 +540,6 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                      address_space_write(as ? as : &address_space_memory,
>                                          addr, MEMTXATTRS_UNSPECIFIED,
>                                          data, file_size);
> -                    g_free(data);
>                  }
>              }
>  
> @@ -547,16 +555,15 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>              struct elf_note *nhdr = NULL;
>  
>              file_size = ph->p_filesz; /* Size of the range of ELF notes */
> -            data = g_malloc0(file_size);
> -            if (ph->p_filesz > 0) {
> -                if (lseek(fd, ph->p_offset, SEEK_SET) < 0) {
> -                    goto fail;
> -                }
> -                if (read(fd, data, file_size) != file_size) {
> -                    goto fail;
> -                }
> +            data_offset = ph->p_offset; /* Offset where the notes are located */
> +
> +            if (g_mapped_file_get_length(gmf) < file_size + data_offset) {
> +                goto fail;
>              }
>  
> +            data = (uint8_t *)g_mapped_file_get_contents(gmf);
> +            data += data_offset;
> +
>              /*
>               * Search the ELF notes to find one with a type matching the
>               * value passed in via 'translate_opaque'
> @@ -570,7 +577,6 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                      sizeof(struct elf_note) == sizeof(struct elf64_note);
>                  elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
>              }
> -            g_free(data);
>              data = NULL;
>          }
>      }
> @@ -582,7 +588,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>          *highaddr = (uint64_t)(elf_sword)high;
>      return total_size;
>   fail:
> -    g_free(data);
> +    g_mapped_file_unref(gmf);
>      g_free(phdr);
>      return ret;
>  }
> 



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

* Re: [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd
  2019-07-23 14:30   ` Paolo Bonzini
@ 2019-07-23 14:47     ` Stefano Garzarella
  0 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-23 14:47 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, qemu-devel,
	Richard Henderson

On Tue, Jul 23, 2019 at 04:30:17PM +0200, Paolo Bonzini wrote:
> On 23/07/19 16:04, Stefano Garzarella wrote:
> > In order to reduce the memory footprint we map into memory
> > the initrd using g_mapped_file_new() instead of reading it.
> > In this way we can share the initrd pages between multiple
> > instances of QEMU.
> > 
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> 
> Coverity is going to complain about the leaks.  Should we instead store
> the initrd GMappedFile in PCMachineState, even if it is just for reference?
> 

Yes, I'll put it in the PCMachineState.

Thanks,
Stefano


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

* Re: [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load
  2019-07-23 14:33   ` Paolo Bonzini
@ 2019-07-23 14:57     ` Stefano Garzarella
  2019-07-23 15:00       ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-23 14:57 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, qemu-devel,
	Richard Henderson

On Tue, Jul 23, 2019 at 04:33:44PM +0200, Paolo Bonzini wrote:
> On 23/07/19 16:04, Stefano Garzarella wrote:
> > +                    /* Increments the reference count to avoid the unmap */
> > +                    g_mapped_file_ref(gmf);
> >                      /* rom_add_elf_program() seize the ownership of 'data' */
> >                      rom_add_elf_program(label, data, file_size, mem_size,
> >                                          addr, as);
> 
> I'm a bit worried about rom_reset g_free'ing rom->data, which goes
> against the comment on top of rom_free:
> 
> /* rom->data must be heap-allocated (do not use with
>    rom_add_elf_program()) */

Thanks for pointing it out. We definitely have to avoid that free in this case.

> 
> 
> Since this is the only call to rom_add_elf_program, what about adding a
> GMappedFile* field to struct Rom and passing it here instead of
> data+file_size?

Ehm, we currently use a subsection of the mmapped file as a ROM.
Should we keep the data+file_size parameter? (plus the new GMappedFile*)

At this point, is it better to call 'g_mapped_file_ref(gmf)' in the
'rom_add_elf_program()'?

> 
> Then the g_mapped_file_ref can be in rom_add_elf_program, and you can
> have a nice
> 
> static void rom_free_data(Rom *rom)
> {
>     if (rom->mapped_file) {
>         g_mapped_file_unref(rom->mapped_file);
>         rom->mapped_file = NULL;
>     } else {
>         g_free(rom->data);
>     }
>     rom->data = NULL;
> }
> 
> that is called from both rom_free and rom_reset.

I'll add this in v3.

Thanks,
Stefano


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

* Re: [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load
  2019-07-23 14:57     ` Stefano Garzarella
@ 2019-07-23 15:00       ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-07-23 15:00 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Peter Maydell, Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin,
	Julio Montes, Dr . David Alan Gilbert, qemu-devel,
	Richard Henderson

On 23/07/19 16:57, Stefano Garzarella wrote:
> On Tue, Jul 23, 2019 at 04:33:44PM +0200, Paolo Bonzini wrote:
>> Since this is the only call to rom_add_elf_program, what about adding a
>> GMappedFile* field to struct Rom and passing it here instead of
>> data+file_size?
> 
> Ehm, we currently use a subsection of the mmapped file as a ROM.
> Should we keep the data+file_size parameter? (plus the new GMappedFile*)

Yes, either that or an offset/file_size.

> At this point, is it better to call 'g_mapped_file_ref(gmf)' in the
> 'rom_add_elf_program()'?

Yes, of course.

Paolo


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

* Re: [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd
  2019-07-23 14:04 [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Stefano Garzarella
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load Stefano Garzarella
  2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd Stefano Garzarella
@ 2019-07-23 17:37 ` Montes, Julio
  2019-07-24  7:36   ` Stefano Garzarella
  2 siblings, 1 reply; 12+ messages in thread
From: Montes, Julio @ 2019-07-23 17:37 UTC (permalink / raw)
  To: sgarzare, qemu-devel
  Cc: peter.maydell, ehabkost, slp, mst, dgilbert, pbonzini, rth

Stefano, Brilliant job!

I can confirm that with these patches the memory footprint is smaller
and the boot time is the same for kata

Here the results using kata metrics

https://pasteboard.co/Ipl06Q0.png
https://pasteboard.co/Ipl3p4d.png

Thanks

-
Julio


On Tue, 2019-07-23 at 16:04 +0200, Stefano Garzarella wrote:
> In order to reduce the memory footprint when PVH kernel and initrd
> are used, we map them into memory instead of reading them.
> In this way we can share them between multiple instances of QEMU.
> 
> v2:
> - Patch 1: used g_mapped_file_new_from_fd() with 'writeble' set to
> 'true',
>            since we can modify the mapped buffer. [Paolo, Peter]
> 
> These are the results using a PVH kernel and initrd (cpio):
> - memory footprint (using smem) [MB]
>         QEMU              before                   now
>     # instances        USS      PSS            USS      PSS
>          1           102.0M   105.8M         102.3M   106.2M
>          2            94.6M   101.2M          72.3M    90.1M
>          4            94.1M    98.0M          72.0M    81.5M
>          8            94.0M    96.2M          71.8M    76.9M
>         16            93.9M    95.1M          71.6M    74.3M
> 
>     Initrd size: 3.0M
>     Kernel
>         image size: 28M
>         sections size [size -A -d vmlinux]:  18.9M
> 
> - boot time [ms]
>                           before                   now
>  qemu_init_end:           63.85                   55.91
>  linux_start_kernel:      82.11 (+18.26)          74.51 (+18.60)
>  linux_start_user:       169.94 (+87.83)         159.06 (+84.56)
> 
> QEMU command used:
> ./qemu-system-x86_64 -bios /path/to/seabios/out/bios.bin -no-hpet \
>     -machine
> q35,accel=kvm,kernel_irqchip,nvdimm,sata=off,smbus=off,vmport=off \
>     -cpu host -m 1G -smp 1 -vga none -display none -no-user-config
> -nodefaults \
>     -kernel /path/to/vmlinux -initrd /path/to/rootfs.cpio \
>     -append 'root=/dev/mem0 ro console=hvc0 pci=lastbus=0 nosmap'
> 
> Stefano Garzarella (2):
>   elf-ops.h: Map into memory the ELF to load
>   hw/i386/pc: Map into memory the initrd
> 
>  hw/i386/pc.c         | 15 ++++++++---
>  include/hw/elf_ops.h | 64 ++++++++++++++++++++++++----------------
> ----
>  2 files changed, 46 insertions(+), 33 deletions(-)
> 

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

* Re: [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd
  2019-07-23 17:37 ` [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Montes, Julio
@ 2019-07-24  7:36   ` Stefano Garzarella
  2019-07-24 13:03     ` Montes, Julio
  0 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-24  7:36 UTC (permalink / raw)
  To: Montes, Julio
  Cc: peter.maydell, ehabkost, slp, mst, qemu-devel, dgilbert, pbonzini, rth

On Tue, Jul 23, 2019 at 05:37:18PM +0000, Montes, Julio wrote:
> Stefano, Brilliant job!
> 
> I can confirm that with these patches the memory footprint is smaller
> and the boot time is the same for kata
> 
> Here the results using kata metrics
> 
> https://pasteboard.co/Ipl06Q0.png
> https://pasteboard.co/Ipl3p4d.png
> 

Hi Julio,
thank you very much for testing :)

When you measure the PPS, how many QEMU instances did you start?
Did you use also the initrd?

Thanks,
Stefano


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

* Re: [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd
  2019-07-24  7:36   ` Stefano Garzarella
@ 2019-07-24 13:03     ` Montes, Julio
  2019-07-24 13:25       ` Stefano Garzarella
  0 siblings, 1 reply; 12+ messages in thread
From: Montes, Julio @ 2019-07-24 13:03 UTC (permalink / raw)
  To: sgarzare
  Cc: peter.maydell, ehabkost, slp, mst, qemu-devel, dgilbert, pbonzini, rth

Hi Stefano

On Wed, 2019-07-24 at 09:36 +0200, Stefano Garzarella wrote:
> On Tue, Jul 23, 2019 at 05:37:18PM +0000, Montes, Julio wrote:
> > Stefano, Brilliant job!
> > 
> > I can confirm that with these patches the memory footprint is
> > smaller
> > and the boot time is the same for kata
> > 
> > Here the results using kata metrics
> > 
> > https://pasteboard.co/Ipl06Q0.png
> > https://pasteboard.co/Ipl3p4d.png
> > 
> 
> Hi Julio,
> thank you very much for testing :)
> 
> When you measure the PPS, how many QEMU instances did you start?
> Did you use also the initrd?

50 VMs with a nvdimm/pmem device as rootfs, I will test your v3 with
initrd :)

> 
> Thanks,
> Stefano

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

* Re: [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd
  2019-07-24 13:03     ` Montes, Julio
@ 2019-07-24 13:25       ` Stefano Garzarella
  0 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-07-24 13:25 UTC (permalink / raw)
  To: Montes, Julio
  Cc: peter.maydell, ehabkost, slp, mst, qemu-devel, dgilbert, pbonzini, rth

On Wed, Jul 24, 2019 at 01:03:42PM +0000, Montes, Julio wrote:
> Hi Stefano
> 
> On Wed, 2019-07-24 at 09:36 +0200, Stefano Garzarella wrote:
> > On Tue, Jul 23, 2019 at 05:37:18PM +0000, Montes, Julio wrote:
> > > Stefano, Brilliant job!
> > > 
> > > I can confirm that with these patches the memory footprint is
> > > smaller
> > > and the boot time is the same for kata
> > > 
> > > Here the results using kata metrics
> > > 
> > > https://pasteboard.co/Ipl06Q0.png
> > > https://pasteboard.co/Ipl3p4d.png
> > > 
> > 
> > Hi Julio,
> > thank you very much for testing :)
> > 
> > When you measure the PPS, how many QEMU instances did you start?
> > Did you use also the initrd?
> 
> 50 VMs with a nvdimm/pmem device as rootfs, I will test your v3 with
> initrd :)

Cool :)

Okay, I'm sending a v4 to fix some issues...

Thanks,
Stefano


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

end of thread, other threads:[~2019-07-24 13:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23 14:04 [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Stefano Garzarella
2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 1/2] elf-ops.h: Map into memory the ELF to load Stefano Garzarella
2019-07-23 14:33   ` Paolo Bonzini
2019-07-23 14:57     ` Stefano Garzarella
2019-07-23 15:00       ` Paolo Bonzini
2019-07-23 14:04 ` [Qemu-devel] [PATCH v2 2/2] hw/i386/pc: Map into memory the initrd Stefano Garzarella
2019-07-23 14:30   ` Paolo Bonzini
2019-07-23 14:47     ` Stefano Garzarella
2019-07-23 17:37 ` [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd Montes, Julio
2019-07-24  7:36   ` Stefano Garzarella
2019-07-24 13:03     ` Montes, Julio
2019-07-24 13:25       ` Stefano Garzarella

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.