RAM cleanup

PATCH In review Alex Williamson 1 participant 4 messages Jun 11, 2010 KVM

Share & tools

Alex Williamson

author

2010-06-11 14:15 UTC #0 lore raw

cover RAM cleanup

These are the first two patches from the RFC sent out a couple days ago. There seemed to be concensus that these were headed in the right direction, so I'll post them for commit while we decided how to name RAMBlocks.

Changes - rename 'ram' to 'ram_list' to avoid polluting the global namespace quite so much, restore the error message and abort for bad address in qemu_get_ram_ptr().

Alex

---

Alex Williamson (2):
ram_blocks: Convert to a QLIST
qemu_ram_alloc: Remove duplicate code

 arch_init.c |   14 ++++---
 cpu-all.h   |   28 +++++++++++----
 exec.c      |  112 ++++++++++++++++++-----------------------------------------
 3 files changed, 62 insertions(+), 92 deletions(-)

Alex Williamson

author

2010-06-11 14:16 UTC #1 lore raw

PATCH 1/2 qemu_ram_alloc: Remove duplicate code

No reason not to call qemu_ram_map() once we have the allocation and remove duplicate code.

Signed-off-by Alex Williamson <[email protected]>

Acked-by Chris Wright <[email protected]>

---

 exec.c |   37 ++++++++++---------------------------
 1 files changed, 10 insertions(+), 27 deletions(-)

exec.c +10 ~~-27~~

index 7b0e1c5..c60f9e7 100644
@@ -2816,18 +2816,17 @@ ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
 
 ram_addr_t qemu_ram_alloc(ram_addr_t size)
 {
-    RAMBlock *new_block;
+    void *host;
 
     size = TARGET_PAGE_ALIGN(size);
-    new_block = qemu_malloc(sizeof(*new_block));
 
     if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
-        new_block->host = file_ram_alloc(size, mem_path);
-        if (!new_block->host) {
-            new_block->host = qemu_vmalloc(size);
+        host = file_ram_alloc(size, mem_path);
+        if (!host) {
+            host = qemu_vmalloc(size);
 #ifdef MADV_MERGEABLE
-            madvise(new_block->host, size, MADV_MERGEABLE);
+            madvise(host, size, MADV_MERGEABLE);
 #endif
         }
 #else

@@ -2837,33 +2836,17 @@ ram_addr_t qemu_ram_alloc(ram_addr_t size)
     } 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);
+        host = mmap((void*)0x1000000, size, PROT_EXEC|PROT_READ|PROT_WRITE,
+                    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 #else
-        new_block->host = qemu_vmalloc(size);
+        host = qemu_vmalloc(size);
 #endif
 #ifdef MADV_MERGEABLE
-        madvise(new_block->host, size, MADV_MERGEABLE);
+        madvise(host, size, MADV_MERGEABLE);
 #endif
     }
-    new_block->offset = last_ram_offset;
-    new_block->length = size;
-
-    new_block->next = ram_blocks;
-    ram_blocks = new_block;
-
-    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
-        (last_ram_offset + size) >> TARGET_PAGE_BITS);
-    memset(phys_ram_dirty + (last_ram_offset >> TARGET_PAGE_BITS),
-           0xff, size >> TARGET_PAGE_BITS);
-
-    last_ram_offset += size;
 
-    if (kvm_enabled())
-        kvm_setup_guest_memory(new_block->host, size);
-
-    return new_block->offset;
+    return qemu_ram_map(size, host);
 }
 
 void qemu_ram_free(ram_addr_t addr)

Alex Williamson

author

2010-06-11 14:16 UTC #2 lore raw

PATCH 2/2 ram_blocks: Convert to a QLIST

In reply to Alex Williamson

This makes the RAM block list easier to manipulate. Also incorporate relevant variables into the RAMList struct.

Signed-off-by Alex Williamson <[email protected]>

Acked-by Chris Wright <[email protected]>

---

 arch_init.c |   14 ++++++-----
 cpu-all.h   |   28 ++++++++++++++++------
 exec.c      |   75 ++++++++++++++++++++---------------------------------------
 3 files changed, 52 insertions(+), 65 deletions(-)

arch_init.c +7 -7

index 8e849a8..eb5b67c 100644
@@ -110,7 +110,7 @@ static int ram_save_block(QEMUFile *f)
     ram_addr_t addr = 0;
     int bytes_sent = 0;
 
-    while (addr < last_ram_offset) {
+    while (addr < ram_list.last_offset) {
         if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
             uint8_t *p;

@@ -133,7 +133,7 @@ static int ram_save_block(QEMUFile *f)
             break;
         }
         addr += TARGET_PAGE_SIZE;
-        current_addr = (saved_addr + addr) % last_ram_offset;
+        current_addr = (saved_addr + addr) % ram_list.last_offset;
     }
 
     return bytes_sent;

