[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block()
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] memory: drop find_ram_block() |
Date: |
Fri, 29 Apr 2016 15:48:33 +0800 |
User-agent: |
Mutt/1.6.0 (2016-04-01) |
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 <address@hidden>
> ---
> 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 <address@hidden>
> 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
>
>