Re: [Qemu-devel] [PATCH 2.1 16/28] memory: reorganize file-based allocation

[Hu Tao]

On Fri, Mar 07, 2014 at 02:09:25PM +0800, Hu Tao wrote:
> On Tue, Mar 04, 2014 at 03:00:44PM +0100, Paolo Bonzini wrote:
> > Split the internal interface in exec.c to a separate function, and
> > push the check on mem_path up to memory_region_init_ram.
> > 
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> >  exec.c                  | 105 
> > +++++++++++++++++++++++++++++-------------------
> >  include/exec/cpu-all.h  |   3 --
> >  include/exec/ram_addr.h |   2 +
> >  include/sysemu/sysemu.h |   2 +
> >  memory.c                |   7 +++-
> >  5 files changed, 73 insertions(+), 46 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index b69fd29..0aa4947 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1240,56 +1240,30 @@ static int memory_try_enable_merging(void *addr, 
> > size_t len)
> >      return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE);
> >  }
> >  
> > -ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> > -                                   MemoryRegion *mr)
> > +static ram_addr_t ram_block_add(RAMBlock *new_block)
> >  {
> > -    RAMBlock *block, *new_block;
> > +    RAMBlock *block;
> >      ram_addr_t old_ram_size, new_ram_size;
> >  
> >      old_ram_size = last_ram_offset() >> TARGET_PAGE_BITS;
> >  
> > -    size = TARGET_PAGE_ALIGN(size);
> > -    new_block = g_malloc0(sizeof(*new_block));
> > -    new_block->fd = -1;
> > -
> >      /* This assumes the iothread lock is taken here too.  */
> >      qemu_mutex_lock_ramlist();
> > -    new_block->mr = mr;
> > -    new_block->offset = find_ram_offset(size);
> > -    if (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");
> > -            exit(1);
> > -        }
> > -        xen_ram_alloc(new_block->offset, size, mr);
> > -    } else {
> > -        if (mem_path) {
> > -            if (phys_mem_alloc != qemu_anon_ram_alloc) {
> > -                /*
> > -                 * file_ram_alloc() needs to allocate just like
> > -                 * phys_mem_alloc, but we haven't bothered to provide
> > -                 * a hook there.
> > -                 */
> > -                fprintf(stderr,
> > -                        "-mem-path not supported with this accelerator\n");
> > -                exit(1);
> > -            }
> > -            new_block->host = file_ram_alloc(new_block, size, mem_path);
> > -        }
> > -        if (!new_block->host) {
> > -            new_block->host = phys_mem_alloc(size);
> > +    new_block->offset = find_ram_offset(new_block->length);
> > +
> > +    if (!new_block->host) {
> > +        if (xen_enabled()) {
> > +            xen_ram_alloc(new_block->offset, new_block->length, 
> > new_block->mr);
> > +        } else {
> > +            new_block->host = phys_mem_alloc(new_block->length);
> >              if (!new_block->host) {
> >                  fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> >                          new_block->mr->name, strerror(errno));
> >                  exit(1);
> >              }
> > -            memory_try_enable_merging(new_block->host, size);
> > +            memory_try_enable_merging(new_block->host, new_block->length);
> >          }
> >      }
> > -    new_block->length = size;
> >  
> >      /* Keep the list sorted from biggest to smallest block.  */
> >      QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> > @@ -1317,18 +1291,65 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
> > void *host,
> >                                     old_ram_size, new_ram_size);
> >         }
> >      }
> > -    cpu_physical_memory_set_dirty_range(new_block->offset, size);
> > +    cpu_physical_memory_set_dirty_range(new_block->offset, 
> > new_block->length);
> >  
> > -    qemu_ram_setup_dump(new_block->host, size);
> > -    qemu_madvise(new_block->host, size, QEMU_MADV_HUGEPAGE);
> > -    qemu_madvise(new_block->host, size, QEMU_MADV_DONTFORK);
> > +    qemu_ram_setup_dump(new_block->host, new_block->length);
> > +    qemu_madvise(new_block->host, new_block->length, QEMU_MADV_HUGEPAGE);
> > +    qemu_madvise(new_block->host, new_block->length, QEMU_MADV_DONTFORK);
> >  
> > -    if (kvm_enabled())
> > -        kvm_setup_guest_memory(new_block->host, size);
> > +    if (kvm_enabled()) {
> > +        kvm_setup_guest_memory(new_block->host, new_block->length);
> > +    }
> >  
> >      return new_block->offset;
> >  }
> >  
> > +ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> > +                                    const char *mem_path)
> > +{
> > +    RAMBlock *new_block;
> > +
> > +    if (xen_enabled()) {
> > +        fprintf(stderr, "-mem-path not supported with Xen\n");
> > +        exit(1);
> > +    }
> > +
> > +    if (phys_mem_alloc != qemu_anon_ram_alloc) {
> > +        /*
> > +         * file_ram_alloc() needs to allocate just like
> > +         * phys_mem_alloc, but we haven't bothered to provide
> > +         * a hook there.
> > +         */
> > +        fprintf(stderr,
> > +                "-mem-path not supported with this accelerator\n");
> > +        exit(1);
> > +    }
> > +
> > +    size = TARGET_PAGE_ALIGN(size);
> > +    new_block = g_malloc0(sizeof(*new_block));
> > +    new_block->mr = mr;
> > +    new_block->length = size;
> > +    new_block->host = file_ram_alloc(new_block, size, mem_path);
> > +    return ram_block_add(new_block);
> > +}
> > +
> > +ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> > +                                   MemoryRegion *mr)
> > +{
> > +    RAMBlock *new_block;
> > +
> > +    size = TARGET_PAGE_ALIGN(size);
> > +    new_block = g_malloc0(sizeof(*new_block));
> > +    new_block->mr = mr;
> > +    new_block->length = size;
> > +    new_block->fd = -1;
> > +    new_block->host = host;
> > +    if (host) {
> > +        new_block->flags |= RAM_PREALLOC_MASK;
> > +    }
> > +    return ram_block_add(new_block);
> > +}
> > +
> >  ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr)
> >  {
> >      return qemu_ram_alloc_from_ptr(size, NULL, mr);
> > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> > index e66ab5b..b44babb 100644
> > --- a/include/exec/cpu-all.h
> > +++ b/include/exec/cpu-all.h
> > @@ -466,9 +466,6 @@ typedef struct RAMList {
> >  } RAMList;
> >  extern RAMList ram_list;
> >  
> > -extern const char *mem_path;
> > -extern int mem_prealloc;
> > -
> >  /* Flags stored in the low bits of the TLB virtual address.  These are
> >     defined so that fast path ram access is all zeros.  */
> >  /* Zero if TLB entry is valid.  */
> > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > index 2edfa96..dedb258 100644
> > --- a/include/exec/ram_addr.h
> > +++ b/include/exec/ram_addr.h
> > @@ -22,6 +22,8 @@
> >  #ifndef CONFIG_USER_ONLY
> >  #include "hw/xen/xen.h"
> >  
> > +ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> > +                                    const char *mem_path);
> >  ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> >                                     MemoryRegion *mr);
> >  ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 4870129..03f5ee5 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -132,6 +132,8 @@ extern uint8_t *boot_splash_filedata;
> >  extern size_t boot_splash_filedata_size;
> >  extern uint8_t qemu_extra_params_fw[2];
> >  extern QEMUClockType rtc_clock;
> > +extern const char *mem_path;
> > +extern int mem_prealloc;
> >  
> >  #define MAX_NODES 128
> >  #define MAX_CPUMASK_BITS 255
> > diff --git a/memory.c b/memory.c
> > index 59ecc28..32b17a8 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -23,6 +23,7 @@
> >  
> >  #include "exec/memory-internal.h"
> >  #include "exec/ram_addr.h"
> > +#include "sysemu/sysemu.h"
> >  
> >  //#define DEBUG_UNASSIGNED
> >  
> > @@ -1016,7 +1017,11 @@ void memory_region_init_ram(MemoryRegion *mr,
> >      mr->ram = true;
> >      mr->terminates = true;
> >      mr->destructor = memory_region_destructor_ram;
> > -    mr->ram_addr = qemu_ram_alloc(size, mr);
> > +    if (mem_path) {
> > +        mr->ram_addr = qemu_ram_alloc_from_file(size, mr, mem_path);
> > +    } else {
> > +        mr->ram_addr = qemu_ram_alloc(size, mr);
> > +    }
> >  }
> 
> This changes the logic of the original code:
> 
>   if (mem_path) {
>       ...
>       new_block->host = file_ram_alloc(new_block, size, mem_path);
>   }
>   if (!new_block->host) {
>       new_block->host = phys_mem_alloc(size);
>       ...
>   }

Never mind. I'm now at patch 18.