All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] memory: some little optimzation
@ 2016-04-20  3:34 Gonglei
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() Gonglei
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Gonglei @ 2016-04-20  3:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, famz, peter.huangpeng, Gonglei

Please see the details in each patch.

Gonglei (3):
  memory: drop find_ram_block()
  exec: adjuest rcu_read_lock requiement
  memory: pass RAMBlock as a agrument

 exec.c                    | 89 +++++++++++++++++++++--------------------------
 hw/misc/ivshmem.c         |  6 ++--
 hw/virtio/vhost-user.c    | 11 +++---
 include/exec/cpu-common.h |  4 +--
 include/exec/ram_addr.h   |  8 ++---
 memory.c                  |  5 +--
 migration/ram.c           |  2 +-
 migration/savevm.c        |  4 +--
 8 files changed, 61 insertions(+), 68 deletions(-)

-- 
1.7.12.4

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

* [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block()
  2016-04-20  3:34 [Qemu-devel] [PATCH 0/3] memory: some little optimzation Gonglei
@ 2016-04-20  3:34 ` Gonglei
  2016-04-29  7:48   ` Fam Zheng
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement Gonglei
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Gonglei @ 2016-04-20  3:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, famz, peter.huangpeng, Gonglei

On the one hand, we have already qemu_get_ram_block() whose function
is similar. On the other hand, we can directly use mr->ram_block but
searching RAMblock by ram_addr which is a kind of waste.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 exec.c                    | 37 +++++++++----------------------------
 include/exec/cpu-common.h |  4 ++--
 include/exec/ram_addr.h   |  2 +-
 memory.c                  |  2 +-
 migration/ram.c           |  2 +-
 migration/savevm.c        |  4 ++--
 6 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/exec.c b/exec.c
index c4f9036..101f0f4 100644
--- a/exec.c
+++ b/exec.c
@@ -1407,34 +1407,19 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)
     }
 }
 
-/* Called within an RCU critical section, or while the ramlist lock
- * is held.
- */
-static RAMBlock *find_ram_block(ram_addr_t addr)
-{
-    RAMBlock *block;
-
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        if (block->offset == addr) {
-            return block;
-        }
-    }
-
-    return NULL;
-}
-
 const char *qemu_ram_get_idstr(RAMBlock *rb)
 {
     return rb->idstr;
 }
 
 /* Called with iothread lock held.  */
-void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
+void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
 {
-    RAMBlock *new_block, *block;
+    RAMBlock *new_block = block;
+    RAMBlock *old_block;
 
     rcu_read_lock();
-    new_block = find_ram_block(addr);
+
     assert(new_block);
     assert(!new_block->idstr[0]);
 
@@ -1447,8 +1432,9 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
     }
     pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
 
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
+    QLIST_FOREACH_RCU(old_block, &ram_list.blocks, next) {
+        if (old_block != new_block &&
+            !strcmp(old_block->idstr, new_block->idstr)) {
             fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
                     new_block->idstr);
             abort();
@@ -1458,17 +1444,14 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
 }
 
 /* Called with iothread lock held.  */
-void qemu_ram_unset_idstr(ram_addr_t addr)
+void qemu_ram_unset_idstr(RAMBlock *block)
 {
-    RAMBlock *block;
-
     /* FIXME: arch_init.c assumes that this is not called throughout
      * migration.  Ignore the problem since hot-unplug during migration
      * does not work anyway.
      */
 
     rcu_read_lock();
-    block = find_ram_block(addr);
     if (block) {
         memset(block->idstr, 0, sizeof(block->idstr));
     }
@@ -1492,10 +1475,8 @@ static int memory_try_enable_merging(void *addr, size_t len)
  * resize callback to update device state and/or add assertions to detect
  * misuse, if necessary.
  */
-int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp)
+int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
 {
-    RAMBlock *block = find_ram_block(base);
-
     assert(block);
 
     newsize = HOST_PAGE_ALIGN(newsize);
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 9e839e5..187df3b 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -65,8 +65,8 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
 RAMBlock *qemu_ram_block_by_name(const char *name);
 RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
                                    ram_addr_t *ram_addr, ram_addr_t *offset);
-void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev);
-void qemu_ram_unset_idstr(ram_addr_t addr);
+void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
+void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
 
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5adf7a4..5b6e1b8 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -110,7 +110,7 @@ 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(RAMBlock *block);
 
-int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
+int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
 
 #define DIRTY_CLIENTS_ALL     ((1 << DIRTY_MEMORY_NUM) - 1)
 #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE))
diff --git a/memory.c b/memory.c
index f76f85d..239e6da 100644
--- a/memory.c
+++ b/memory.c
@@ -1671,7 +1671,7 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp
 {
     assert(mr->ram_block);
 
-    qemu_ram_resize(memory_region_get_ram_addr(mr), newsize, errp);
+    qemu_ram_resize(mr->ram_block, newsize, errp);
 }
 
 static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as)
