Re: [Qemu-devel] [PATCH v2 4/6] hostmem: hostmem listener pin RAM-Device by refcnt

[Stefan Hajnoczi]

On Fri, May 03, 2013 at 10:45:20AM +0800, Liu Ping Fan wrote:
> From: Liu Ping Fan <address@hidden>
> 
> With ref()/unref() interface of MemoryRegion, we can pin RAM-Device
> when using its memory, and release it when done.
> 
> Signed-off-by: Liu Ping Fan <address@hidden>
> ---
>  hw/virtio/dataplane/hostmem.c         |   44 +++++++++++++++++++++++++++-----
>  hw/virtio/dataplane/vring.c           |    8 +++---
>  include/hw/virtio/dataplane/hostmem.h |    4 ++-
>  3 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
> index 1fd3e06..0e28dfc 100644
> --- a/hw/virtio/dataplane/hostmem.c
> +++ b/hw/virtio/dataplane/hostmem.c
> @@ -42,18 +42,28 @@ static void hostmem_ref(HostMem *hostmem)
>  
>  static void hostmem_unref(HostMem *hostmem)
>  {
> -    int t;
> +    int i, t;
> +    HostMemRegion *hmr;
>  
>      t = __sync_sub_and_fetch(&hostmem->ref, 1);
>      assert(t >= 0);
>      if (!t) {
> +        for (i = 0; i < hostmem->num_current_regions; i++) {
> +            hmr = &hostmem->current_regions[i];
> +            /* Fix me, when memory hotunplug implement
> +             * assert(hmr->mr_ops->unref)
> +             */
> +            if (hmr->mr->ops && hmr->mr->ops->unref) {
> +                hmr->mr->ops->unref();
> +            }

This patch should use memory_region_ref()/unref() which you introduced
in the previous patch instead of open-coding this.

> @@ -158,6 +176,18 @@ static void 
> hostmem_listener_append_region(MemoryListener *listener,
>      hostmem_append_new_region(as_mem->next_hostmem, section);
>  }
>  
> +static void hostmem_listener_append_region(MemoryListener *listener,
> +                                           MemoryRegionSection *section)
> +{
> +    hostmem_listener_nop_region(listener, section);
> +    /* Fix me, when memory hotunplug implement
> +     * assert(section->mr->ops->ref)
> +     */
> +    if (section->mr->ops && section->mr->ops->ref) {
> +        section->mr->ops->ref();
> +    }
> +}

Why does append increment the refcount while nop does not?

It seems that we always need to increment the memory region refcount
since we're building a completely new hostmem.  The refcount ownership
is not passed from the current hostmen to the next hostmem, all memory
regions are released in hostmem_unref().