@@ -146,7 +146,7 @@ static ram_addr_t ram_save_remaining(void)
     ram_addr_t addr;
     ram_addr_t count = 0;
 
-    for (addr = 0; addr < last_ram_offset; addr += TARGET_PAGE_SIZE) {
+    for (addr = 0; addr < ram_list.last_offset; addr += TARGET_PAGE_SIZE) {
         if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
             count++;
         }

@@ -167,7 +167,7 @@ uint64_t ram_bytes_transferred(void)
 
 uint64_t ram_bytes_total(void)
 {
-    return last_ram_offset;
+    return ram_list.last_offset;
 }
 
 int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)

@@ -191,7 +191,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
         bytes_transferred = 0;
 
         /* Make sure all dirty bits are set */
-        for (addr = 0; addr < last_ram_offset; addr += TARGET_PAGE_SIZE) {
+        for (addr = 0; addr < ram_list.last_offset; addr += TARGET_PAGE_SIZE) {
             if (!cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
                 cpu_physical_memory_set_dirty(addr);
             }

@@ -200,7 +200,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
         /* Enable dirty memory tracking */
         cpu_physical_memory_set_dirty_tracking(1);
 
-        qemu_put_be64(f, last_ram_offset | RAM_SAVE_FLAG_MEM_SIZE);
+        qemu_put_be64(f, ram_list.last_offset | RAM_SAVE_FLAG_MEM_SIZE);
     }
 
     bytes_transferred_last = bytes_transferred;

@@ -259,7 +259,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
         addr &= TARGET_PAGE_MASK;
 
         if (flags & RAM_SAVE_FLAG_MEM_SIZE) {
-            if (addr != last_ram_offset) {
+            if (addr != ram_list.last_offset) {
                 return -EINVAL;
             }
         }

cpu-all.h +20 -8

index 77eaf85..e31c2de 100644
@@ -859,9 +859,21 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr);
 /* memory API */
 
 extern int phys_ram_fd;
-extern uint8_t *phys_ram_dirty;
 extern ram_addr_t ram_size;
-extern ram_addr_t last_ram_offset;
+
+typedef struct RAMBlock {
+    uint8_t *host;
+    ram_addr_t offset;
+    ram_addr_t length;
+    QLIST_ENTRY(RAMBlock) next;
+} RAMBlock;
+
+typedef struct RAMList {
+    uint8_t *phys_dirty;
+    ram_addr_t last_offset;
+    QLIST_HEAD(ram, RAMBlock) blocks;
+} RAMList;
+extern RAMList ram_list;
 
 extern const char *mem_path;
 extern int mem_prealloc;

@@ -891,29 +903,29 @@ extern int mem_prealloc;
 /* read dirty bit (return 0 or 1) */
 static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
 {
-    return phys_ram_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
+    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
 }
 
 static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
 {
-    return phys_ram_dirty[addr >> TARGET_PAGE_BITS];
+    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS];
 }
 
 static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
                                                 int dirty_flags)
 {
-    return phys_ram_dirty[addr >> TARGET_PAGE_BITS] & dirty_flags;
+    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & dirty_flags;
 }
 
 static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
 {
-    phys_ram_dirty[addr >> TARGET_PAGE_BITS] = 0xff;
+    ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] = 0xff;
 }
 
 static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
                                                       int dirty_flags)
 {
-    return phys_ram_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
+    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
 }
 
 static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,

@@ -925,7 +937,7 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
 
     len = length >> TARGET_PAGE_BITS;
     mask = ~dirty_flags;
-    p = phys_ram_dirty + (start >> TARGET_PAGE_BITS);
+    p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
     for (i = 0; i < len; i++) {
         p[i] &= mask;
     }

exec.c +25 ~~-50~~

110 lines not on this page · raw message

Alex Williamson

author

2010-06-11 17:34 UTC #3 lore raw

In reply to Alex Williamson

Quoted 9 lines · Alex Williamson

These are the first two patches from the RFC sent out a couple days ago. There seemed to be concensus that these were headed in the right direction, so I'll post them for commit while we decided how to name RAMBlocks.
Changes - rename 'ram' to 'ram_list' to avoid polluting the global namespace quite so much, restore the error message and abort for bad address in qemu_get_ram_ptr().

Quoted 7 lines

Alex
---
Alex Williamson (2):
ram_blocks: Convert to a QLIST
qemu_ram_alloc: Remove duplicate code

Marcelo pointed out that I forgot to rebase against qemu, where qemu_ram_map doesn't exist yet. So I'll drop the first patch, we can do that in kvm later, and I'll repost the qemu version of the second.

Alex