diff --git a/migration/ram.c b/migration/ram.c
index 3f05738..28b5dd8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2476,7 +2476,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                     if (length != block->used_length) {
                         Error *local_err = NULL;
 
-                        ret = qemu_ram_resize(block->offset, length,
+                        ret = qemu_ram_resize(block, length,
                                               &local_err);
                         if (local_err) {
                             error_report_err(local_err);
diff --git a/migration/savevm.c b/migration/savevm.c
index 16ba443..3a2e843 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2228,13 +2228,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
-    qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
+    qemu_ram_set_idstr(mr->ram_block,
                        memory_region_name(mr), dev);
 }
 
 void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev)
 {
-    qemu_ram_unset_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
+    qemu_ram_unset_idstr(mr->ram_block);
 }
 
 void vmstate_register_ram_global(MemoryRegion *mr)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement
  2016-04-20  3:34 [Qemu-devel] [PATCH 0/3] memory: some little optimzation Gonglei
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() Gonglei
@ 2016-04-20  3:34 ` Gonglei
  2016-04-29  7:56   ` Fam Zheng
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument Gonglei
  2016-05-09 11:41 ` [Qemu-devel] [PATCH 0/3] memory: some little optimzation Paolo Bonzini
  3 siblings, 1 reply; 12+ messages in thread
From: Gonglei @ 2016-04-20  3:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, famz, peter.huangpeng, Gonglei

qemu_ram_unset_idstr() don't need rcu lock anymore,
meanwhile adjuest the range of rcu lock in
qemu_ram_set_idstr() as small as possible.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 exec.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/exec.c b/exec.c
index 101f0f4..b0ecbcd 100644
--- a/exec.c
+++ b/exec.c
@@ -1418,8 +1418,6 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
     RAMBlock *new_block = block;
     RAMBlock *old_block;
 
-    rcu_read_lock();
-
     assert(new_block);
     assert(!new_block->idstr[0]);
 
@@ -1432,6 +1430,7 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
     }
     pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
 
+    rcu_read_lock();
     QLIST_FOREACH_RCU(old_block, &ram_list.blocks, next) {
         if (old_block != new_block &&
             !strcmp(old_block->idstr, new_block->idstr)) {
@@ -1443,7 +1442,6 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
     rcu_read_unlock();
 }
 
-/* Called with iothread lock held.  */
 void qemu_ram_unset_idstr(RAMBlock *block)
 {
     /* FIXME: arch_init.c assumes that this is not called throughout
@@ -1451,11 +1449,9 @@ void qemu_ram_unset_idstr(RAMBlock *block)
      * does not work anyway.
      */
 
-    rcu_read_lock();
     if (block) {
         memset(block->idstr, 0, sizeof(block->idstr));
     }
-    rcu_read_unlock();
 }
 
 static int memory_try_enable_merging(void *addr, size_t len)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument
  2016-04-20  3:34 [Qemu-devel] [PATCH 0/3] memory: some little optimzation Gonglei
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() Gonglei
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement Gonglei
@ 2016-04-20  3:34 ` Gonglei
  2016-04-29  8:06   ` Fam Zheng
  2016-05-09 11:41 ` [Qemu-devel] [PATCH 0/3] memory: some little optimzation Paolo Bonzini
  3 siblings, 1 reply; 12+ messages in thread
From: Gonglei @ 2016-04-20  3:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, famz, peter.huangpeng, Gonglei

In this way, we can avoid to invoke qemu_get_ram_block()
at most time, which can save cpu cycle.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 exec.c                  | 48 ++++++++++++++++++++++++++++++------------------
 hw/misc/ivshmem.c       |  6 ++++--
 hw/virtio/vhost-user.c  | 11 ++++++-----
 include/exec/ram_addr.h |  6 +++---
 memory.c                |  3 ++-
 5 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/exec.c b/exec.c
index b0ecbcd..f3348aa 100644
--- a/exec.c
+++ b/exec.c
@@ -1813,37 +1813,49 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 }
 #endif /* !_WIN32 */
 
-int qemu_get_ram_fd(ram_addr_t addr)
+int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr)
 {
-    RAMBlock *block;
+    RAMBlock *block = ram_block;
     int fd;
 
-    rcu_read_lock();
-    block = qemu_get_ram_block(addr);
-    fd = block->fd;
-    rcu_read_unlock();
+    if (block == NULL) {
+        rcu_read_lock();
+        block = qemu_get_ram_block(addr);
+        fd = block->fd;
+        rcu_read_unlock();
+    } else {
+        fd = block->fd;
+    }
     return fd;
 }
 
-void qemu_set_ram_fd(ram_addr_t addr, int fd)
+void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd)
 {
-    RAMBlock *block;
+    RAMBlock *block = ram_block;
 
-    rcu_read_lock();
-    block = qemu_get_ram_block(addr);
-    block->fd = fd;
-    rcu_read_unlock();
+    if (block == NULL) {
+        rcu_read_lock();
+        block = qemu_get_ram_block(addr);
+        block->fd = fd;
+        rcu_read_unlock();
+    } else {
+        block->fd = fd;
+    }
 }
 
