From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UF1uI-0004h9-5T for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:32:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UF1uG-0004D1-4N for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:32:46 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:50522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UF1uF-0004Cf-Rm for qemu-devel@nongnu.org; Mon, 11 Mar 2013 08:32:44 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 11 Mar 2013 12:31:31 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id B930D1B08061 for ; Mon, 11 Mar 2013 12:32:37 +0000 (GMT) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2BCWSjO29360276 for ; Mon, 11 Mar 2013 12:32:28 GMT Received: from d06av05.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r2BCWawu006300 for ; Mon, 11 Mar 2013 06:32:37 -0600 Date: Mon, 11 Mar 2013 13:32:34 +0100 From: Cornelia Huck Message-ID: <20130311133234.05d99f15@gondolin> In-Reply-To: <513DCC95.6070407@suse.de> 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> <513DCC95.6070407@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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: Alexander Graf Cc: Paolo Bonzini , Jens Freimann , qemu-devel , Andreas =?UTF-8?B?RsOkcmJlcg==?= , Christian Borntraeger On Mon, 11 Mar 2013 13:22:45 +0100 Alexander Graf 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 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 >