* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing [not found] ` <512B46C3.4030805@de.ibm.com> @ 2013-03-08 20:11 ` Alexander Graf 2013-03-11 12:04 ` Cornelia Huck 0 siblings, 1 reply; 5+ messages in thread From: Alexander Graf @ 2013-03-08 20:11 UTC (permalink / raw) To: Christian Borntraeger Cc: Cornelia Huck, Paolo Bonzini, Jens Freimann, qemu-devel, Andreas Färber On 25.02.2013, at 12:10, Christian Borntraeger wrote: > On 25/02/13 11:44, Paolo Bonzini wrote: >> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: >>> Hmm, the old sequence was >>> >>> object_unparent(OBJECT(dev)); >>> qdev_free(dev) ---+ >>> | >>> V >>> ... >>> object_unparent(OBJECT(dev)); now the last reference is gone, object is freed >>> object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 >>> ... >>> >>> Isnt that a problem in itself that we modify a reference counter in an deleted object? >> >> The second object_unparent should do nothing. So before you had: >> >> object_unparent(OBJECT(dev)); leaves refcount=1 >> qdev_free(dev) ---+ >> | >> V >> object_unparent(OBJECT(dev)); do nothing >> object_unref(OBJECT(dev)); refcount=0, object freed >> >> After the object_unref was removed you had: >> >> object_unparent(OBJECT(dev)); refcount=0, object freed >> qdev_free(dev) ---+ >> | >> V >> object_unparent(OBJECT(dev)); dangling pointer! >> > > > Got it. Thanks So is the patch valid? Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing 2013-03-08 20:11 ` [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing Alexander Graf @ 2013-03-11 12:04 ` Cornelia Huck 2013-03-11 12:16 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Cornelia Huck @ 2013-03-11 12:04 UTC (permalink / raw) To: Alexander Graf Cc: Christian Borntraeger, Jens Freimann, qemu-devel, Andreas Färber, Paolo Bonzini On Fri, 8 Mar 2013 21:11:13 +0100 Alexander Graf <agraf@suse.de> wrote: > > On 25.02.2013, at 12:10, Christian Borntraeger wrote: > > > On 25/02/13 11:44, Paolo Bonzini wrote: > >> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: > >>> Hmm, the old sequence was > >>> > >>> object_unparent(OBJECT(dev)); > >>> qdev_free(dev) ---+ > >>> | > >>> V > >>> ... > >>> object_unparent(OBJECT(dev)); now the last reference is gone, object is freed > >>> object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 > >>> ... > >>> > >>> Isnt that a problem in itself that we modify a reference counter in an deleted object? > >> > >> The second object_unparent should do nothing. So before you had: > >> > >> object_unparent(OBJECT(dev)); leaves refcount=1 > >> qdev_free(dev) ---+ > >> | > >> V > >> object_unparent(OBJECT(dev)); do nothing > >> object_unref(OBJECT(dev)); refcount=0, object freed > >> > >> After the object_unref was removed you had: > >> > >> object_unparent(OBJECT(dev)); refcount=0, object freed > >> qdev_free(dev) ---+ > >> | > >> V > >> object_unparent(OBJECT(dev)); dangling pointer! > >> > > > > > > Got it. Thanks > > So is the patch valid? To my understanding, yes. > > > Alex > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing 2013-03-11 12:04 ` Cornelia Huck @ 2013-03-11 12:16 ` Paolo Bonzini 2013-03-11 12:22 ` Alexander Graf 0 siblings, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2013-03-11 12:16 UTC (permalink / raw) To: Cornelia Huck Cc: Christian Borntraeger, Jens Freimann, Alexander Graf, Andreas Färber, qemu-devel Il 11/03/2013 13:04, Cornelia Huck ha scritto: > On Fri, 8 Mar 2013 21:11:13 +0100 > Alexander Graf <agraf@suse.de> wrote: > >> >> On 25.02.2013, at 12:10, Christian Borntraeger wrote: >> >>> On 25/02/13 11:44, Paolo Bonzini wrote: >>>> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: >>>>> Hmm, the old sequence was >>>>> >>>>> object_unparent(OBJECT(dev)); >>>>> qdev_free(dev) ---+ >>>>> | >>>>> V >>>>> ... >>>>> object_unparent(OBJECT(dev)); now the last reference is gone, object is freed >>>>> object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 >>>>> ... >>>>> >>>>> Isnt that a problem in itself that we modify a reference counter in an deleted object? >>>> >>>> The second object_unparent should do nothing. So before you had: >>>> >>>> object_unparent(OBJECT(dev)); leaves refcount=1 >>>> qdev_free(dev) ---+ >>>> | >>>> V >>>> object_unparent(OBJECT(dev)); do nothing >>>> object_unref(OBJECT(dev)); refcount=0, object freed >>>> >>>> After the object_unref was removed you had: >>>> >>>> object_unparent(OBJECT(dev)); refcount=0, object freed >>>> qdev_free(dev) ---+ >>>> | >>>> V >>>> object_unparent(OBJECT(dev)); dangling pointer! >>>> >>> >>> >>> Got it. Thanks >> >> So is the patch valid? > > To my understanding, yes. Yes, except that the "fixed a crash" part in the commit message is probably no longer accurate. No big deal. :) Paolo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing 2013-03-11 12:16 ` Paolo Bonzini @ 2013-03-11 12:22 ` Alexander Graf 2013-03-11 12:32 ` Cornelia Huck 0 siblings, 1 reply; 5+ messages in thread From: Alexander Graf @ 2013-03-11 12:22 UTC (permalink / raw) To: Paolo Bonzini Cc: Cornelia Huck, Christian Borntraeger, Jens Freimann, qemu-devel, Andreas Färber On 03/11/2013 01:16 PM, Paolo Bonzini wrote: > Il 11/03/2013 13:04, Cornelia Huck ha scritto: >> On Fri, 8 Mar 2013 21:11:13 +0100 >> Alexander Graf<agraf@suse.de> wrote: >> >>> On 25.02.2013, at 12:10, Christian Borntraeger wrote: >>> >>>> On 25/02/13 11:44, Paolo Bonzini wrote: >>>>> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: >>>>>> Hmm, the old sequence was >>>>>> >>>>>> object_unparent(OBJECT(dev)); >>>>>> qdev_free(dev) ---+ >>>>>> | >>>>>> V >>>>>> ... >>>>>> object_unparent(OBJECT(dev)); now the last reference is gone, object is freed >>>>>> object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 >>>>>> ... >>>>>> >>>>>> Isnt that a problem in itself that we modify a reference counter in an deleted object? >>>>> The second object_unparent should do nothing. So before you had: >>>>> >>>>> object_unparent(OBJECT(dev)); leaves refcount=1 >>>>> qdev_free(dev) ---+ >>>>> | >>>>> V >>>>> object_unparent(OBJECT(dev)); do nothing >>>>> object_unref(OBJECT(dev)); refcount=0, object freed >>>>> >>>>> After the object_unref was removed you had: >>>>> >>>>> object_unparent(OBJECT(dev)); refcount=0, object freed >>>>> qdev_free(dev) ---+ >>>>> | >>>>> V >>>>> object_unparent(OBJECT(dev)); dangling pointer! >>>>> >>>> >>>> Got it. Thanks >>> So is the patch valid? >> To my understanding, yes. > Yes, except that the "fixed a crash" part in the commit message is > probably no longer accurate. No big deal. :) Ok, Connie could you please include it in your next pull then please? Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing 2013-03-11 12:22 ` Alexander Graf @ 2013-03-11 12:32 ` Cornelia Huck 0 siblings, 0 replies; 5+ messages in thread From: Cornelia Huck @ 2013-03-11 12:32 UTC (permalink / raw) To: Alexander Graf Cc: Paolo Bonzini, Jens Freimann, qemu-devel, Andreas Färber, Christian Borntraeger On Mon, 11 Mar 2013 13:22:45 +0100 Alexander Graf <agraf@suse.de> wrote: > On 03/11/2013 01:16 PM, Paolo Bonzini wrote: > > Il 11/03/2013 13:04, Cornelia Huck ha scritto: > >> On Fri, 8 Mar 2013 21:11:13 +0100 > >> Alexander Graf<agraf@suse.de> wrote: > >> > >>> On 25.02.2013, at 12:10, Christian Borntraeger wrote: > >>> > >>>> On 25/02/13 11:44, Paolo Bonzini wrote: > >>>>> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: > >>>>>> Hmm, the old sequence was > >>>>>> > >>>>>> object_unparent(OBJECT(dev)); > >>>>>> qdev_free(dev) ---+ > >>>>>> | > >>>>>> V > >>>>>> ... > >>>>>> object_unparent(OBJECT(dev)); now the last reference is gone, object is freed > >>>>>> object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 > >>>>>> ... > >>>>>> > >>>>>> Isnt that a problem in itself that we modify a reference counter in an deleted object? > >>>>> The second object_unparent should do nothing. So before you had: > >>>>> > >>>>> object_unparent(OBJECT(dev)); leaves refcount=1 > >>>>> qdev_free(dev) ---+ > >>>>> | > >>>>> V > >>>>> object_unparent(OBJECT(dev)); do nothing > >>>>> object_unref(OBJECT(dev)); refcount=0, object freed > >>>>> > >>>>> After the object_unref was removed you had: > >>>>> > >>>>> object_unparent(OBJECT(dev)); refcount=0, object freed > >>>>> qdev_free(dev) ---+ > >>>>> | > >>>>> V > >>>>> object_unparent(OBJECT(dev)); dangling pointer! > >>>>> > >>>> > >>>> Got it. Thanks > >>> So is the patch valid? > >> To my understanding, yes. > > Yes, except that the "fixed a crash" part in the commit message is > > probably no longer accurate. No big deal. :) > > Ok, Connie could you please include it in your next pull then please? Sure, will do. > > > Alex > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-11 12:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1489845274.730285.1361778940139.JavaMail.root@redhat.com>
[not found] ` <512B1C35.3040500@de.ibm.com>
[not found] ` <512B4073.6050100@redhat.com>
[not found] ` <512B46C3.4030805@de.ibm.com>
2013-03-08 20:11 ` [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing Alexander Graf
2013-03-11 12:04 ` Cornelia Huck
2013-03-11 12:16 ` Paolo Bonzini
2013-03-11 12:22 ` Alexander Graf
2013-03-11 12:32 ` Cornelia Huck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).