-void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
+void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t addr)
 {
-    RAMBlock *block;
+    RAMBlock *block = ram_block;
     void *ptr;
 
-    rcu_read_lock();
-    block = qemu_get_ram_block(addr);
-    ptr = ramblock_ptr(block, 0);
-    rcu_read_unlock();
+    if (block == NULL) {
+        rcu_read_lock();
+        block = qemu_get_ram_block(addr);
+        ptr = ramblock_ptr(block, 0);
+        rcu_read_unlock();
+    } else {
+        ptr = ramblock_ptr(block, 0);
+    }
     return ptr;
 }
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index e40f23b..5ba0af7 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -533,7 +533,8 @@ static void process_msg_shmem(IVShmemState *s, int fd, Error **errp)
     }
     memory_region_init_ram_ptr(&s->server_bar2, OBJECT(s),
                                "ivshmem.bar2", size, ptr);
-    qemu_set_ram_fd(memory_region_get_ram_addr(&s->server_bar2), fd);
+    qemu_set_ram_fd(s->server_bar2.ram_block,
+                    memory_region_get_ram_addr(&s->server_bar2), fd);
     s->ivshmem_bar2 = &s->server_bar2;
 }
 
@@ -940,7 +941,8 @@ static void ivshmem_exit(PCIDevice *dev)
                              strerror(errno));
             }
 
-            fd = qemu_get_ram_fd(memory_region_get_ram_addr(s->ivshmem_bar2));
+            fd = qemu_get_ram_fd(s->ivshmem_bar2->ram_block,
+                                 memory_region_get_ram_addr(s->ivshmem_bar2));
             close(fd);
         }
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 5914e85..4b93533 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -248,17 +248,18 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
     for (i = 0; i < dev->mem->nregions; ++i) {
         struct vhost_memory_region *reg = dev->mem->regions + i;
         ram_addr_t ram_addr;
+        MemoryRegion *mr;
 
         assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
-        qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
+        mr = qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
                                 &ram_addr);
-        fd = qemu_get_ram_fd(ram_addr);
+        fd = qemu_get_ram_fd(mr->ram_block, ram_addr);
         if (fd > 0) {
             msg.payload.memory.regions[fd_num].userspace_addr = reg->userspace_addr;
             msg.payload.memory.regions[fd_num].memory_size  = reg->memory_size;
             msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;
             msg.payload.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
-                (uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
+                (uintptr_t) qemu_get_ram_block_host_ptr(mr->ram_block, ram_addr);
             assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
             fds[fd_num++] = fd;
         }
@@ -622,11 +623,11 @@ static bool vhost_user_can_merge(struct vhost_dev *dev,
 
     mr = qemu_ram_addr_from_host((void *)(uintptr_t)start1, &ram_addr);
     assert(mr);
-    mfd = qemu_get_ram_fd(ram_addr);
+    mfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
 
     mr = qemu_ram_addr_from_host((void *)(uintptr_t)start2, &ram_addr);
     assert(mr);
-    rfd = qemu_get_ram_fd(ram_addr);
+    rfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
 
     return mfd == rfd;
 }
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5b6e1b8..4e1e5c3 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -105,9 +105,9 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
                                                     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);
+int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr);
+void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd);
+void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t addr);
 void qemu_ram_free(RAMBlock *block);
 
 int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
diff --git a/memory.c b/memory.c
index 239e6da..fa2b6e2 100644
--- a/memory.c
+++ b/memory.c
@@ -1641,7 +1641,8 @@ int memory_region_get_fd(MemoryRegion *mr)
 
     assert(mr->ram_block);
 
-    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
+    return qemu_get_ram_fd(mr->ram_block,
+                      memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
 }
 
 void *memory_region_get_ram_ptr(MemoryRegion *mr)
-- 
1.7.12.4

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

