From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UF1lJ-0001m9-L2 for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:23:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UF1lI-0001Wh-6x for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:23:29 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49083 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UF1lH-0001Pr-Tf for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:23:28 -0400 Message-ID: <513DCC95.6070407@suse.de> Date: Mon, 11 Mar 2013 13:22:45 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1489845274.730285.1361778940139.JavaMail.root@redhat.com> <512B1C35.3040500@de.ibm.com> <512B4073.6050100@redhat.com> <512B46C3.4030805@de.ibm.com> <20130311130447.620e713d@gondolin> <513DCB2C.5030404@redhat.com> In-Reply-To: <513DCB2C.5030404@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Cornelia Huck , Christian Borntraeger , Jens Freimann , qemu-devel , =?ISO-8859-1?Q?Andreas_F=E4rber?= 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 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