Patchwork exec: remove code duplication in qemu_ram_alloc() and qemu_ram_alloc_from_ptr()

login
register
mail settings
Submitter Yoshiaki Tamura
Date Aug. 18, 2010, 6:41 a.m.
Message ID <1282113709-3010-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/62010/
State New
Headers show

Comments

Yoshiaki Tamura - Aug. 18, 2010, 6:41 a.m.
Since most of the code in qemu_ram_alloc() and
qemu_ram_alloc_from_ptr() are duplicated, let
qemu_ram_alloc_from_ptr() to switch by checking void *host, and change
qemu_ram_alloc() to a wrapper.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 exec.c |   84 +++++++++++++++++++--------------------------------------------
 1 files changed, 26 insertions(+), 58 deletions(-)
Alex Williamson - Aug. 18, 2010, 1:45 p.m.
On Wed, 2010-08-18 at 15:41 +0900, Yoshiaki Tamura wrote:
> Since most of the code in qemu_ram_alloc() and
> qemu_ram_alloc_from_ptr() are duplicated, let
> qemu_ram_alloc_from_ptr() to switch by checking void *host, and change
> qemu_ram_alloc() to a wrapper.
> 
> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> ---
>  exec.c |   84 +++++++++++++++++++--------------------------------------------
>  1 files changed, 26 insertions(+), 58 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 4fc96cb..82bfffc 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2809,7 +2809,7 @@ static ram_addr_t last_ram_offset(void)
>  }
>  
>  ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
> -                        ram_addr_t size, void *host)
> +                                   ram_addr_t size, void *host)
>  {
>      RAMBlock *new_block, *block;
>  
> @@ -2833,74 +2833,37 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
>          }
>      }
>  
> -    new_block->host = host;
> -
> -    new_block->offset = find_ram_offset(size);
> -    new_block->length = size;
> -
> -    QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
> -
> -    ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
> -                                       last_ram_offset() >> TARGET_PAGE_BITS);
> -    memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
> -           0xff, size >> TARGET_PAGE_BITS);
> -
> -    if (kvm_enabled())
> -        kvm_setup_guest_memory(new_block->host, size);
> -
> -    return new_block->offset;
> -}
> -
> -ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
> -{
> -    RAMBlock *new_block, *block;
> -
> -    size = TARGET_PAGE_ALIGN(size);
> -    new_block = qemu_mallocz(sizeof(*new_block));
> -
> -    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> -        if (id) {
> -            snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
> -            qemu_free(id);
> -        }
> -    }
> -    pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
> -
> -    QLIST_FOREACH(block, &ram_list.blocks, next) {
> -        if (!strcmp(block->idstr, new_block->idstr)) {
> -            fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
> -                    new_block->idstr);
> -            abort();
> -        }
> -    }
> -
> -    if (mem_path) {
> +    if (host) {
> +        new_block->host = host;
> +    } else {
> +        if (mem_path) {
>  #if defined (__linux__) && !defined(TARGET_S390X)
> -        new_block->host = file_ram_alloc(new_block, size, mem_path);
> -        if (!new_block->host) {
> -            new_block->host = qemu_vmalloc(size);
> +            new_block->host = file_ram_alloc(new_block, size, mem_path);
> +            if (!new_block->host) {
> +                new_block->host = qemu_vmalloc(size);
>  #ifdef MADV_MERGEABLE
> -            madvise(new_block->host, size, MADV_MERGEABLE);
> +                madvise(new_block->host, size, MADV_MERGEABLE);
>  #endif
> -        }
> +            }
>  #else
> -        fprintf(stderr, "-mem-path option unsupported\n");
> -        exit(1);
> +            fprintf(stderr, "-mem-path option unsupported\n");
> +            exit(1);
>  #endif
> -    } else {
> +        } else {
>  #if defined(TARGET_S390X) && defined(CONFIG_KVM)
> -        /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */
> -        new_block->host = mmap((void*)0x1000000, size,
> -                                PROT_EXEC|PROT_READ|PROT_WRITE,
> -                                MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +            /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */
> +            new_block->host = mmap((void*)0x1000000, size,
> +                                   PROT_EXEC|PROT_READ|PROT_WRITE,
> +                                   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>  #else
> -        new_block->host = qemu_vmalloc(size);
> +            new_block->host = qemu_vmalloc(size);
>  #endif
>  #ifdef MADV_MERGEABLE
> -        madvise(new_block->host, size, MADV_MERGEABLE);
> +            madvise(new_block->host, size, MADV_MERGEABLE);
>  #endif
> +        }
>      }
> +
>      new_block->offset = find_ram_offset(size);
>      new_block->length = size;
>  
> @@ -2917,6 +2880,11 @@ ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
>      return new_block->offset;
>  }
>  
> +ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
> +{
> +    return qemu_ram_alloc_from_ptr(dev, name, size, NULL);
> +}
> +
>  void qemu_ram_free(ram_addr_t addr)
>  {
>      RAMBlock *block;
Anthony Liguori - Aug. 22, 2010, 9:30 p.m.
On 08/18/2010 01:41 AM, Yoshiaki Tamura wrote:
> Since most of the code in qemu_ram_alloc() and
> qemu_ram_alloc_from_ptr() are duplicated, let
> qemu_ram_alloc_from_ptr() to switch by checking void *host, and change
> qemu_ram_alloc() to a wrapper.
>
> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   exec.c |   84 +++++++++++++++++++--------------------------------------------
>   1 files changed, 26 insertions(+), 58 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 4fc96cb..82bfffc 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2809,7 +2809,7 @@ static ram_addr_t last_ram_offset(void)
>   }
>
>   ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
> -                        ram_addr_t size, void *host)
> +                                   ram_addr_t size, void *host)
>   {
>       RAMBlock *new_block, *block;
>
> @@ -2833,74 +2833,37 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
>           }
>       }
>
> -    new_block->host = host;
> -
> -    new_block->offset = find_ram_offset(size);
> -    new_block->length = size;
> -
> -    QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
> -
> -    ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
> -                                       last_ram_offset()>>  TARGET_PAGE_BITS);
> -    memset(ram_list.phys_dirty + (new_block->offset>>  TARGET_PAGE_BITS),
> -           0xff, size>>  TARGET_PAGE_BITS);
> -
> -    if (kvm_enabled())
> -        kvm_setup_guest_memory(new_block->host, size);
> -
> -    return new_block->offset;
> -}
> -
> -ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
> -{
> -    RAMBlock *new_block, *block;
> -
> -    size = TARGET_PAGE_ALIGN(size);
> -    new_block = qemu_mallocz(sizeof(*new_block));
> -
> -    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> -        if (id) {
> -            snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
> -            qemu_free(id);
> -        }
> -    }
> -    pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
> -
> -    QLIST_FOREACH(block,&ram_list.blocks, next) {
> -        if (!strcmp(block->idstr, new_block->idstr)) {
> -            fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
> -                    new_block->idstr);
> -            abort();
> -        }
> -    }
> -
> -    if (mem_path) {
> +    if (host) {
> +        new_block->host = host;
> +    } else {
> +        if (mem_path) {
>   #if defined (__linux__)&&  !defined(TARGET_S390X)
> -        new_block->host = file_ram_alloc(new_block, size, mem_path);
> -        if (!new_block->host) {
> -            new_block->host = qemu_vmalloc(size);
> +            new_block->host = file_ram_alloc(new_block, size, mem_path);
> +            if (!new_block->host) {
> +                new_block->host = qemu_vmalloc(size);
>   #ifdef MADV_MERGEABLE
> -            madvise(new_block->host, size, MADV_MERGEABLE);
> +                madvise(new_block->host, size, MADV_MERGEABLE);
>   #endif
> -        }
> +            }
>   #else
> -        fprintf(stderr, "-mem-path option unsupported\n");
> -        exit(1);
> +            fprintf(stderr, "-mem-path option unsupported\n");
> +            exit(1);
>   #endif
> -    } else {
> +        } else {
>   #if defined(TARGET_S390X)&&  defined(CONFIG_KVM)
> -        /* XXX S390 KVM requires the topmost vma of the RAM to be<  256GB */
> -        new_block->host = mmap((void*)0x1000000, size,
> -                                PROT_EXEC|PROT_READ|PROT_WRITE,
> -                                MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +            /* XXX S390 KVM requires the topmost vma of the RAM to be<  256GB */
> +            new_block->host = mmap((void*)0x1000000, size,
> +                                   PROT_EXEC|PROT_READ|PROT_WRITE,
> +                                   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>   #else
> -        new_block->host = qemu_vmalloc(size);
> +            new_block->host = qemu_vmalloc(size);
>   #endif
>   #ifdef MADV_MERGEABLE
> -        madvise(new_block->host, size, MADV_MERGEABLE);
> +            madvise(new_block->host, size, MADV_MERGEABLE);
>   #endif
> +        }
>       }
> +
>       new_block->offset = find_ram_offset(size);
>       new_block->length = size;
>
> @@ -2917,6 +2880,11 @@ ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
>       return new_block->offset;
>   }
>
> +ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
> +{
> +    return qemu_ram_alloc_from_ptr(dev, name, size, NULL);
> +}
> +
>   void qemu_ram_free(ram_addr_t addr)
>   {
>       RAMBlock *block;
>

Patch

diff --git a/exec.c b/exec.c
index 4fc96cb..82bfffc 100644
--- a/exec.c
+++ b/exec.c
@@ -2809,7 +2809,7 @@  static ram_addr_t last_ram_offset(void)
 }
 
 ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
-                        ram_addr_t size, void *host)
+                                   ram_addr_t size, void *host)
 {
     RAMBlock *new_block, *block;
 
@@ -2833,74 +2833,37 @@  ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
         }
     }
 