* Re: [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block()
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() Gonglei
@ 2016-04-29  7:48   ` Fam Zheng
  2016-04-29  8:57     ` Gonglei (Arei)
  0 siblings, 1 reply; 12+ messages in thread
From: Fam Zheng @ 2016-04-29  7:48 UTC (permalink / raw)
  To: Gonglei; +Cc: qemu-devel, pbonzini, peter.huangpeng

On Wed, 04/20 11:34, Gonglei wrote:
> On the one hand, we have already qemu_get_ram_block() whose function
> is similar. On the other hand, we can directly use mr->ram_block but
> searching RAMblock by ram_addr which is a kind of waste.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  exec.c                    | 37 +++++++++----------------------------
>  include/exec/cpu-common.h |  4 ++--
>  include/exec/ram_addr.h   |  2 +-
>  memory.c                  |  2 +-
>  migration/ram.c           |  2 +-
>  migration/savevm.c        |  4 ++--
>  6 files changed, 16 insertions(+), 35 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index c4f9036..101f0f4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1407,34 +1407,19 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)
>      }
>  }
>  
> -/* Called within an RCU critical section, or while the ramlist lock
> - * is held.
> - */
> -static RAMBlock *find_ram_block(ram_addr_t addr)
> -{
> -    RAMBlock *block;
> -
> -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> -        if (block->offset == addr) {
> -            return block;
> -        }
> -    }
> -
> -    return NULL;
> -}
> -
>  const char *qemu_ram_get_idstr(RAMBlock *rb)
>  {
>      return rb->idstr;
>  }
>  
>  /* Called with iothread lock held.  */
> -void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
> +void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
>  {
> -    RAMBlock *new_block, *block;
> +    RAMBlock *new_block = block;
> +    RAMBlock *old_block;
>  
>      rcu_read_lock();
> -    new_block = find_ram_block(addr);
> +
>      assert(new_block);
>      assert(!new_block->idstr[0]);
>  
> @@ -1447,8 +1432,9 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
>      }
>      pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
>  
> -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> -        if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
> +    QLIST_FOREACH_RCU(old_block, &ram_list.blocks, next) {
> +        if (old_block != new_block &&
> +            !strcmp(old_block->idstr, new_block->idstr)) {

Cosmetic point: maybe you don't need to touch these lines if you name the
function parameter to new_block.

Reviewed-by: Fam Zheng <famz@redhat.com>

>              fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
>                      new_block->idstr);
>              abort();
> @@ -1458,17 +1444,14 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
>  }
>  
>  /* Called with iothread lock held.  */
> -void qemu_ram_unset_idstr(ram_addr_t addr)
> +void qemu_ram_unset_idstr(RAMBlock *block)
>  {
> -    RAMBlock *block;
> -
>      /* FIXME: arch_init.c assumes that this is not called throughout
>       * migration.  Ignore the problem since hot-unplug during migration
>       * does not work anyway.
>       */
>  
>      rcu_read_lock();
> -    block = find_ram_block(addr);
>      if (block) {
>          memset(block->idstr, 0, sizeof(block->idstr));
>      }
> @@ -1492,10 +1475,8 @@ static int memory_try_enable_merging(void *addr, size_t len)
>   * resize callback to update device state and/or add assertions to detect
>   * misuse, if necessary.
>   */
> -int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp)
> +int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
>  {
> -    RAMBlock *block = find_ram_block(base);
> -
>      assert(block);
>  
>      newsize = HOST_PAGE_ALIGN(newsize);
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 9e839e5..187df3b 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -65,8 +65,8 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
>  RAMBlock *qemu_ram_block_by_name(const char *name);
>  RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
>                                     ram_addr_t *ram_addr, ram_addr_t *offset);
> -void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev);
> -void qemu_ram_unset_idstr(ram_addr_t addr);
> +void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
> +void qemu_ram_unset_idstr(RAMBlock *block);
>  const char *qemu_ram_get_idstr(RAMBlock *rb);
>  
>  void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5adf7a4..5b6e1b8 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -110,7 +110,7 @@ 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(RAMBlock *block);
>  
> -int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
> +int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
>  
>  #define DIRTY_CLIENTS_ALL     ((1 << DIRTY_MEMORY_NUM) - 1)
>  #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE))
> diff --git a/memory.c b/memory.c
> index f76f85d..239e6da 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1671,7 +1671,7 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp
>  {
>      assert(mr->ram_block);
>  
> -    qemu_ram_resize(memory_region_get_ram_addr(mr), newsize, errp);
> +    qemu_ram_resize(mr->ram_block, newsize, errp);
>  }
>  
>  static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as)
> diff --git a/migration/ram.c b/migration/ram.c
> index 3f05738..28b5dd8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2476,7 +2476,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>                      if (length != block->used_length) {
>                          Error *local_err = NULL;
>  
> -                        ret = qemu_ram_resize(block->offset, length,
> +                        ret = qemu_ram_resize(block, length,
>                                                &local_err);
>                          if (local_err) {
>                              error_report_err(local_err);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 16ba443..3a2e843 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2228,13 +2228,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
>  
>  void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>  {
> -    qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
> +    qemu_ram_set_idstr(mr->ram_block,
>                         memory_region_name(mr), dev);
>  }
>  
>  void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev)
>  {
> -    qemu_ram_unset_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
> +    qemu_ram_unset_idstr(mr->ram_block);
>  }
>  
>  void vmstate_register_ram_global(MemoryRegion *mr)
> -- 
> 1.7.12.4
> 
> 

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

* Re: [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement Gonglei
@ 2016-04-29  7:56   ` Fam Zheng
  2016-04-29  8:59     ` Gonglei (Arei)
  0 siblings, 1 reply; 12+ messages in thread
From: Fam Zheng @ 2016-04-29  7:56 UTC (permalink / raw)
  To: Gonglei; +Cc: qemu-devel, pbonzini, peter.huangpeng

In the subject line, s/adjuest/adjust/ and s/requiement/requirement/.

On Wed, 04/20 11:34, Gonglei wrote:
> qemu_ram_unset_idstr() don't need rcu lock anymore,

s/don't/doesn't/

> meanwhile adjuest the range of rcu lock in

s/adjuest/adjust/ or just s/adjuest/make/

> qemu_ram_set_idstr() as small as possible.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  exec.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 101f0f4..b0ecbcd 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1418,8 +1418,6 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
>      RAMBlock *new_block = block;
>      RAMBlock *old_block;
>  
> -    rcu_read_lock();
> -
>      assert(new_block);
>      assert(!new_block->idstr[0]);
>  
> @@ -1432,6 +1430,7 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
>      }
>      pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
>  
> +    rcu_read_lock();
>      QLIST_FOREACH_RCU(old_block, &ram_list.blocks, next) {
>          if (old_block != new_block &&
>              !strcmp(old_block->idstr, new_block->idstr)) {
> @@ -1443,7 +1442,6 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev)
>      rcu_read_unlock();
>  }
>  
> -/* Called with iothread lock held.  */

This still seems true to me, why removing?

>  void qemu_ram_unset_idstr(RAMBlock *block)
>  {
>      /* FIXME: arch_init.c assumes that this is not called throughout
> @@ -1451,11 +1449,9 @@ void qemu_ram_unset_idstr(RAMBlock *block)
>       * does not work anyway.
>       */
>  
> -    rcu_read_lock();
>      if (block) {
>          memset(block->idstr, 0, sizeof(block->idstr));
>      }
> -    rcu_read_unlock();
>  }
>  
>  static int memory_try_enable_merging(void *addr, size_t len)
> -- 
> 1.7.12.4
> 
> 

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

* Re: [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument Gonglei
@ 2016-04-29  8:06   ` Fam Zheng
  2016-04-29  9:03     ` Gonglei (Arei)
  0 siblings, 1 reply; 12+ messages in thread
From: Fam Zheng @ 2016-04-29  8:06 UTC (permalink / raw)
  To: Gonglei; +Cc: qemu-devel, pbonzini, peter.huangpeng

On Wed, 04/20 11:34, Gonglei wrote:
> In this way, we can avoid to invoke qemu_get_ram_block()
> at most time, which can save cpu cycle.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  exec.c                  | 48 ++++++++++++++++++++++++++++++------------------
>  hw/misc/ivshmem.c       |  6 ++++--
>  hw/virtio/vhost-user.c  | 11 ++++++-----
>  include/exec/ram_addr.h |  6 +++---
>  memory.c                |  3 ++-
>  5 files changed, 45 insertions(+), 29 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index b0ecbcd..f3348aa 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1813,37 +1813,49 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>  }
>  #endif /* !_WIN32 */
>  
> -int qemu_get_ram_fd(ram_addr_t addr)
> +int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr)
>  {
> -    RAMBlock *block;
> +    RAMBlock *block = ram_block;
>      int fd;
>  
> -    rcu_read_lock();
> -    block = qemu_get_ram_block(addr);
> -    fd = block->fd;
> -    rcu_read_unlock();
> +    if (block == NULL) {

Is this branch even used? It seems no caller passes in literal NULL in this
patch. Can the variables be NULL?

If not, I think we can simply drop addr.

> +        rcu_read_lock();
> +        block = qemu_get_ram_block(addr);
> +        fd = block->fd;
> +        rcu_read_unlock();
> +    } else {
> +        fd = block->fd;
> +    }
>      return fd;
>  }
>  
> -void qemu_set_ram_fd(ram_addr_t addr, int fd)
> +void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd)
>  {
> -    RAMBlock *block;
> +    RAMBlock *block = ram_block;
>  
> -    rcu_read_lock();
> -    block = qemu_get_ram_block(addr);
> -    block->fd = fd;
> -    rcu_read_unlock();
> +    if (block == NULL) {

Same here...

> +        rcu_read_lock();
> +        block = qemu_get_ram_block(addr);
> +        block->fd = fd;
> +        rcu_read_unlock();
> +    } else {
> +        block->fd = fd;
> +    }
>  }
>  
> -void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
> +void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t addr)
>  {
> -    RAMBlock *block;
> +    RAMBlock *block = ram_block;
>      void *ptr;
>  
> -    rcu_read_lock();
> -    block = qemu_get_ram_block(addr);
> -    ptr = ramblock_ptr(block, 0);
> -    rcu_read_unlock();
> +    if (block == NULL) {

And here.

> +        rcu_read_lock();
> +        block = qemu_get_ram_block(addr);
> +        ptr = ramblock_ptr(block, 0);
> +        rcu_read_unlock();
> +    } else {
> +        ptr = ramblock_ptr(block, 0);
> +    }
>      return ptr;
>  }
>  
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index e40f23b..5ba0af7 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -533,7 +533,8 @@ static void process_msg_shmem(IVShmemState *s, int fd, Error **errp)
>      }
>      memory_region_init_ram_ptr(&s->server_bar2, OBJECT(s),
>                                 "ivshmem.bar2", size, ptr);
> -    qemu_set_ram_fd(memory_region_get_ram_addr(&s->server_bar2), fd);
> +    qemu_set_ram_fd(s->server_bar2.ram_block,
> +                    memory_region_get_ram_addr(&s->server_bar2), fd);
>      s->ivshmem_bar2 = &s->server_bar2;
>  }
>  
> @@ -940,7 +941,8 @@ static void ivshmem_exit(PCIDevice *dev)
>                               strerror(errno));
>              }
>  
> -            fd = qemu_get_ram_fd(memory_region_get_ram_addr(s->ivshmem_bar2));
> +            fd = qemu_get_ram_fd(s->ivshmem_bar2->ram_block,
> +                                 memory_region_get_ram_addr(s->ivshmem_bar2));
>              close(fd);
>          }
>  
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 5914e85..4b93533 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -248,17 +248,18 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
>      for (i = 0; i < dev->mem->nregions; ++i) {
>          struct vhost_memory_region *reg = dev->mem->regions + i;
>          ram_addr_t ram_addr;
> +        MemoryRegion *mr;
>  
>          assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> -        qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
> +        mr = qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
>                                  &ram_addr);
> -        fd = qemu_get_ram_fd(ram_addr);
> +        fd = qemu_get_ram_fd(mr->ram_block, ram_addr);
>          if (fd > 0) {
>              msg.payload.memory.regions[fd_num].userspace_addr = reg->userspace_addr;
>              msg.payload.memory.regions[fd_num].memory_size  = reg->memory_size;
>              msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;
>              msg.payload.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
> -                (uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
> +                (uintptr_t) qemu_get_ram_block_host_ptr(mr->ram_block, ram_addr);
>              assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>              fds[fd_num++] = fd;
>          }
> @@ -622,11 +623,11 @@ static bool vhost_user_can_merge(struct vhost_dev *dev,
>  
>      mr = qemu_ram_addr_from_host((void *)(uintptr_t)start1, &ram_addr);
>      assert(mr);
> -    mfd = qemu_get_ram_fd(ram_addr);
> +    mfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
>  
>      mr = qemu_ram_addr_from_host((void *)(uintptr_t)start2, &ram_addr);
>      assert(mr);
> -    rfd = qemu_get_ram_fd(ram_addr);
> +    rfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
>  
>      return mfd == rfd;
>  }
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5b6e1b8..4e1e5c3 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -105,9 +105,9 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
>                                                      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);
> +int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr);
> +void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd);
> +void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t addr);
>  void qemu_ram_free(RAMBlock *block);
>  
>  int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
> diff --git a/memory.c b/memory.c
> index 239e6da..fa2b6e2 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1641,7 +1641,8 @@ int memory_region_get_fd(MemoryRegion *mr)
>  
>      assert(mr->ram_block);
>  
> -    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
> +    return qemu_get_ram_fd(mr->ram_block,
> +                      memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
>  }
>  
>  void *memory_region_get_ram_ptr(MemoryRegion *mr)
> -- 
> 1.7.12.4
> 
> 

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

