From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLxHm-0003mL-4Q for qemu-devel@nongnu.org; Thu, 12 Feb 2015 12:10:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLxHi-0008JG-VT for qemu-devel@nongnu.org; Thu, 12 Feb 2015 12:10:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39305) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLxHi-0008J5-Nt for qemu-devel@nongnu.org; Thu, 12 Feb 2015 12:10:38 -0500 Message-ID: <54DCDE5C.5060509@redhat.com> Date: Thu, 12 Feb 2015 18:09:48 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1423758091-26462-1-git-send-email-mjrosato@linux.vnet.ibm.com> In-Reply-To: <1423758091-26462-1-git-send-email-mjrosato@linux.vnet.ibm.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: Matthew Rosato , qemu-devel@nongnu.org Cc: borntraeger@de.ibm.com, famz@redhat.com On 12/02/2015 17:21, Matthew Rosato wrote: > Since 374f2981d1 "memory: protect current_map by RCU", > address_space_update_topology unrefs the old_flatview twice, > once by call_rcu and once by direct call. This patch removes > the direct call in favor of the call_rcu. Fixes at least one > assertion failure seen in s390, where a ref count for a memory > region attempts to go negative during hot-unplug of guest memory. > > Signed-off-by: Matthew Rosato The two unrefs are correct. One is needed to balance address_space_get_flatview; the other is needed because as->current_map does not point to old_view anymore. You can remove them with something like this (your patch plus one extra hunk): diff --git a/memory.c b/memory.c index a844ced..5add529 100644 --- a/memory.c +++ b/memory.c @@ -747,7 +747,7 @@ static void address_space_update_topology_pass(AddressSpace *as, static void address_space_update_topology(AddressSpace *as) { - FlatView *old_view = address_space_get_flatview(as); + FlatView *old_view = as->current_map; FlatView *new_view = generate_memory_topology(as->root); address_space_update_topology_pass(as, old_view, new_view, false); @@ -755,7 +755,6 @@ static void address_space_update_topology(AddressSpace *as) /* Writes are protected by the BQL. */ atomic_rcu_set(&as->current_map, new_view); - call_rcu(old_view, flatview_unref, rcu); /* Note that all the old MemoryRegions are still alive up to this * point. This relieves most MemoryListeners from the need to @@ -763,7 +762,7 @@ static void address_space_update_topology(AddressSpace *as) * outside the iothread mutex, in which case precise reference * counting is necessary. */ - flatview_unref(old_view); + call_rcu(old_view, flatview_unref, rcu); address_space_update_ioeventfds(as); } but it wouldn't affect your bug. Paolo