-    new_block->host = host;
-
-    new_block->offset = find_ram_offset(size);
-    new_block->length = size;
-
-    QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
-
-    ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
-                                       last_ram_offset() >> TARGET_PAGE_BITS);
-    memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
-           0xff, size >> TARGET_PAGE_BITS);
-
-    if (kvm_enabled())
-        kvm_setup_guest_memory(new_block->host, size);
-
-    return new_block->offset;
-}
-
-ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
-{
-    RAMBlock *new_block, *block;
-
-    size = TARGET_PAGE_ALIGN(size);
-    new_block = qemu_mallocz(sizeof(*new_block));
-
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
-        if (id) {
-            snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
-            qemu_free(id);
-        }
-    }
-    pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
-
-    QLIST_FOREACH(block, &ram_list.blocks, next) {
-        if (!strcmp(block->idstr, new_block->idstr)) {
-            fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
-                    new_block->idstr);
-            abort();
-        }
-    }
-
-    if (mem_path) {
+    if (host) {
+        new_block->host = host;
+    } else {
+        if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
-        new_block->host = file_ram_alloc(new_block, size, mem_path);
-        if (!new_block->host) {
-            new_block->host = qemu_vmalloc(size);
+            new_block->host = file_ram_alloc(new_block, size, mem_path);
+            if (!new_block->host) {
+                new_block->host = qemu_vmalloc(size);
 #ifdef MADV_MERGEABLE
-            madvise(new_block->host, size, MADV_MERGEABLE);
+                madvise(new_block->host, size, MADV_MERGEABLE);
 #endif
-        }
+            }
 #else
-        fprintf(stderr, "-mem-path option unsupported\n");
-        exit(1);
+            fprintf(stderr, "-mem-path option unsupported\n");
+            exit(1);
 #endif
-    } else {
+        } else {
 #if defined(TARGET_S390X) && defined(CONFIG_KVM)
-        /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */
-        new_block->host = mmap((void*)0x1000000, size,
-                                PROT_EXEC|PROT_READ|PROT_WRITE,
-                                MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+            /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */
+            new_block->host = mmap((void*)0x1000000, size,
+                                   PROT_EXEC|PROT_READ|PROT_WRITE,
+                                   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 #else
-        new_block->host = qemu_vmalloc(size);
+            new_block->host = qemu_vmalloc(size);
 #endif
 #ifdef MADV_MERGEABLE
-        madvise(new_block->host, size, MADV_MERGEABLE);
+            madvise(new_block->host, size, MADV_MERGEABLE);
 #endif
+        }
     }
+
     new_block->offset = find_ram_offset(size);
     new_block->length = size;
 
@@ -2917,6 +2880,11 @@  ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
     return new_block->offset;
 }
 
+ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size)
+{
+    return qemu_ram_alloc_from_ptr(dev, name, size, NULL);
+}
+
 void qemu_ram_free(ram_addr_t addr)
 {
     RAMBlock *block;