From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59078) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YM6x7-0005wj-S4 for qemu-devel@nongnu.org; Thu, 12 Feb 2015 22:30:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YM6x2-0006Mj-SO for qemu-devel@nongnu.org; Thu, 12 Feb 2015 22:30:01 -0500 Received: from e32.co.us.ibm.com ([32.97.110.150]:38287) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YM6x2-0006Mf-Ll for qemu-devel@nongnu.org; Thu, 12 Feb 2015 22:29:56 -0500 Received: from /spool/local by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 12 Feb 2015 20:29:55 -0700 Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 9CC6C1FF0040 for ; Thu, 12 Feb 2015 20:21:04 -0700 (MST) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t1D3TqkB46137458 for ; Thu, 12 Feb 2015 20:29:52 -0700 Received: from d03av05.boulder.ibm.com (localhost [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t1D3Tqtd002774 for ; Thu, 12 Feb 2015 20:29:52 -0700 Message-ID: <54DD6FAE.2070500@linux.vnet.ibm.com> Date: Thu, 12 Feb 2015 22:29:50 -0500 From: Matthew Rosato MIME-Version: 1.0 References: <1423758091-26462-1-git-send-email-mjrosato@linux.vnet.ibm.com> <54DCE440.6000609@redhat.com> <54DCFFB3.1070908@linux.vnet.ibm.com> <54DD1085.4030503@redhat.com> In-Reply-To: <54DD1085.4030503@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] memory: Fix double unref of flatview List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Christian Borntraeger , Fam Zheng , qemu-devel On 02/12/2015 03:43 PM, Paolo Bonzini wrote: > > > On 12/02/2015 20:32, Matthew Rosato wrote: >> Could it be that the order in which flatview_unref (and therefore >> memory_region_unref) vs object_unparent(mr) matters (ie, object_unparent >> should always happen last)? Prior to RCUification, seems like the >> old_view was always unreferenced prior to return from >> memory_region_del_subregion (so, memory_region_unref always occurred >> before the object_unparent(mr)). Now, the two could happen in either >> order -- and looking at memory_region_unref, it is specifically looking >> at the existence of obj->parent. > > I think you're right. FYI, then this probably also affects the places you hit in d8d9581460 "memory: convert memory_region_destroy to object_unparent", as that's what I modeled this approach on -- but I haven't tested any of them. > > To test your theory, you could try using call_rcu to unparent the memory > region. The complete fix would be to wrap the MemoryRegion within a Yep, this test worked. > container device, and do object_unparent on the container device. Then > the following happens: > > 1) the container object's unparent method calls memory_region_del_subregion > > 2) when the old flatview is destroyed, it calls memory_region_unref but > the parent is still in place > > 3) the container object is destroyed > > 4) the container object destroys the container device. OK great! I noticed this problem while re-structuring this area of code anyway, so I'll roll this in. > > Note that these rules apply only to actual memory regions (I/O or RAM), > not aliases or containers. I hope to post a doc patch next week, since > I'm completing the patches that fix the only other problematic case > (PCIe MMCONFIG). I was not expecting this to trigger so early, which is > why I had left this for part 3. Looking forward to it -- Thanks for the help Paolo. Matt > > Paolo >