From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuNsh-00006H-UZ for qemu-devel@nongnu.org; Thu, 26 Jul 2012 09:13:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SuNsY-0004bg-CO for qemu-devel@nongnu.org; Thu, 26 Jul 2012 09:13:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47590) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuNsY-0004bZ-3W for qemu-devel@nongnu.org; Thu, 26 Jul 2012 09:13:22 -0400 Message-ID: <50114269.2000906@redhat.com> Date: Thu, 26 Jul 2012 16:13:13 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1343187070-27371-1-git-send-email-qemulist@gmail.com> <1343187070-27371-3-git-send-email-qemulist@gmail.com> <500FD13B.7040108@redhat.com> <500FE623.8060304@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/5] exec.c: use refcnt to protect device during dispatching List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: kvm@vger.kernel.org, Jan Kiszka , Marcelo Tosatti , qemu-devel@nongnu.org, Anthony Liguori , Stefan Hajnoczi On 07/26/2012 04:06 PM, liu ping fan wrote: > On Wed, Jul 25, 2012 at 8:27 PM, Avi Kivity wrote: >> On 07/25/2012 01:58 PM, Avi Kivity wrote: >>>> while (len > 0) { >>>> page = addr & TARGET_PAGE_MASK; >>>> l = (page + TARGET_PAGE_SIZE) - addr; >>>> if (l > len) >>>> l = len; >>>> + >>>> + qemu_rwlock_rdlock_devtree(); >>>> section = phys_page_find(page >> TARGET_PAGE_BITS); >>> >>> Does the devtree lock also protect the data structures accessed by >>> phys_page_find()? Seems wrong. >> >> The right way is to object_ref() in core_region_add() and object_unref() >> in core_region_del(). We're guaranteed that mr->object is alive during >> _add(), and DeviceClass::unmap() ensures that the extra ref doesn't >> block destruction. >> > OK, I see. I will try in this way. But when > memory_region_destroy()->..->core_region_del(), should we reset the > lp.ptr to phys_section_unassigned , otherwise, if using removed > target_phys_addr_t, we will still get the pointer to invalid > MemoryRegion? The intent was to use rcu, so when we rebuild phys_map we build it into a new tree, use rcu_assign_pointer() to switch into the new tree, then synchronize_rcu() and drop the old tree. Since we don't have rcu yet we can emulate it with a lock. We can start with a simple mutex around the lookup and rebuild, then switch to rwlock or rcu if needed. (without the lock or rcu, just changing lp.ptr is dangerous, since it is a bit field) -- error compiling committee.c: too many arguments to function