On Thu, Jul 4, 2013 at 12:36 AM, Andreas Färber <afaer...@suse.de> wrote: > Am 03.07.2013 03:23, schrieb liu ping fan: >> On Wed, Jul 3, 2013 at 12:36 AM, Anthony Liguori <anth...@codemonkey.ws> >> wrote: >>> Paolo Bonzini <pbonz...@redhat.com> writes: >>> >>>> Il 02/07/2013 16:47, Anthony Liguori ha scritto: >>>>> Jan Kiszka <jan.kis...@siemens.com> writes: >>>>> >>>>>> Objects can soon be referenced/dereference outside the BQL. So we need >>>>>> to use atomics in object_ref/unref. >>>>>> >>>>>> Based on patch by Liu Ping Fan. >>>>>> >>>>>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com> >>>>>> --- >>>>>> qom/object.c | 5 ++--- >>>>>> 1 files changed, 2 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/qom/object.c b/qom/object.c >>>>>> index 803b94b..a76a30b 100644 >>>>>> --- a/qom/object.c >>>>>> +++ b/qom/object.c >>>>>> @@ -683,16 +683,15 @@ GSList *object_class_get_list(const char >>>>>> *implements_type, >>>>>> >>>>>> void object_ref(Object *obj) >>>>>> { >>>>>> - obj->ref++; >>>>>> + __sync_fetch_and_add(&obj->ref, 1); >>>>>> } >>>>>> >>>>>> void object_unref(Object *obj) >>>>>> { >>>>>> g_assert(obj->ref > 0); >>>>>> - obj->ref--; >>>>>> >>>>>> /* parent always holds a reference to its children */ >>>>>> - if (obj->ref == 0) { >>>>>> + if (__sync_sub_and_fetch(&obj->ref, 1) == 0) { >>>>>> object_finalize(obj); >>>>>> } >>>>>> } >>>>> >>>>> Should we introduce something akin to kref now that referencing counting >>>>> has gotten fancy? >>>> >>>> I'm not a big fan of kref (it seems _too_ thin a wrapper to me, i.e. it >>>> doesn't really wrap enough to be useful), but I wouldn't oppose it if >>>> someone else does it. >>> >>> I had honestly hoped Object was light enough to be used for this >>> purpose. What do you think? >>> >> I think it is a good idea. So we can consider the object_finalize() as >> the place to release everything. Take the DeviceState as example, we >> will have >> >> -- >8 -- >> Subject: [PATCH] qom: delay DeviceState destructor until object finialize >> >> Until refcnt->0, we know that the DeviceState can be safely dropped, >> so put the destructor there. >> >> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com> > > It would be nice to get CC'ed on such proposals. :) > I will CC you for qom related topic. :) And according to MAINTAINER, I had better CCed maintainer of Device Tree.
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 6985ad8..1f4e5d8 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -794,9 +794,7 @@ static void device_unparent(Object *obj) >> bus = QLIST_FIRST(&dev->child_bus); >> qbus_free(bus); >> } >> - if (dev->realized) { >> - object_property_set_bool(obj, false, "realized", NULL); >> - } >> + >> if (dev->parent_bus) { >> bus_remove_child(dev->parent_bus, dev); >> object_unref(OBJECT(dev->parent_bus)); >> diff --git a/qom/object.c b/qom/object.c >> index 803b94b..2c945f0 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -393,6 +393,7 @@ static void object_finalize(void *data) >> Object *obj = data; >> TypeImpl *ti = obj->class->type; >> >> + object_property_set_bool(obj, false, "realized", NULL); > > This is incorrect since we specifically only have "realized" for > devices, not for all QOM objects. > > If we want to move it to the finalizer you'll need to use > .instance_finalize on the device type in hw/core/qdev.c. > However the derived type's finalizer is run before its parent's, which Do you mean the sequence in object_deinit()? > may lead to realized = false accessing freed memory. If my understanding as above is correct, we just need to guarantee realized=false (e.g. pci_e1000_uninit )for derived type will only free the resource at its layer, and not touch its parent's, then it can not access freed memory, right? Regards, Pingfan > > Regards, > Andreas > >> object_deinit(obj, ti); >> object_property_del_all(obj); >> > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg