qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate
@ 2016-02-29  2:37 Fam Zheng
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions Fam Zheng
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

The first four patches drop ram_addr from MemoryRegion on top of Gonglei's
optimization.

The next patch simplifies qemu_ram_free a bit by passing the RAMBlock pointer.

The last patch speeds up address_space_translate with a cache pointer inside
the AddressSpaceDispatch.



Fam Zheng (6):
  exec: Return RAMBlock pointer from allocating functions
  memory: Move assignment to ram_block to memory_region_init_*
  memory: Implement memory_region_get_ram_addr with mr->ram_block
  memory: Drop MemoryRegion.ram_addr
  exec: Pass RAMBlock pointer to qemu_ram_free
  exec: Introduce AddressSpaceDispatch.mru_section

 cputlb.c                |  4 +--
 exec.c                  | 93 ++++++++++++++++++++++++-------------------------
 hw/misc/ivshmem.c       |  9 +++--
 include/exec/memory.h   |  9 +----
 include/exec/ram_addr.h | 24 ++++++-------
 kvm-all.c               |  3 +-
 memory.c                | 56 ++++++++++++++++-------------
 7 files changed, 101 insertions(+), 97 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  2016-03-01  2:26   ` Gonglei (Arei)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_* Fam Zheng
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

Previously we return RAMBlock.offset; now return the pointer to the
whole structure.

ram_block_add returns void now, error is completely passed with errp.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c                  | 51 +++++++++++++++++++++----------------------------
 include/exec/ram_addr.h | 22 ++++++++++-----------
 memory.c                | 25 +++++++++++++++++++-----
 3 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/exec.c b/exec.c
index c62c439..2b14b79 100644
--- a/exec.c
+++ b/exec.c
@@ -1554,7 +1554,7 @@ static void dirty_memory_extend(ram_addr_t old_ram_size,
     }
 }
 
-static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
+static void ram_block_add(RAMBlock *new_block, Error **errp)
 {
     RAMBlock *block;
     RAMBlock *last_block = NULL;
@@ -1573,7 +1573,6 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
             if (err) {
                 error_propagate(errp, err);
                 qemu_mutex_unlock_ramlist();
-                return -1;
             }
         } else {
             new_block->host = phys_mem_alloc(new_block->max_length,
@@ -1583,7 +1582,6 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
                                  "cannot set up guest memory '%s'",
                                  memory_region_name(new_block->mr));
                 qemu_mutex_unlock_ramlist();
-                return -1;
             }
             memory_try_enable_merging(new_block->host, new_block->max_length);
         }
@@ -1631,22 +1629,19 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
             kvm_setup_guest_memory(new_block->host, new_block->max_length);
         }
     }
-
-    return new_block->offset;
 }
 
 #ifdef __linux__
-ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-                                    bool share, const char *mem_path,
-                                    Error **errp)
+RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
+                                   bool share, const char *mem_path,
+                                   Error **errp)
 {
     RAMBlock *new_block;
-    ram_addr_t addr;
     Error *local_err = NULL;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");
-        return -1;
+        return NULL;
     }
 
     if (phys_mem_alloc != qemu_anon_ram_alloc) {
@@ -1657,7 +1652,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
          */
         error_setg(errp,
                    "-mem-path not supported with this accelerator");
-        return -1;
+        return NULL;
     }
 
     size = HOST_PAGE_ALIGN(size);
@@ -1670,29 +1665,28 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                      mem_path, errp);
     if (!new_block->host) {
         g_free(new_block);
-        return -1;
+        return NULL;
     }
 
-    addr = ram_block_add(new_block, &local_err);
+    ram_block_add(new_block, &local_err);
     if (local_err) {
         g_free(new_block);
         error_propagate(errp, local_err);
-        return -1;
+        return NULL;
     }
-    return addr;
+    return new_block;
 }
 #endif
 
 static
-ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
-                                   void (*resized)(const char*,
-                                                   uint64_t length,
-                                                   void *host),
-                                   void *host, bool resizeable,
-                                   MemoryRegion *mr, Error **errp)
+RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
+                                  void (*resized)(const char*,
+                                                  uint64_t length,
+                                                  void *host),
+                                  void *host, bool resizeable,
+                                  MemoryRegion *mr, Error **errp)
 {
     RAMBlock *new_block;
-    ram_addr_t addr;
     Error *local_err = NULL;
 
     size = HOST_PAGE_ALIGN(size);
@@ -1711,29 +1705,28 @@ ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
     if (resizeable) {
         new_block->flags |= RAM_RESIZEABLE;
     }
-    addr = ram_block_add(new_block, &local_err);
+    ram_block_add(new_block, &local_err);
     if (local_err) {
         g_free(new_block);
         error_propagate(errp, local_err);
-        return -1;
+        return NULL;
     }
-
     mr->ram_block = new_block;
-    return addr;
+    return new_block;
 }
 
-ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
+RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                                    MemoryRegion *mr, Error **errp)
 {
     return qemu_ram_alloc_internal(size, size, NULL, host, false, mr, errp);
 }
 
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
+RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
 {
     return qemu_ram_alloc_internal(size, size, NULL, NULL, false, mr, errp);
 }
 
-ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz,
+RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz,
                                      void (*resized)(const char*,
                                                      uint64_t length,
                                                      void *host),
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5d33def..865e19b 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -94,17 +94,17 @@ ram_addr_t last_ram_offset(void);
 void qemu_mutex_lock_ramlist(void);
 void qemu_mutex_unlock_ramlist(void);
 
-ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-                                    bool share, const char *mem_path,
-                                    Error **errp);
-ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr, Error **errp);
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
-ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
-                                     void (*resized)(const char*,
-                                                     uint64_t length,
-                                                     void *host),
-                                     MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
+                                   bool share, const char *mem_path,
+                                   Error **errp);
+RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
+                                  MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
+                                    void (*resized)(const char*,
+                                                    uint64_t length,
+                                                    void *host),
+                                    MemoryRegion *mr, Error **errp);
 int qemu_get_ram_fd(ram_addr_t addr);
 void qemu_set_ram_fd(ram_addr_t addr, int fd);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
diff --git a/memory.c b/memory.c
index 0dd9695..ae13ba9 100644
--- a/memory.c
+++ b/memory.c
@@ -1226,11 +1226,14 @@ void memory_region_init_ram(MemoryRegion *mr,
                             uint64_t size,
                             Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+    ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1244,11 +1247,14 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
                                                        void *host),
                                        Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1261,11 +1267,14 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       const char *path,
                                       Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
@@ -1276,6 +1285,8 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
                                 uint64_t size,
                                 void *ptr)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
@@ -1284,7 +1295,8 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
-    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    mr->ram_addr = ram_block->offset;
 }
 
 void memory_region_set_skip_dump(MemoryRegion *mr)
@@ -1312,13 +1324,16 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    uint64_t size,
                                    Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+    ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_addr = ram_block->offset;
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_*
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  2016-03-01  2:27   ` Gonglei (Arei)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block Fam Zheng
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

We don't force "const" qualifiers with pointers in QEMU, but it's still
good to keep a clean function interface. Assigning to mr->ram_block is
in this sense ugly - one initializer mutating its owning object's state.

Move it to memory_region_init_*, where mr->ram_addr is assigned.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c   | 1 -
 memory.c | 5 +++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 2b14b79..83e3f7d 100644
--- a/exec.c
+++ b/exec.c
@@ -1711,7 +1711,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
         error_propagate(errp, local_err);
         return NULL;
     }
-    mr->ram_block = new_block;
     return new_block;
 }
 
diff --git a/memory.c b/memory.c
index ae13ba9..fe70075 100644
--- a/memory.c
+++ b/memory.c
@@ -1233,6 +1233,7 @@ void memory_region_init_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1254,6 +1255,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1274,6 +1276,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1296,6 +1299,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
     ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
 }
 
@@ -1333,6 +1337,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
     ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions Fam Zheng
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_* Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  2016-03-01  2:32   ` Gonglei (Arei)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr Fam Zheng
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/exec/memory.h | 8 +-------
 memory.c              | 5 +++++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d5284c2..810d2c0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -978,14 +978,8 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
 /**
  * memory_region_get_ram_addr: Get the ram address associated with a memory
  *                             region
- *
- * DO NOT USE THIS FUNCTION.  This is a temporary workaround while the Xen
- * code is being reworked.
  */
-static inline ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
-{
-    return mr->ram_addr;
-}
+ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
 
 uint64_t memory_region_get_alignment(const MemoryRegion *mr);
 /**
diff --git a/memory.c b/memory.c
index fe70075..769825e 100644
--- a/memory.c
+++ b/memory.c
@@ -1596,6 +1596,11 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
     return ptr + offset;
 }
 
+ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
+{
+    return mr->ram_block->offset;
+}
+
 void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp)
 {
     assert(mr->ram_addr != RAM_ADDR_INVALID);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
                   ` (2 preceding siblings ...)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  2016-03-01  2:37   ` Gonglei (Arei)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free Fam Zheng
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 6/6] exec: Introduce AddressSpaceDispatch.mru_section Fam Zheng
  5 siblings, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

All references to mr->ram_addr are replaced by
memory_region_get_ram_addr(mr) (except for a few assertions that are
replaced with mr->ram_block).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 cputlb.c              |  4 +--
 exec.c                |  3 ++-
 hw/misc/ivshmem.c     |  9 ++++---
 include/exec/memory.h |  1 -
 kvm-all.c             |  3 ++-
 memory.c              | 71 ++++++++++++++++++++-------------------------------
 6 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 3973030..2f7a166 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -416,8 +416,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
             /* Write access calls the I/O callback.  */
             te->addr_write = address | TLB_MMIO;
         } else if (memory_region_is_ram(section->mr)
-                   && cpu_physical_memory_is_clean(section->mr->ram_addr
-                                                   + xlat)) {
+                   && cpu_physical_memory_is_clean(
+                        memory_region_get_ram_addr(section->mr) + xlat)) {
             te->addr_write = address | TLB_NOTDIRTY;
         } else {
             te->addr_write = address;
diff --git a/exec.c b/exec.c
index 83e3f7d..6ed4203 100644
--- a/exec.c
+++ b/exec.c
@@ -2699,7 +2699,8 @@ MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
             }
         } else {
             /* RAM case */
-            ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr + addr1);
+            ptr = qemu_get_ram_ptr(mr->ram_block,
+                                   memory_region_get_ram_addr(mr) + addr1);
             memcpy(buf, ptr, l);
         }
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 48b7a34..1838bc8 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -400,7 +400,7 @@ static int create_shared_memory_BAR(IVShmemState *s, int fd, uint8_t attr,
 
     memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s), "ivshmem.bar2",
                                s->ivshmem_size, ptr);
-    qemu_set_ram_fd(s->ivshmem.ram_addr, fd);
+    qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem), fd);
     vmstate_register_ram(&s->ivshmem, DEVICE(s));
     memory_region_add_subregion(&s->bar, 0, &s->ivshmem);
 
@@ -661,7 +661,8 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
         }
         memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s),
                                    "ivshmem.bar2", s->ivshmem_size, map_ptr);
-        qemu_set_ram_fd(s->ivshmem.ram_addr, incoming_fd);
+        qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem),
+                        incoming_fd);
         vmstate_register_ram(&s->ivshmem, DEVICE(s));
 
         IVSHMEM_DPRINTF("guest h/w addr = %p, size = %" PRIu64 "\n",
@@ -996,8 +997,10 @@ static void pci_ivshmem_exit(PCIDevice *dev)
                              strerror(errno));
             }
 
-            if ((fd = qemu_get_ram_fd(s->ivshmem.ram_addr)) != -1)
+            fd = qemu_get_ram_fd(memory_region_get_ram_addr(&s->ivshmem));
+            if (fd != -1) {
                 close(fd);
+            }
         }
 
         vmstate_unregister_ram(&s->ivshmem, DEVICE(dev));
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 810d2c0..2de7898 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -169,7 +169,6 @@ struct MemoryRegion {
     bool flush_coalesced_mmio;
     bool global_locking;
     uint8_t dirty_log_mask;
-    ram_addr_t ram_addr;
     RAMBlock *ram_block;
     Object *owner;
     const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/kvm-all.c b/kvm-all.c
index a65e73f..161200e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -366,7 +366,8 @@ static void kvm_log_stop(MemoryListener *listener,
 static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
                                          unsigned long *bitmap)
 {
-    ram_addr_t start = section->offset_within_region + section->mr->ram_addr;
+    ram_addr_t start = section->offset_within_region +
+                       memory_region_get_ram_addr(section->mr);
     ram_addr_t pages = int128_get64(section->size) / getpagesize();
 
     cpu_physical_memory_set_dirty_lebitmap(bitmap, start, pages);
diff --git a/memory.c b/memory.c
index 769825e..b221f3c 100644
--- a/memory.c
+++ b/memory.c
@@ -858,12 +858,12 @@ static void memory_region_destructor_none(MemoryRegion *mr)
 
 static void memory_region_destructor_ram(MemoryRegion *mr)
 {
-    qemu_ram_free(mr->ram_addr);
+    qemu_ram_free(memory_region_get_ram_addr(mr));
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
-    qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
+    qemu_ram_free(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
 }
 
 static bool memory_region_need_escape(char c)
@@ -994,7 +994,6 @@ static void memory_region_initfn(Object *obj)
     ObjectProperty *op;
 
     mr->ops = &unassigned_mem_ops;
-    mr->ram_addr = RAM_ADDR_INVALID;
     mr->enabled = true;
     mr->romd_mode = true;
     mr->global_locking = true;
@@ -1226,15 +1225,11 @@ void memory_region_init_ram(MemoryRegion *mr,
                             uint64_t size,
                             Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc(size, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc(size, mr, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1248,15 +1243,12 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
                                                        void *host),
                                        Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
+                                              mr, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1269,15 +1261,11 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       const char *path,
                                       Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
@@ -1288,8 +1276,6 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
                                 uint64_t size,
                                 void *ptr)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
@@ -1298,9 +1284,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
-    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
 }
 
 void memory_region_set_skip_dump(MemoryRegion *mr)
@@ -1328,17 +1312,13 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    uint64_t size,
                                    Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    ram_block = qemu_ram_alloc(size, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc(size, mr, errp);
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,
@@ -1503,24 +1483,26 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
+    assert(mr->ram_block);
+    return cpu_physical_memory_get_dirty(memory_region_get_ram_addr(mr) + addr,
+                                         size, client);
 }
 
 void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
+    assert(mr->ram_block);
+    cpu_physical_memory_set_dirty_range(memory_region_get_ram_addr(mr) + addr,
+                                        size,
                                         memory_region_get_dirty_log_mask(mr));
 }
 
 bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
                                         hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr,
-                                                    size, client);
+    assert(mr->ram_block);
+    return cpu_physical_memory_test_and_clear_dirty(
+                memory_region_get_ram_addr(mr) + addr, size, client);
 }
 
 
@@ -1563,9 +1545,9 @@ void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
 void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
                                hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
-                                             client);
+    assert(mr->ram_block);
+    cpu_physical_memory_test_and_clear_dirty(
+        memory_region_get_ram_addr(mr) + addr, size, client);
 }
 
 int memory_region_get_fd(MemoryRegion *mr)
@@ -1574,9 +1556,9 @@ int memory_region_get_fd(MemoryRegion *mr)
         return memory_region_get_fd(mr->alias);
     }
 
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
+    assert(mr->ram_block);
 
-    return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK);
+    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
 }
 
 void *memory_region_get_ram_ptr(MemoryRegion *mr)
@@ -1589,8 +1571,9 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
         offset += mr->alias_offset;
         mr = mr->alias;
     }
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr & TARGET_PAGE_MASK);
+    assert(mr->ram_block);
+    ptr = qemu_get_ram_ptr(mr->ram_block,
+                           memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
     rcu_read_unlock();
 
     return ptr + offset;
@@ -1603,9 +1586,9 @@ ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
 
 void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
+    assert(mr->ram_block);
 
-    qemu_ram_resize(mr->ram_addr, newsize, errp);
+    qemu_ram_resize(memory_region_get_ram_addr(mr), newsize, errp);
 }
 
 static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as)
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
                   ` (3 preceding siblings ...)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  2016-03-01  2:39   ` Gonglei (Arei)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 6/6] exec: Introduce AddressSpaceDispatch.mru_section Fam Zheng
  5 siblings, 1 reply; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

The only caller now knows exactly which RAMBlock to free, so it's not
necessary to do the lookup.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c                  | 21 +++++++--------------
 include/exec/ram_addr.h |  2 +-
 memory.c                |  4 ++--
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index 6ed4203..ad8b826 100644
--- a/exec.c
+++ b/exec.c
@@ -1751,22 +1751,15 @@ static void reclaim_ramblock(RAMBlock *block)
     g_free(block);
 }
 
-void qemu_ram_free(ram_addr_t addr)
+void qemu_ram_free(RAMBlock *block)
 {
-    RAMBlock *block;
-
     qemu_mutex_lock_ramlist();
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        if (addr == block->offset) {
-            QLIST_REMOVE_RCU(block, next);
-            ram_list.mru_block = NULL;
-            /* Write list before version */
-            smp_wmb();
-            ram_list.version++;
-            call_rcu(block, reclaim_ramblock, rcu);
-            break;
-        }
-    }
+    QLIST_REMOVE_RCU(block, next);
+    ram_list.mru_block = NULL;
+    /* Write list before version */
+    smp_wmb();
+    ram_list.version++;
+    call_rcu(block, reclaim_ramblock, rcu);
     qemu_mutex_unlock_ramlist();
 }
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 865e19b..5adf7a4 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -108,7 +108,7 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
 int qemu_get_ram_fd(ram_addr_t addr);
 void qemu_set_ram_fd(ram_addr_t addr, int fd);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
-void qemu_ram_free(ram_addr_t addr);
+void qemu_ram_free(RAMBlock *block);
 
 int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
 
diff --git a/memory.c b/memory.c
index b221f3c..32d2912 100644
--- a/memory.c
+++ b/memory.c
@@ -858,12 +858,12 @@ static void memory_region_destructor_none(MemoryRegion *mr)
 
 static void memory_region_destructor_ram(MemoryRegion *mr)
 {
-    qemu_ram_free(memory_region_get_ram_addr(mr));
+    qemu_ram_free(mr->ram_block);
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
-    qemu_ram_free(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
+    qemu_ram_free(mr->ram_block);
 }
 
 static bool memory_region_need_escape(char c)
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 6/6] exec: Introduce AddressSpaceDispatch.mru_section
  2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
                   ` (4 preceding siblings ...)
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free Fam Zheng
@ 2016-02-29  2:37 ` Fam Zheng
  5 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2016-02-29  2:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

Under heavy workloads the lookup will likely end up with the same
MemoryRegionSection from last time. Using a pointer to cache the result, like
ram_list.mru_block, significantly reduce computation cost of
address_space_translate.

During address space topology update, as->dispatch will be reallocated
so the pointer is invalidated automatically.

Perf reports a visible drop on the cpu usage.  Before:

   + 2.06% phys_page_find
   + 0.95% address_space_translate_internal
   + 0.80% address_space_translate

After:
   + 0.78% address_space_translate
   + 0.77% address_space_translate_internal
   + 0.69% address_space_lookup_region

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index ad8b826..3232861 100644
--- a/exec.c
+++ b/exec.c
@@ -135,6 +135,7 @@ typedef struct PhysPageMap {
 struct AddressSpaceDispatch {
     struct rcu_head rcu;
 
+    MemoryRegionSection *mru_section;
     /* This is a multi-level map on the physical address space.
      * The bottom level has pointers to MemoryRegionSections.
      */
@@ -342,14 +343,26 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
                                                         hwaddr addr,
                                                         bool resolve_subpage)
 {
-    MemoryRegionSection *section;
+    MemoryRegionSection *section = atomic_read(&d->mru_section);
     subpage_t *subpage;
+    bool update;
 
-    section = phys_page_find(d->phys_map, addr, d->map.nodes, d->map.sections);
+    if (section && section != &d->map.sections[PHYS_SECTION_UNASSIGNED] &&
+        range_covers_byte(section->offset_within_address_space,
+                          section->size.lo, addr)) {
+        update = false;
+    } else {
+        section = phys_page_find(d->phys_map, addr, d->map.nodes,
+                                 d->map.sections);
+        update = true;
+    }
     if (resolve_subpage && section->mr->subpage) {
         subpage = container_of(section->mr, subpage_t, iomem);
         section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
     }
+    if (update) {
+        atomic_set(&d->mru_section, section);
+    }
     return section;
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions Fam Zheng
@ 2016-03-01  2:26   ` Gonglei (Arei)
  0 siblings, 0 replies; 13+ messages in thread
