Re: [Qemu-devel] [PATCH 03/15] qom: introduce reclaimer to release obj

[Paolo Bonzini]

Il 08/08/2012 08:25, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <address@hidden>
> 
> Collect unused object and release them at caller demand.
> 
> Signed-off-by: Liu Ping Fan <address@hidden>
> ---
>  include/qemu/reclaimer.h |   28 ++++++++++++++++++++++
>  main-loop.c              |    5 ++++
>  qemu-tool.c              |    5 ++++
>  qom/Makefile.objs        |    2 +-
>  qom/reclaimer.c          |   58 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 97 insertions(+), 1 deletions(-)
>  create mode 100644 include/qemu/reclaimer.h
>  create mode 100644 qom/reclaimer.c
> 
> diff --git a/include/qemu/reclaimer.h b/include/qemu/reclaimer.h
> new file mode 100644
> index 0000000..9307e93
> --- /dev/null
> +++ b/include/qemu/reclaimer.h
> @@ -0,0 +1,28 @@
> +/*
> + * QEMU reclaimer
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_RECLAIMER
> +#define QEMU_RECLAIMER
> +
> +typedef void ReleaseHandler(void *opaque);
> +typedef struct Chunk {
> +    QLIST_ENTRY(Chunk) list;
> +    void *opaque;
> +    ReleaseHandler *release;
> +} Chunk;
> +
> +typedef struct ChunkHead {
> +        struct Chunk *lh_first;
> +} ChunkHead;
> +
> +void reclaimer_enqueue(ChunkHead *head, void *opaque, ReleaseHandler 
> *release);
> +void reclaimer_worker(ChunkHead *head);
> +void qemu_reclaimer_enqueue(void *opaque, ReleaseHandler *release);
> +void qemu_reclaimer(void);

So "enqueue" is call_rcu and qemu_reclaimer marks a quiescent state +
empties the pending call_rcu.

But what's the difference between the two pairs of APIs?

> +#endif
> diff --git a/main-loop.c b/main-loop.c
> index eb3b6e6..be9d095 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -26,6 +26,7 @@
>  #include "qemu-timer.h"
>  #include "slirp/slirp.h"
>  #include "main-loop.h"
> +#include "qemu/reclaimer.h"
>  
>  #ifndef _WIN32
>  
> @@ -505,5 +506,9 @@ int main_loop_wait(int nonblocking)
>         them.  */
>      qemu_bh_poll();
>  
> +    /* ref to device from iohandler/bh/timer do not obey the rules, so delay
> +     * reclaiming until now.
> +     */
> +    qemu_reclaimer();
>      return ret;
>  }
> diff --git a/qemu-tool.c b/qemu-tool.c
> index 318c5fc..f5fe319 100644
> --- a/qemu-tool.c
> +++ b/qemu-tool.c
> @@ -21,6 +21,7 @@
>  #include "main-loop.h"
>  #include "qemu_socket.h"
>  #include "slirp/libslirp.h"
> +#include "qemu/reclaimer.h"
>  
>  #include <sys/time.h>
>  
> @@ -75,6 +76,10 @@ void qemu_mutex_unlock_iothread(void)
>  {
>  }
>  
> +void qemu_reclaimer(void)
> +{
> +}
> +
>  int use_icount;
>  
>  void qemu_clock_warp(QEMUClock *clock)
> diff --git a/qom/Makefile.objs b/qom/Makefile.objs
> index 5ef060a..a579261 100644
> --- a/qom/Makefile.objs
> +++ b/qom/Makefile.objs
> @@ -1,4 +1,4 @@
> -qom-obj-y = object.o container.o qom-qobject.o
> +qom-obj-y = object.o container.o qom-qobject.o reclaimer.o
>  qom-obj-twice-y = cpu.o
>  common-obj-y = $(qom-obj-twice-y)
>  user-obj-y = $(qom-obj-twice-y)
> diff --git a/qom/reclaimer.c b/qom/reclaimer.c
> new file mode 100644
> index 0000000..6cb53e3
> --- /dev/null
> +++ b/qom/reclaimer.c
> @@ -0,0 +1,58 @@
> +/*
> + * QEMU reclaimer
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu-common.h"
> +#include "qemu-thread.h"
> +#include "main-loop.h"
> +#include "qemu-queue.h"
> +#include "qemu/reclaimer.h"
> +
> +static struct QemuMutex reclaimer_lock;
> +static QLIST_HEAD(rcl, Chunk) reclaimer_list;
> +
> +void reclaimer_enqueue(ChunkHead *head, void *opaque, ReleaseHandler 
> *release)
> +{
> +    Chunk *r = g_malloc0(sizeof(Chunk));
> +    r->opaque = opaque;
> +    r->release = release;
> +    QLIST_INSERT_HEAD_RCU(head, r, list);
> +}

No lock?

> +void reclaimer_worker(ChunkHead *head)
> +{
> +    Chunk *cur, *next;
> +
> +    QLIST_FOREACH_SAFE(cur, head, list, next) {
> +        QLIST_REMOVE(cur, list);
> +        cur->release(cur->opaque);
> +        g_free(cur);
> +    }

QLIST_REMOVE needs a lock too, so using the lockless
QLIST_INSERT_HEAD_RCU is not necessary.

> +}
> +
> +void qemu_reclaimer_enqueue(void *opaque, ReleaseHandler *release)
> +{
> +    Chunk *r = g_malloc0(sizeof(Chunk));
> +    r->opaque = opaque;
> +    r->release = release;
> +    qemu_mutex_lock(&reclaimer_lock);
> +    QLIST_INSERT_HEAD_RCU(&reclaimer_list, r, list);
> +    qemu_mutex_unlock(&reclaimer_lock);
> +}
> +
> +
> +void qemu_reclaimer(void)
> +{
> +    Chunk *cur, *next;
> +
> +    QLIST_FOREACH_SAFE(cur, &reclaimer_list, list, next) {
> +        QLIST_REMOVE(cur, list);
> +        cur->release(cur->opaque);
> +        g_free(cur);
> +    }

Same here.

> +}
>