Patchwork [14/15] qom: object_unref call reclaimer

login
register
mail settings
Submitter pingfan liu
Date Aug. 8, 2012, 6:25 a.m.
Message ID <1344407156-25562-15-git-send-email-qemulist@gmail.com>
Download mbox | patch
Permalink /patch/175868/
State New
Headers show

Comments

pingfan liu - Aug. 8, 2012, 6:25 a.m.
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

iohandler/bh/timer may use DeviceState when its refcnt=0,
postpone the reclaimer till they have done with it.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 qom/object.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Paolo Bonzini - Aug. 8, 2012, 9:40 a.m.
Il 08/08/2012 08:25, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> 
> iohandler/bh/timer may use DeviceState when its refcnt=0,
> postpone the reclaimer till they have done with it.
> 
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
>  qom/object.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 822bdb7..1452b1b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -23,6 +23,8 @@
>  #include "qbool.h"
>  #include "qint.h"
>  #include "qstring.h"
> +#include "hw/qdev.h"
> +#include "qemu/reclaimer.h"
>  
>  #define MAX_INTERFACES 32
>  
> @@ -646,7 +648,12 @@ void object_unref(Object *obj)
>  {
>      g_assert(atomic_read(&obj->ref) > 0);
>      if (atomic_dec_and_test(&obj->ref)) {
> -        object_finalize(obj);
> +        /* fixme, maybe introduce obj->finalze to make this more elegant */
> +        if (object_dynamic_cast(obj, TYPE_DEVICE) != NULL) {
> +            qemu_reclaimer_enqueue(obj, object_finalize);
> +        } else {
> +            object_finalize(obj);
> +        }
>      }
>  }
>  
> 

Just do this unconditionally.

Paolo
Marcelo Tosatti - Aug. 13, 2012, 6:56 p.m.
On Wed, Aug 08, 2012 at 02:25:55PM +0800, Liu Ping Fan wrote:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> 
> iohandler/bh/timer may use DeviceState when its refcnt=0,
> postpone the reclaimer till they have done with it.
> 
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
>  qom/object.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 822bdb7..1452b1b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -23,6 +23,8 @@
>  #include "qbool.h"
>  #include "qint.h"
>  #include "qstring.h"
> +#include "hw/qdev.h"
> +#include "qemu/reclaimer.h"
>  
>  #define MAX_INTERFACES 32
>  
> @@ -646,7 +648,12 @@ void object_unref(Object *obj)
>  {
>      g_assert(atomic_read(&obj->ref) > 0);
>      if (atomic_dec_and_test(&obj->ref)) {
> -        object_finalize(obj);
> +        /* fixme, maybe introduce obj->finalze to make this more elegant */
> +        if (object_dynamic_cast(obj, TYPE_DEVICE) != NULL) {
> +            qemu_reclaimer_enqueue(obj, object_finalize);
> +        } else {
> +            object_finalize(obj);
> +        }
>      }

As mentioned, removal under the original plan is combination of
per-device lock and object reference. An initial step is parallel mmio
only, with everything else under the big lock.

Again, _only_ parallel mmio lookup as a first target is a good goal,
IMO.

Patch

diff --git a/qom/object.c b/qom/object.c
index 822bdb7..1452b1b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -23,6 +23,8 @@ 
 #include "qbool.h"
 #include "qint.h"
 #include "qstring.h"
+#include "hw/qdev.h"
+#include "qemu/reclaimer.h"
 
 #define MAX_INTERFACES 32
 
@@ -646,7 +648,12 @@  void object_unref(Object *obj)
 {
     g_assert(atomic_read(&obj->ref) > 0);
     if (atomic_dec_and_test(&obj->ref)) {
-        object_finalize(obj);
+        /* fixme, maybe introduce obj->finalze to make this more elegant */
+        if (object_dynamic_cast(obj, TYPE_DEVICE) != NULL) {
+            qemu_reclaimer_enqueue(obj, object_finalize);
+        } else {
+            object_finalize(obj);
+        }
     }
 }