Hi
On Thu, May 26, 2016 at 10:49 AM, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Remove direct uses of ram_addr_t and optimize memory_region_{get,set}_fd
> now that a MemoryRegion knows its RAMBlock directly.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> exec.c | 34 ----------------------------------
> hw/misc/ivshmem.c | 5 ++---
> hw/virtio/vhost-user.c | 15 +++++++--------
> include/exec/memory.h | 11 +++++++++++
> include/exec/ram_addr.h | 3 ---
> memory.c | 21 +++++++++++++++++----
> 6 files changed, 37 insertions(+), 52 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index a3a93ae..3330e7d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1815,40 +1815,6 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
> }
> #endif /* !_WIN32 */
>
> -int qemu_get_ram_fd(ram_addr_t addr)
> -{
> - RAMBlock *block;
> - int fd;
> -
> - rcu_read_lock();
> - block = qemu_get_ram_block(addr);
> - fd = block->fd;
> - rcu_read_unlock();
> - return fd;
> -}
> -
> -void qemu_set_ram_fd(ram_addr_t addr, int fd)
> -{
> - RAMBlock *block;
> -
> - rcu_read_lock();
> - block = qemu_get_ram_block(addr);
> - block->fd = fd;
> - rcu_read_unlock();
> -}
> -
> -void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
> -{
> - RAMBlock *block;
> - void *ptr;
> -
> - rcu_read_lock();
> - block = qemu_get_ram_block(addr);
> - ptr = ramblock_ptr(block, 0);
> - rcu_read_unlock();
> - return ptr;
> -}
> -
> /* Return a host pointer to ram allocated with qemu_ram_alloc.
> * This should not be used for general purpose DMA. Use address_space_map
> * or address_space_rw instead. For local memory (e.g. video ram) that the
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index e40f23b..90be9f7 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -33,7 +33,6 @@
> #include "sysemu/hostmem.h"
> #include "sysemu/qtest.h"
> #include "qapi/visitor.h"
> -#include "exec/ram_addr.h"
>
> #include "hw/misc/ivshmem.h"
>
> @@ -533,7 +532,7 @@ 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);
> + memory_region_set_fd(&s->server_bar2, fd);
> s->ivshmem_bar2 = &s->server_bar2;
> }
>
> @@ -940,7 +939,7 @@ static void ivshmem_exit(PCIDevice *dev)
> strerror(errno));
> }
>
> - fd =
> qemu_get_ram_fd(memory_region_get_ram_addr(s->ivshmem_bar2));
> + fd = memory_region_get_fd(s->ivshmem_bar2);
> close(fd);
> }
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 5914e85..41908c0 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,
> - &ram_addr);
> - fd = qemu_get_ram_fd(ram_addr);
> + mr = qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
> + &ram_addr);
> + fd = memory_region_get_fd(mr);
> 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) memory_region_get_ram_ptr(mr);
> assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> fds[fd_num++] = fd;
> }
> @@ -621,12 +622,10 @@ static bool vhost_user_can_merge(struct vhost_dev *dev,
> MemoryRegion *mr;
>
> mr = qemu_ram_addr_from_host((void *)(uintptr_t)start1, &ram_addr);
> - assert(mr);
> - mfd = qemu_get_ram_fd(ram_addr);
> + mfd = memory_region_get_fd(mr);
>
> mr = qemu_ram_addr_from_host((void *)(uintptr_t)start2, &ram_addr);
> - assert(mr);
> - rfd = qemu_get_ram_fd(ram_addr);
> + rfd = memory_region_get_fd(mr);
>
You get rid of a few assert in the patch, any particular reason?
> return mfd == rfd;
> }
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index f649697..1678494 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -667,6 +667,17 @@ static inline bool memory_region_is_rom(MemoryRegion *mr)
> int memory_region_get_fd(MemoryRegion *mr);
>
> /**
> + * memory_region_set_fd: Mark a RAM memory region as backed by a
> + * file descriptor.
> + *
> + * This function is typically used after memory_region_init_ram_ptr().
> + *
> + * @mr: the memory region being queried.
...being updated
> + * @fd: the file descriptor that backs @mr.
> + */
> +void memory_region_set_fd(MemoryRegion *mr, int fd);
> +
> +/**
> * memory_region_get_ram_ptr: Get a pointer into a RAM memory region.
> *
> * Returns a host pointer to a RAM memory region (created with
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5b6e1b8..2a9465d 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -105,9 +105,6 @@ 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);
> 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 0f52522..d6a4a68 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1626,13 +1626,26 @@ void memory_region_reset_dirty(MemoryRegion *mr,
> hwaddr addr,
>
> int memory_region_get_fd(MemoryRegion *mr)
> {
> - if (mr->alias) {
> - return memory_region_get_fd(mr->alias);
> + int fd;
> +
> + rcu_read_lock();
> + while (mr->alias) {
> + mr = mr->alias;
> }
> + fd = mr->ram_block->fd;
> + rcu_read_unlock();
>
> - assert(mr->ram_block);
> + return fd;
> +}
>
> - return qemu_get_ram_fd(memory_region_get_ram_addr(mr));
> +void memory_region_set_fd(MemoryRegion *mr, int fd)
> +{
> + rcu_read_lock();
> + while (mr->alias) {
> + mr = mr->alias;
> + }
> + mr->ram_block->fd = fd;
> + rcu_read_unlock();
> }
>
> void *memory_region_get_ram_ptr(MemoryRegion *mr)
> --
> 2.5.5
>
>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
--
Marc-André Lureau
