From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zow-0003XF-Qw for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0zos-0004qP-Nm for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15703) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zos-0004q5-G8 for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:54 -0400 Date: Mon, 13 Aug 2012 15:53:43 -0300 From: Marcelo Tosatti Message-ID: <20120813185343.GE25268@amt.cnet> References: <1344407156-25562-1-git-send-email-qemulist@gmail.com> <1344407156-25562-14-git-send-email-qemulist@gmail.com> <502236D8.3040902@redhat.com> <50236E10.8030709@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: kvm@vger.kernel.org, Jan Kiszka , qemu-devel@nongnu.org, Blue Swirl , Avi Kivity , Anthony Liguori , Stefan Hajnoczi , Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= On Fri, Aug 10, 2012 at 02:42:58PM +0800, liu ping fan wrote: > On Thu, Aug 9, 2012 at 4:00 PM, Paolo Bonzini wrote: > > Il 09/08/2012 09:28, liu ping fan ha scritto: > >>> > VCPU thread I/O thread > >>> > ===================================================================== > >>> > get MMIO request > >>> > rcu_read_lock() > >>> > walk memory map > >>> > qdev_unmap() > >>> > lock_devtree() > >>> > ... > >>> > unlock_devtree > >>> > unref dev -> refcnt=0, free enqueued > >>> > ref() > >> No ref() for dev here, while we have ref to flatview+radix in my patches. > >> I use rcu to protect radix+flatview+mr refered. As to dev, its ref has > >> inc when it is added into mem view -- that is > >> memory_region_add_subregion -> memory_region_get() { > >> if(atomic_add_and_return()) dev->ref++ }. > >> So not until reclaimer of mem view, the dev's ref is hold by mem view. > >> In a short word, rcu protect mem view, while device is protected by refcnt. > > > > But the RCU critical section should not include the whole processing of > > MMIO, only the walk of the memory map. > > > Yes, you are right. And I think cur_map_get() can be broken into the > style "lock, ref++, phys_page_find(); unlock". easily. > > > And in general I think this is a bit too tricky... I understand not > > adding refcounting to all of bottom halves, timers, etc., but if you are > > using a device you should have explicit ref/unref pairs. > > > Actually, there are pairs -- when dev enter mem view, inc ref; and > when it leave, dec ref. > But as Avi has pointed out, the mr->refcnt introduce complicate and no > gain. So I will discard this design The plan that you refer that has been relatively well thought out, IIRC. Instead of designing something, i would try to understand/improve on that. > Thanks and regards, > pingfan > > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html