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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 07/03/2014 07:09, Hu Tao ha scritto:
> 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 <pbonzini@redhat.com>
>> ---
>>  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);
>       ...
>   }

ram_block_add is still calling phys_mem_alloc:

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

Paolo

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