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

[Hu Tao]

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);
      ...
  }

>  
>  void memory_region_init_ram_ptr(MemoryRegion *mr,
> -- 
> 1.8.5.3
>