From: Gonglei (Arei) @ 2016-03-01  2:26 UTC (permalink / raw)
  To: Fam Zheng, qemu-devel@nongnu.org; +Cc: Paolo Bonzini

>
> Subject: [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from
> allocating functions
> 
> Previously we return RAMBlock.offset; now return the pointer to the
> whole structure.
> 
> ram_block_add returns void now, error is completely passed with errp.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  exec.c                  | 51
> +++++++++++++++++++++----------------------------
>  include/exec/ram_addr.h | 22 ++++++++++-----------
>  memory.c                | 25 +++++++++++++++++++-----
>  3 files changed, 53 insertions(+), 45 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/exec.c b/exec.c
> index c62c439..2b14b79 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1554,7 +1554,7 @@ static void dirty_memory_extend(ram_addr_t
> old_ram_size,
>      }
>  }
> 
> -static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
> +static void ram_block_add(RAMBlock *new_block, Error **errp)
>  {
>      RAMBlock *block;
>      RAMBlock *last_block = NULL;
> @@ -1573,7 +1573,6 @@ static ram_addr_t ram_block_add(RAMBlock
> *new_block, Error **errp)
>              if (err) {
>                  error_propagate(errp, err);
>                  qemu_mutex_unlock_ramlist();
> -                return -1;
>              }
>          } else {
>              new_block->host = phys_mem_alloc(new_block->max_length,
> @@ -1583,7 +1582,6 @@ static ram_addr_t ram_block_add(RAMBlock
> *new_block, Error **errp)
>                                   "cannot set up guest memory '%s'",
> 
> memory_region_name(new_block->mr));
>                  qemu_mutex_unlock_ramlist();
> -                return -1;
>              }
>              memory_try_enable_merging(new_block->host,
> new_block->max_length);
>          }
> @@ -1631,22 +1629,19 @@ static ram_addr_t ram_block_add(RAMBlock
> *new_block, Error **errp)
>              kvm_setup_guest_memory(new_block->host,
> new_block->max_length);
>          }
>      }
> -
> -    return new_block->offset;
>  }
> 
>  #ifdef __linux__
> -ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> -                                    bool share, const char
> *mem_path,
> -                                    Error **errp)
> +RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> +                                   bool share, const char
> *mem_path,
> +                                   Error **errp)
>  {
>      RAMBlock *new_block;
> -    ram_addr_t addr;
>      Error *local_err = NULL;
> 
>      if (xen_enabled()) {
>          error_setg(errp, "-mem-path not supported with Xen");
> -        return -1;
> +        return NULL;
>      }
> 
>      if (phys_mem_alloc != qemu_anon_ram_alloc) {
> @@ -1657,7 +1652,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t
> size, MemoryRegion *mr,
>           */
>          error_setg(errp,
>                     "-mem-path not supported with this accelerator");
> -        return -1;
> +        return NULL;
>      }
> 
>      size = HOST_PAGE_ALIGN(size);
> @@ -1670,29 +1665,28 @@ ram_addr_t
> qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>                                       mem_path, errp);
>      if (!new_block->host) {
>          g_free(new_block);
> -        return -1;
> +        return NULL;
>      }
> 
> -    addr = ram_block_add(new_block, &local_err);
> +    ram_block_add(new_block, &local_err);
>      if (local_err) {
>          g_free(new_block);
>          error_propagate(errp, local_err);
> -        return -1;
> +        return NULL;
>      }
> -    return addr;
> +    return new_block;
>  }
>  #endif
> 
>  static
> -ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t
> max_size,
> -                                   void (*resized)(const char*,
> -                                                   uint64_t
> length,
> -                                                   void *host),
> -                                   void *host, bool resizeable,
> -                                   MemoryRegion *mr, Error **errp)
> +RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t
> max_size,
> +                                  void (*resized)(const char*,
> +                                                  uint64_t length,
> +                                                  void *host),
> +                                  void *host, bool resizeable,
> +                                  MemoryRegion *mr, Error **errp)
>  {
>      RAMBlock *new_block;
> -    ram_addr_t addr;
>      Error *local_err = NULL;
> 
>      size = HOST_PAGE_ALIGN(size);
> @@ -1711,29 +1705,28 @@ ram_addr_t
> qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
>      if (resizeable) {
>          new_block->flags |= RAM_RESIZEABLE;
>      }
> -    addr = ram_block_add(new_block, &local_err);
> +    ram_block_add(new_block, &local_err);
>      if (local_err) {
>          g_free(new_block);
>          error_propagate(errp, local_err);
> -        return -1;
> +        return NULL;
>      }
> -
>      mr->ram_block = new_block;
> -    return addr;
> +    return new_block;
>  }
> 
> -ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> +RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>                                     MemoryRegion *mr, Error **errp)
>  {
>      return qemu_ram_alloc_internal(size, size, NULL, host, false, mr, errp);
>  }
> 
> -ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error
> **errp)
> +RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error
> **errp)
>  {
>      return qemu_ram_alloc_internal(size, size, NULL, NULL, false, mr, errp);
>  }
> 
> -ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t
> maxsz,
> +RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t
> maxsz,
>                                       void (*resized)(const char*,
>                                                       uint64_t
> length,
>                                                       void *host),
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5d33def..865e19b 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -94,17 +94,17 @@ ram_addr_t last_ram_offset(void);
>  void qemu_mutex_lock_ramlist(void);
>  void qemu_mutex_unlock_ramlist(void);
> 
> -ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> -                                    bool share, const char
> *mem_path,
> -                                    Error **errp);
> -ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> -                                   MemoryRegion *mr, Error **errp);
> -ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error
> **errp);
> -ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t
> max_size,
> -                                     void (*resized)(const char*,
> -                                                     uint64_t
> length,
> -                                                     void *host),
> -                                     MemoryRegion *mr, Error
> **errp);
> +RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> +                                   bool share, const char
> *mem_path,
> +                                   Error **errp);
> +RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> +                                  MemoryRegion *mr, Error **errp);
> +RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error
> **errp);
> +RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t
> max_size,
> +                                    void (*resized)(const char*,
> +                                                    uint64_t
> length,
> +                                                    void *host),
> +                                    MemoryRegion *mr, Error
> **errp);
>  int qemu_get_ram_fd(ram_addr_t addr);
>  void qemu_set_ram_fd(ram_addr_t addr, int fd);
>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
> diff --git a/memory.c b/memory.c
> index 0dd9695..ae13ba9 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1226,11 +1226,14 @@ void memory_region_init_ram(MemoryRegion
> *mr,
>                              uint64_t size,
>                              Error **errp)
>  {
> +    RAMBlock *ram_block;
> +
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> +    ram_block = qemu_ram_alloc(size, mr, errp);
> +    mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> 
> @@ -1244,11 +1247,14 @@ void
> memory_region_init_resizeable_ram(MemoryRegion *mr,
>                                                         void
> *host),
>                                         Error **errp)
>  {
> +    RAMBlock *ram_block;
> +
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
> errp);
> +    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
> errp);
> +    mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> 
> @@ -1261,11 +1267,14 @@ void
> memory_region_init_ram_from_file(MemoryRegion *mr,
>                                        const char *path,
>                                        Error **errp)
>  {
> +    RAMBlock *ram_block;
> +
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
> +    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
> +    mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
>  #endif
> @@ -1276,6 +1285,8 @@ void memory_region_init_ram_ptr(MemoryRegion
> *mr,
>                                  uint64_t size,
>                                  void *ptr)
>  {
> +    RAMBlock *ram_block;
> +
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
> @@ -1284,7 +1295,8 @@ void memory_region_init_ram_ptr(MemoryRegion
> *mr,
> 
>      /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
>      assert(ptr != NULL);
> -    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
> +    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
> +    mr->ram_addr = ram_block->offset;
>  }
> 
>  void memory_region_set_skip_dump(MemoryRegion *mr)
> @@ -1312,13 +1324,16 @@ void
> memory_region_init_rom_device(MemoryRegion *mr,
>                                     uint64_t size,
>                                     Error **errp)
>  {
> +    RAMBlock *ram_block;
> +
>      memory_region_init(mr, owner, name, size);
>      mr->ops = ops;
>      mr->opaque = opaque;
>      mr->terminates = true;
>      mr->rom_device = true;
>      mr->destructor = memory_region_destructor_rom_device;
> -    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> +    ram_block = qemu_ram_alloc(size, mr, errp);
> +    mr->ram_addr = ram_block->offset;
>  }
> 
>  void memory_region_init_iommu(MemoryRegion *mr,
> --
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_*
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_* Fam Zheng
@ 2016-03-01  2:27   ` Gonglei (Arei)
  0 siblings, 0 replies; 13+ messages in thread
From: Gonglei (Arei) @ 2016-03-01  2:27 UTC (permalink / raw)
  To: Fam Zheng, qemu-devel@nongnu.org; +Cc: Paolo Bonzini


> Subject: [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to
> memory_region_init_*
> 
> We don't force "const" qualifiers with pointers in QEMU, but it's still
> good to keep a clean function interface. Assigning to mr->ram_block is
> in this sense ugly - one initializer mutating its owning object's state.
> 
> Move it to memory_region_init_*, where mr->ram_addr is assigned.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  exec.c   | 1 -
>  memory.c | 5 +++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/exec.c b/exec.c
> index 2b14b79..83e3f7d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1711,7 +1711,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t
> size, ram_addr_t max_size,
>          error_propagate(errp, local_err);
>          return NULL;
>      }
> -    mr->ram_block = new_block;
>      return new_block;
>  }
> 
> diff --git a/memory.c b/memory.c
> index ae13ba9..fe70075 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1233,6 +1233,7 @@ void memory_region_init_ram(MemoryRegion *mr,
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
>      ram_block = qemu_ram_alloc(size, mr, errp);
> +    mr->ram_block = ram_block;
>      mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> @@ -1254,6 +1255,7 @@ void
> memory_region_init_resizeable_ram(MemoryRegion *mr,
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
>      ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
> errp);
> +    mr->ram_block = ram_block;
>      mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> @@ -1274,6 +1276,7 @@ void
> memory_region_init_ram_from_file(MemoryRegion *mr,
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
>      ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
> +    mr->ram_block = ram_block;
>      mr->ram_addr = ram_block->offset;
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> @@ -1296,6 +1299,7 @@ void memory_region_init_ram_ptr(MemoryRegion
> *mr,
>      /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
>      assert(ptr != NULL);
>      ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
> +    mr->ram_block = ram_block;
>      mr->ram_addr = ram_block->offset;
>  }
> 
> @@ -1333,6 +1337,7 @@ void
> memory_region_init_rom_device(MemoryRegion *mr,
>      mr->rom_device = true;
>      mr->destructor = memory_region_destructor_rom_device;
>      ram_block = qemu_ram_alloc(size, mr, errp);
> +    mr->ram_block = ram_block;
>      mr->ram_addr = ram_block->offset;
>  }
> 
> --
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block Fam Zheng
@ 2016-03-01  2:32   ` Gonglei (Arei)
  2016-03-01  2:46     ` Fam Zheng
  0 siblings, 1 reply; 13+ messages in thread
From: Gonglei (Arei) @ 2016-03-01  2:32 UTC (permalink / raw)
  To: Fam Zheng, qemu-devel@nongnu.org; +Cc: Paolo Bonzini


> Subject: [Qemu-devel] [PATCH 3/6] memory: Implement
> memory_region_get_ram_addr with mr->ram_block
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  include/exec/memory.h | 8 +-------
>  memory.c              | 5 +++++
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index d5284c2..810d2c0 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -978,14 +978,8 @@ void
> memory_region_add_subregion_overlap(MemoryRegion *mr,
>  /**
>   * memory_region_get_ram_addr: Get the ram address associated with a
> memory
>   *                             region
> - *
> - * DO NOT USE THIS FUNCTION.  This is a temporary workaround while the
> Xen
> - * code is being reworked.
>   */
> -static inline ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> -{
> -    return mr->ram_addr;
> -}
> +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
> 
>  uint64_t memory_region_get_alignment(const MemoryRegion *mr);
>  /**
> diff --git a/memory.c b/memory.c
> index fe70075..769825e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1596,6 +1596,11 @@ void
> *memory_region_get_ram_ptr(MemoryRegion *mr)
>      return ptr + offset;
>  }
> 
> +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> +{
> +    return mr->ram_block->offset;
> +}
> +

Do we need add NULL check for mr->ram_block ?


Regards,
-Gonglei

>  void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize,
> Error **errp)
>  {
>      assert(mr->ram_addr != RAM_ADDR_INVALID);
> --
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr Fam Zheng
@ 2016-03-01  2:37   ` Gonglei (Arei)
  0 siblings, 0 replies; 13+ messages in thread
From: Gonglei (Arei) @ 2016-03-01  2:37 UTC (permalink / raw)
  To: Fam Zheng, qemu-devel@nongnu.org; +Cc: Paolo Bonzini

> Subject: [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr
> 
> All references to mr->ram_addr are replaced by
> memory_region_get_ram_addr(mr) (except for a few assertions that are
> replaced with mr->ram_block).
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  cputlb.c              |  4 +--
>  exec.c                |  3 ++-
>  hw/misc/ivshmem.c     |  9 ++++---
>  include/exec/memory.h |  1 -
>  kvm-all.c             |  3 ++-
>  memory.c              | 71
> ++++++++++++++++++++-------------------------------
>  6 files changed, 39 insertions(+), 52 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/cputlb.c b/cputlb.c
> index 3973030..2f7a166 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -416,8 +416,8 @@ void tlb_set_page_with_attrs(CPUState *cpu,
> target_ulong vaddr,
>              /* Write access calls the I/O callback.  */
>              te->addr_write = address | TLB_MMIO;
>          } else if (memory_region_is_ram(section->mr)
> -                   &&
> cpu_physical_memory_is_clean(section->mr->ram_addr
> -                                                   + xlat)) {
> +                   && cpu_physical_memory_is_clean(
> +                        memory_region_get_ram_addr(section->mr) +
> xlat)) {
>              te->addr_write = address | TLB_NOTDIRTY;
>          } else {
>              te->addr_write = address;
> diff --git a/exec.c b/exec.c
> index 83e3f7d..6ed4203 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2699,7 +2699,8 @@ MemTxResult
> address_space_read_continue(AddressSpace *as, hwaddr addr,
>              }
>          } else {
>              /* RAM case */
> -            ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr +
> addr1);
> +            ptr = qemu_get_ram_ptr(mr->ram_block,
> +
> memory_region_get_ram_addr(mr) + addr1);
>              memcpy(buf, ptr, l);
>          }
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 48b7a34..1838bc8 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -400,7 +400,7 @@ static int create_shared_memory_BAR(IVShmemState
> *s, int fd, uint8_t attr,
> 
>      memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s),
> "ivshmem.bar2",
>                                 s->ivshmem_size, ptr);
> -    qemu_set_ram_fd(s->ivshmem.ram_addr, fd);
> +    qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem), fd);
>      vmstate_register_ram(&s->ivshmem, DEVICE(s));
>      memory_region_add_subregion(&s->bar, 0, &s->ivshmem);
> 
> @@ -661,7 +661,8 @@ static void ivshmem_read(void *opaque, const uint8_t
> *buf, int size)
>          }
>          memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s),
>                                     "ivshmem.bar2", s->ivshmem_size,
> map_ptr);
> -        qemu_set_ram_fd(s->ivshmem.ram_addr, incoming_fd);
> +        qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem),
> +                        incoming_fd);
>          vmstate_register_ram(&s->ivshmem, DEVICE(s));
> 
>          IVSHMEM_DPRINTF("guest h/w addr = %p, size = %" PRIu64 "\n",
> @@ -996,8 +997,10 @@ static void pci_ivshmem_exit(PCIDevice *dev)
>                               strerror(errno));
>              }
> 
> -            if ((fd = qemu_get_ram_fd(s->ivshmem.ram_addr)) != -1)
> +            fd =
> qemu_get_ram_fd(memory_region_get_ram_addr(&s->ivshmem));
> +            if (fd != -1) {
>                  close(fd);
> +            }
>          }
> 
>          vmstate_unregister_ram(&s->ivshmem, DEVICE(dev));
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 810d2c0..2de7898 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -169,7 +169,6 @@ struct MemoryRegion {
>      bool flush_coalesced_mmio;
>      bool global_locking;
>      uint8_t dirty_log_mask;
> -    ram_addr_t ram_addr;
>      RAMBlock *ram_block;
>      Object *owner;
>      const MemoryRegionIOMMUOps *iommu_ops;
> diff --git a/kvm-all.c b/kvm-all.c
> index a65e73f..161200e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -366,7 +366,8 @@ static void kvm_log_stop(MemoryListener *listener,
>  static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
>                                           unsigned long *bitmap)
>  {
> -    ram_addr_t start = section->offset_within_region +
> section->mr->ram_addr;
> +    ram_addr_t start = section->offset_within_region +
> +                       memory_region_get_ram_addr(section->mr);
>      ram_addr_t pages = int128_get64(section->size) / getpagesize();
> 
>      cpu_physical_memory_set_dirty_lebitmap(bitmap, start, pages);
> diff --git a/memory.c b/memory.c
> index 769825e..b221f3c 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -858,12 +858,12 @@ static void
> memory_region_destructor_none(MemoryRegion *mr)
> 
>  static void memory_region_destructor_ram(MemoryRegion *mr)
>  {
> -    qemu_ram_free(mr->ram_addr);
> +    qemu_ram_free(memory_region_get_ram_addr(mr));
>  }
> 
>  static void memory_region_destructor_rom_device(MemoryRegion *mr)
>  {
> -    qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
> +    qemu_ram_free(memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
>  }
> 
>  static bool memory_region_need_escape(char c)
> @@ -994,7 +994,6 @@ static void memory_region_initfn(Object *obj)
>      ObjectProperty *op;
> 
>      mr->ops = &unassigned_mem_ops;
> -    mr->ram_addr = RAM_ADDR_INVALID;
>      mr->enabled = true;
>      mr->romd_mode = true;
>      mr->global_locking = true;
> @@ -1226,15 +1225,11 @@ void memory_region_init_ram(MemoryRegion
> *mr,
>                              uint64_t size,
>                              Error **errp)
>  {
> -    RAMBlock *ram_block;
> -
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    ram_block = qemu_ram_alloc(size, mr, errp);
> -    mr->ram_block = ram_block;
> -    mr->ram_addr = ram_block->offset;
> +    mr->ram_block = qemu_ram_alloc(size, mr, errp);
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> 
> @@ -1248,15 +1243,12 @@ void
> memory_region_init_resizeable_ram(MemoryRegion *mr,
>                                                         void
> *host),
>                                         Error **errp)
>  {
> -    RAMBlock *ram_block;
> -
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
> errp);
> -    mr->ram_block = ram_block;
> -    mr->ram_addr = ram_block->offset;
> +    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
> +                                              mr, errp);
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
> 
> @@ -1269,15 +1261,11 @@ void
> memory_region_init_ram_from_file(MemoryRegion *mr,
>                                        const char *path,
>                                        Error **errp)
>  {
> -    RAMBlock *ram_block;
> -
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
> -    mr->ram_block = ram_block;
> -    mr->ram_addr = ram_block->offset;
> +    mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
>      mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>  }
>  #endif
> @@ -1288,8 +1276,6 @@ void memory_region_init_ram_ptr(MemoryRegion
> *mr,
>                                  uint64_t size,
>                                  void *ptr)
>  {
> -    RAMBlock *ram_block;
> -
>      memory_region_init(mr, owner, name, size);
>      mr->ram = true;
>      mr->terminates = true;
> @@ -1298,9 +1284,7 @@ void memory_region_init_ram_ptr(MemoryRegion
> *mr,
> 
>      /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
>      assert(ptr != NULL);
> -    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
> -    mr->ram_block = ram_block;
> -    mr->ram_addr = ram_block->offset;
> +    mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
>  }
> 
>  void memory_region_set_skip_dump(MemoryRegion *mr)
> @@ -1328,17 +1312,13 @@ void
> memory_region_init_rom_device(MemoryRegion *mr,
>                                     uint64_t size,
>                                     Error **errp)
>  {
> -    RAMBlock *ram_block;
> -
>      memory_region_init(mr, owner, name, size);
>      mr->ops = ops;
>      mr->opaque = opaque;
>      mr->terminates = true;
>      mr->rom_device = true;
>      mr->destructor = memory_region_destructor_rom_device;
> -    ram_block = qemu_ram_alloc(size, mr, errp);
> -    mr->ram_block = ram_block;
> -    mr->ram_addr = ram_block->offset;
> +    mr->ram_block = qemu_ram_alloc(size, mr, errp);
>  }
> 
>  void memory_region_init_iommu(MemoryRegion *mr,
> @@ -1503,24 +1483,26 @@ void memory_region_set_log(MemoryRegion *mr,
> bool log, unsigned client)
>  bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
>                               hwaddr size, unsigned client)
>  {
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> -    return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
> client);
> +    assert(mr->ram_block);
> +    return
> cpu_physical_memory_get_dirty(memory_region_get_ram_addr(mr) + addr,
> +                                         size, client);
>  }
> 
>  void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
>                               hwaddr size)
>  {
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> -    cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
> +    assert(mr->ram_block);
> +
> cpu_physical_memory_set_dirty_range(memory_region_get_ram_addr(mr) +
> addr,
> +                                        size,
> 
> memory_region_get_dirty_log_mask(mr));
>  }
> 
>  bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
>                                          hwaddr size, unsigned
> client)
>  {
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> -    return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr +
> addr,
> -                                                    size, client);
> +    assert(mr->ram_block);
> +    return cpu_physical_memory_test_and_clear_dirty(
> +                memory_region_get_ram_addr(mr) + addr, size, client);
>  }
> 
> 
> @@ -1563,9 +1545,9 @@ void
> memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
>  void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
>                                 hwaddr size, unsigned client)
>  {
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> -    cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
> -                                             client);
> +    assert(mr->ram_block);
> +    cpu_physical_memory_test_and_clear_dirty(
> +        memory_region_get_ram_addr(mr) + addr, size, client);
>  }
> 
>  int memory_region_get_fd(MemoryRegion *mr)
> @@ -1574,9 +1556,9 @@ int memory_region_get_fd(MemoryRegion *mr)
>          return memory_region_get_fd(mr->alias);
>      }
> 
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> +    assert(mr->ram_block);
> 
> -    return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK);
> +    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
>  }
> 
>  void *memory_region_get_ram_ptr(MemoryRegion *mr)
> @@ -1589,8 +1571,9 @@ void *memory_region_get_ram_ptr(MemoryRegion
> *mr)
>          offset += mr->alias_offset;
>          mr = mr->alias;
>      }
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> -    ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr &
> TARGET_PAGE_MASK);
> +    assert(mr->ram_block);
> +    ptr = qemu_get_ram_ptr(mr->ram_block,
> +                           memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
>      rcu_read_unlock();
> 
>      return ptr + offset;
> @@ -1603,9 +1586,9 @@ ram_addr_t
> memory_region_get_ram_addr(MemoryRegion *mr)
> 
>  void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize,
> Error **errp)
>  {
> -    assert(mr->ram_addr != RAM_ADDR_INVALID);
> +    assert(mr->ram_block);
> 
> -    qemu_ram_resize(mr->ram_addr, newsize, errp);
> +    qemu_ram_resize(memory_region_get_ram_addr(mr), newsize, errp);
>  }
> 
>  static void memory_region_update_coalesced_range_as(MemoryRegion *mr,
> AddressSpace *as)
> --
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free
  2016-02-29  2:37 ` [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free Fam Zheng
@ 2016-03-01  2:39   ` Gonglei (Arei)
  0 siblings, 0 replies; 13+ messages in thread