* Re: [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block()
  2016-04-29  7:48   ` Fam Zheng
@ 2016-04-29  8:57     ` Gonglei (Arei)
  0 siblings, 0 replies; 12+ messages in thread
From: Gonglei (Arei) @ 2016-04-29  8:57 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel, pbonzini, Huangpeng (Peter)

Hi,

> >
> > @@ -1447,8 +1432,9 @@ void qemu_ram_set_idstr(ram_addr_t addr, const
> char *name, DeviceState *dev)
> >      }
> >      pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
> >
> > -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> > -        if (block != new_block && !strcmp(block->idstr, new_block->idstr))
> {
> > +    QLIST_FOREACH_RCU(old_block, &ram_list.blocks, next) {
> > +        if (old_block != new_block &&
> > +            !strcmp(old_block->idstr, new_block->idstr)) {
> 
> Cosmetic point: maybe you don't need to touch these lines if you name the
> function parameter to new_block.
> 
Yes, we can.

> Reviewed-by: Fam Zheng <famz@redhat.com>
> 
Thanks.


Regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement
  2016-04-29  7:56   ` Fam Zheng
@ 2016-04-29  8:59     ` Gonglei (Arei)
  0 siblings, 0 replies; 12+ messages in thread
From: Gonglei (Arei) @ 2016-04-29  8:59 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel, pbonzini, Huangpeng (Peter)

> From: Fam Zheng [mailto:famz@redhat.com]
> Sent: Friday, April 29, 2016 3:57 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; Huangpeng (Peter)
> Subject: Re: [PATCH 2/3] exec: adjuest rcu_read_lock requiement
> 
> In the subject line, s/adjuest/adjust/ and s/requiement/requirement/.
> 
> On Wed, 04/20 11:34, Gonglei wrote:
> > qemu_ram_unset_idstr() don't need rcu lock anymore,
> 
> s/don't/doesn't/
> 
> > meanwhile adjuest the range of rcu lock in
> 
> s/adjuest/adjust/ or just s/adjuest/make/
> 
OK, thanks :)

> > qemu_ram_set_idstr() as small as possible.
> >

Regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument
  2016-04-29  8:06   ` Fam Zheng
@ 2016-04-29  9:03     ` Gonglei (Arei)
  0 siblings, 0 replies; 12+ messages in thread
From: Gonglei (Arei) @ 2016-04-29  9:03 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel, pbonzini, Huangpeng (Peter)

> From: Fam Zheng [mailto:famz@redhat.com]
> Sent: Friday, April 29, 2016 4:07 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; Huangpeng (Peter)
> Subject: Re: [PATCH 3/3] memory: pass RAMBlock as a agrument
> 
> On Wed, 04/20 11:34, Gonglei wrote:
> > In this way, we can avoid to invoke qemu_get_ram_block()
> > at most time, which can save cpu cycle.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  exec.c                  | 48
> ++++++++++++++++++++++++++++++------------------
> >  hw/misc/ivshmem.c       |  6 ++++--
> >  hw/virtio/vhost-user.c  | 11 ++++++-----
> >  include/exec/ram_addr.h |  6 +++---
> >  memory.c                |  3 ++-
> >  5 files changed, 45 insertions(+), 29 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index b0ecbcd..f3348aa 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1813,37 +1813,49 @@ void qemu_ram_remap(ram_addr_t addr,
> ram_addr_t length)
> >  }
> >  #endif /* !_WIN32 */
> >
> > -int qemu_get_ram_fd(ram_addr_t addr)
> > +int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr)
> >  {
> > -    RAMBlock *block;
> > +    RAMBlock *block = ram_block;
> >      int fd;
> >
> > -    rcu_read_lock();
> > -    block = qemu_get_ram_block(addr);
> > -    fd = block->fd;
> > -    rcu_read_unlock();
> > +    if (block == NULL) {
> 
> Is this branch even used? It seems no caller passes in literal NULL in this
> patch. Can the variables be NULL?
> 
I'm not sure. But currently the variables never be NULL.

> If not, I think we can simply drop addr.
> 
Yes, add assert(block) at the front of those functions.

Regards,
-Gonglei

> > +        rcu_read_lock();
> > +        block = qemu_get_ram_block(addr);
> > +        fd = block->fd;
> > +        rcu_read_unlock();
> > +    } else {
> > +        fd = block->fd;
> > +    }
> >      return fd;
> >  }
> >
> > -void qemu_set_ram_fd(ram_addr_t addr, int fd)
> > +void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd)
> >  {
> > -    RAMBlock *block;
> > +    RAMBlock *block = ram_block;
> >
> > -    rcu_read_lock();
> > -    block = qemu_get_ram_block(addr);
> > -    block->fd = fd;
> > -    rcu_read_unlock();
> > +    if (block == NULL) {
> 
> Same here...
> 
> > +        rcu_read_lock();
> > +        block = qemu_get_ram_block(addr);
> > +        block->fd = fd;
> > +        rcu_read_unlock();
> > +    } else {
> > +        block->fd = fd;
> > +    }
> >  }
> >
> > -void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
> > +void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t
> addr)
> >  {
> > -    RAMBlock *block;
> > +    RAMBlock *block = ram_block;
> >      void *ptr;
> >
> > -    rcu_read_lock();
> > -    block = qemu_get_ram_block(addr);
> > -    ptr = ramblock_ptr(block, 0);
> > -    rcu_read_unlock();
> > +    if (block == NULL) {
> 
> And here.
> 
> > +        rcu_read_lock();
> > +        block = qemu_get_ram_block(addr);
> > +        ptr = ramblock_ptr(block, 0);
> > +        rcu_read_unlock();
> > +    } else {
> > +        ptr = ramblock_ptr(block, 0);
> > +    }
> >      return ptr;
> >  }
> >
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index e40f23b..5ba0af7 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -533,7 +533,8 @@ static void process_msg_shmem(IVShmemState *s,
> int fd, Error **errp)
> >      }
> >      memory_region_init_ram_ptr(&s->server_bar2, OBJECT(s),
> >                                 "ivshmem.bar2", size, ptr);
> > -    qemu_set_ram_fd(memory_region_get_ram_addr(&s->server_bar2),
> fd);
> > +    qemu_set_ram_fd(s->server_bar2.ram_block,
> > +                    memory_region_get_ram_addr(&s->server_bar2),
> fd);
> >      s->ivshmem_bar2 = &s->server_bar2;
> >  }
> >
> > @@ -940,7 +941,8 @@ static void ivshmem_exit(PCIDevice *dev)
> >                               strerror(errno));
> >              }
> >
> > -            fd =
> qemu_get_ram_fd(memory_region_get_ram_addr(s->ivshmem_bar2));
> > +            fd = qemu_get_ram_fd(s->ivshmem_bar2->ram_block,
> > +
> memory_region_get_ram_addr(s->ivshmem_bar2));
> >              close(fd);
> >          }
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 5914e85..4b93533 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -248,17 +248,18 @@ static int vhost_user_set_mem_table(struct
> vhost_dev *dev,
> >      for (i = 0; i < dev->mem->nregions; ++i) {
> >          struct vhost_memory_region *reg = dev->mem->regions + i;
> >          ram_addr_t ram_addr;
> > +        MemoryRegion *mr;
> >
> >          assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> > -        qemu_ram_addr_from_host((void
> *)(uintptr_t)reg->userspace_addr,
> > +        mr = qemu_ram_addr_from_host((void
> *)(uintptr_t)reg->userspace_addr,
> >                                  &ram_addr);
> > -        fd = qemu_get_ram_fd(ram_addr);
> > +        fd = qemu_get_ram_fd(mr->ram_block, ram_addr);
> >          if (fd > 0) {
> >              msg.payload.memory.regions[fd_num].userspace_addr =
> reg->userspace_addr;
> >              msg.payload.memory.regions[fd_num].memory_size  =
> reg->memory_size;
> >              msg.payload.memory.regions[fd_num].guest_phys_addr =
> reg->guest_phys_addr;
> >              msg.payload.memory.regions[fd_num].mmap_offset =
> reg->userspace_addr -
> > -                (uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
> > +                (uintptr_t)
> qemu_get_ram_block_host_ptr(mr->ram_block, ram_addr);
> >              assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> >              fds[fd_num++] = fd;
> >          }
> > @@ -622,11 +623,11 @@ static bool vhost_user_can_merge(struct
> vhost_dev *dev,
> >
> >      mr = qemu_ram_addr_from_host((void *)(uintptr_t)start1,
> &ram_addr);
> >      assert(mr);
> > -    mfd = qemu_get_ram_fd(ram_addr);
> > +    mfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
> >
> >      mr = qemu_ram_addr_from_host((void *)(uintptr_t)start2,
> &ram_addr);
> >      assert(mr);
> > -    rfd = qemu_get_ram_fd(ram_addr);
> > +    rfd = qemu_get_ram_fd(mr->ram_block, ram_addr);
> >
> >      return mfd == rfd;
> >  }
> > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > index 5b6e1b8..4e1e5c3 100644
> > --- a/include/exec/ram_addr.h
> > +++ b/include/exec/ram_addr.h
> > @@ -105,9 +105,9 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t
> size, ram_addr_t max_size,
> >                                                      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);
> > +int qemu_get_ram_fd(RAMBlock *ram_block, ram_addr_t addr);
> > +void qemu_set_ram_fd(RAMBlock *ram_block, ram_addr_t addr, int fd);
> > +void *qemu_get_ram_block_host_ptr(RAMBlock *ram_block, ram_addr_t
> addr);
> >  void qemu_ram_free(RAMBlock *block);
> >
> >  int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error
> **errp);
> > diff --git a/memory.c b/memory.c
> > index 239e6da..fa2b6e2 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1641,7 +1641,8 @@ int memory_region_get_fd(MemoryRegion *mr)
> >
> >      assert(mr->ram_block);
> >
> > -    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
> > +    return qemu_get_ram_fd(mr->ram_block,
> > +                      memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
> >  }
> >
> >  void *memory_region_get_ram_ptr(MemoryRegion *mr)
> > --
> > 1.7.12.4
> >
> >

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

* Re: [Qemu-devel] [PATCH 0/3] memory: some little optimzation
  2016-04-20  3:34 [Qemu-devel] [PATCH 0/3] memory: some little optimzation Gonglei
                   ` (2 preceding siblings ...)
  2016-04-20  3:34 ` [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument Gonglei
@ 2016-05-09 11:41 ` Paolo Bonzini
  2016-05-10  2:05   ` Gonglei (Arei)
  3 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Gonglei, qemu-devel; +Cc: famz, peter.huangpeng



On 20/04/2016 05:34, Gonglei wrote:
> Please see the details in each patch.
> 
> Gonglei (3):
>   memory: drop find_ram_block()
>   exec: adjuest rcu_read_lock requiement
>   memory: pass RAMBlock as a agrument

Hi,

patches 1 and 2 look good.  Please send v2 according to Fam remarks.

For patch 3, I think it's okay if memory.c touches block->fd directly.
Then the functions can be just removed.  I have a patch pending for 2.7
that I'll send in the next weeks.

Thanks,

Paolo

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

* Re: [Qemu-devel] [PATCH 0/3] memory: some little optimzation
  2016-05-09 11:41 ` [Qemu-devel] [PATCH 0/3] memory: some little optimzation Paolo Bonzini
@ 2016-05-10  2:05   ` Gonglei (Arei)
  0 siblings, 0 replies; 12+ messages in thread
From: Gonglei (Arei) @ 2016-05-10  2:05 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: famz, Huangpeng (Peter)

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Subject: Re: [PATCH 0/3] memory: some little optimzation
> 
> 
> 
> On 20/04/2016 05:34, Gonglei wrote:
> > Please see the details in each patch.
> >
> > Gonglei (3):
> >   memory: drop find_ram_block()
> >   exec: adjuest rcu_read_lock requiement
> >   memory: pass RAMBlock as a agrument
> 
> Hi,
> 
> patches 1 and 2 look good.  Please send v2 according to Fam remarks.
> 
> For patch 3, I think it's okay if memory.c touches block->fd directly.
> Then the functions can be just removed.  I have a patch pending for 2.7
> that I'll send in the next weeks.
> 
Done, thank you.

Regards,
-Gonglei

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

end of thread, other threads:[~2016-05-10  2:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-20  3:34 [Qemu-devel] [PATCH 0/3] memory: some little optimzation Gonglei
2016-04-20  3:34 ` [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() Gonglei
2016-04-29  7:48   ` Fam Zheng
2016-04-29  8:57     ` Gonglei (Arei)
2016-04-20  3:34 ` [Qemu-devel] [PATCH 2/3] exec: adjuest rcu_read_lock requiement Gonglei
2016-04-29  7:56   ` Fam Zheng
2016-04-29  8:59     ` Gonglei (Arei)
2016-04-20  3:34 ` [Qemu-devel] [PATCH 3/3] memory: pass RAMBlock as a agrument Gonglei
2016-04-29  8:06   ` Fam Zheng
2016-04-29  9:03     ` Gonglei (Arei)
2016-05-09 11:41 ` [Qemu-devel] [PATCH 0/3] memory: some little optimzation Paolo Bonzini
2016-05-10  2:05   ` Gonglei (Arei)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.