From: Gonglei (Arei) @ 2016-03-01  2:39 UTC (permalink / raw)
  To: Fam Zheng, qemu-devel@nongnu.org; +Cc: Paolo Bonzini


> Subject: [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to
> qemu_ram_free
> 
> The only caller now knows exactly which RAMBlock to free, so it's not
> necessary to do the lookup.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  exec.c                  | 21 +++++++--------------
>  include/exec/ram_addr.h |  2 +-
>  memory.c                |  4 ++--
>  3 files changed, 10 insertions(+), 17 deletions(-)
> 

Nice ;)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/exec.c b/exec.c
> index 6ed4203..ad8b826 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1751,22 +1751,15 @@ static void reclaim_ramblock(RAMBlock *block)
>      g_free(block);
>  }
> 
> -void qemu_ram_free(ram_addr_t addr)
> +void qemu_ram_free(RAMBlock *block)
>  {
> -    RAMBlock *block;
> -
>      qemu_mutex_lock_ramlist();
> -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> -        if (addr == block->offset) {
> -            QLIST_REMOVE_RCU(block, next);
> -            ram_list.mru_block = NULL;
> -            /* Write list before version */
> -            smp_wmb();
> -            ram_list.version++;
> -            call_rcu(block, reclaim_ramblock, rcu);
> -            break;
> -        }
> -    }
> +    QLIST_REMOVE_RCU(block, next);
> +    ram_list.mru_block = NULL;
> +    /* Write list before version */
> +    smp_wmb();
> +    ram_list.version++;
> +    call_rcu(block, reclaim_ramblock, rcu);
>      qemu_mutex_unlock_ramlist();
>  }
> 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 865e19b..5adf7a4 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -108,7 +108,7 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t
> size, ram_addr_t max_size,
>  int qemu_get_ram_fd(ram_addr_t addr);
>  void qemu_set_ram_fd(ram_addr_t addr, int fd);
>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
> -void qemu_ram_free(ram_addr_t addr);
> +void qemu_ram_free(RAMBlock *block);
> 
>  int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
> 
> diff --git a/memory.c b/memory.c
> index b221f3c..32d2912 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -858,12 +858,12 @@ static void
> memory_region_destructor_none(MemoryRegion *mr)
> 
>  static void memory_region_destructor_ram(MemoryRegion *mr)
>  {
> -    qemu_ram_free(memory_region_get_ram_addr(mr));
> +    qemu_ram_free(mr->ram_block);
>  }
> 
>  static void memory_region_destructor_rom_device(MemoryRegion *mr)
>  {
> -    qemu_ram_free(memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
> +    qemu_ram_free(mr->ram_block);
>  }
> 
>  static bool memory_region_need_escape(char c)
> --
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block
  2016-03-01  2:32   ` Gonglei (Arei)
@ 2016-03-01  2:46     ` Fam Zheng
  0 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2016-03-01  2:46 UTC (permalink / raw)
  To: Gonglei (Arei); +Cc: Paolo Bonzini, qemu-devel@nongnu.org

On Tue, 03/01 02:32, Gonglei (Arei) wrote:
> 
> > Subject: [Qemu-devel] [PATCH 3/6] memory: Implement
> > memory_region_get_ram_addr with mr->ram_block
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  include/exec/memory.h | 8 +-------
> >  memory.c              | 5 +++++
> >  2 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index d5284c2..810d2c0 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -978,14 +978,8 @@ void
> > memory_region_add_subregion_overlap(MemoryRegion *mr,
> >  /**
> >   * memory_region_get_ram_addr: Get the ram address associated with a
> > memory
> >   *                             region
> > - *
> > - * DO NOT USE THIS FUNCTION.  This is a temporary workaround while the
> > Xen
> > - * code is being reworked.
> >   */
> > -static inline ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> > -{
> > -    return mr->ram_addr;
> > -}
> > +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
> > 
> >  uint64_t memory_region_get_alignment(const MemoryRegion *mr);
> >  /**
> > diff --git a/memory.c b/memory.c
> > index fe70075..769825e 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1596,6 +1596,11 @@ void
> > *memory_region_get_ram_ptr(MemoryRegion *mr)
> >      return ptr + offset;
> >  }
> > 
> > +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> > +{
> > +    return mr->ram_block->offset;
> > +}
> > +
> 
> Do we need add NULL check for mr->ram_block ?

Yes, will add it. Thanks!

Fam

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-03-01  2:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-29  2:37 [Qemu-devel] [PATCH 0/6] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate Fam Zheng
2016-02-29  2:37 ` [Qemu-devel] [PATCH 1/6] exec: Return RAMBlock pointer from allocating functions Fam Zheng
2016-03-01  2:26   ` Gonglei (Arei)
2016-02-29  2:37 ` [Qemu-devel] [PATCH 2/6] memory: Move assignment to ram_block to memory_region_init_* Fam Zheng
2016-03-01  2:27   ` Gonglei (Arei)
2016-02-29  2:37 ` [Qemu-devel] [PATCH 3/6] memory: Implement memory_region_get_ram_addr with mr->ram_block Fam Zheng
2016-03-01  2:32   ` Gonglei (Arei)
2016-03-01  2:46     ` Fam Zheng
2016-02-29  2:37 ` [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr Fam Zheng
2016-03-01  2:37   ` Gonglei (Arei)
2016-02-29  2:37 ` [Qemu-devel] [PATCH 5/6] exec: Pass RAMBlock pointer to qemu_ram_free Fam Zheng
2016-03-01  2:39   ` Gonglei (Arei)
2016-02-29  2:37 ` [Qemu-devel] [PATCH 6/6] exec: Introduce AddressSpaceDispatch.mru_section Fam Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).