From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views Date: Mon, 13 Aug 2012 15:51:58 -0300 Message-ID: <20120813185158.GD25268@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 Cc: kvm@vger.kernel.org, Jan Kiszka , liu ping fan , qemu-devel@nongnu.org, Blue Swirl , Avi Kivity , Anthony Liguori , Stefan Hajnoczi , Andreas =?iso-8859-1?Q?F=E4rber?= To: Paolo Bonzini Return-path: Content-Disposition: inline In-Reply-To: <50236E10.8030709@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org On Thu, Aug 09, 2012 at 10:00:16AM +0200, 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. The idea, written on that plan, was: - RCU protects memory maps. - Object reference protects device in the window between 4. and 5. The unplug/remove path should: 1) Lock memmap_lock for write (if not using RCU). 2) Remove any memmap entries (which is possible due to write lock on memmap_lock. Alternatively wait for an RCU grace period). Device should not be visible after that. 3) Lock dev->lock. 4) Wait until references are removed (no new references can be made since device is not visible). 5) Remove device. So its a combination of both dev->lock and reference counter. Note: a first step can be only parallel execution of MMIO lookups (actually that is a very good first target). dev->lock above would be qemu_big_lock in that first stage, then _only devices which are performance sensitive need to be converted_. > But the RCU critical section should not include the whole processing of > MMIO, only the walk of the memory map. Yes. > 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. > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33478) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zov-0003X4-0B for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0zos-0004qJ-Js for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14188) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zos-0004q3-Bw for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:56:54 -0400 Date: Mon, 13 Aug 2012 15:51:58 -0300 From: Marcelo Tosatti Message-ID: <20120813185158.GD25268@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: <50236E10.8030709@redhat.com> 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: Paolo Bonzini Cc: kvm@vger.kernel.org, Jan Kiszka , liu ping fan , qemu-devel@nongnu.org, Blue Swirl , Avi Kivity , Anthony Liguori , Stefan Hajnoczi , Andreas =?iso-8859-1?Q?F=E4rber?= On Thu, Aug 09, 2012 at 10:00:16AM +0200, 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. The idea, written on that plan, was: - RCU protects memory maps. - Object reference protects device in the window between 4. and 5. The unplug/remove path should: 1) Lock memmap_lock for write (if not using RCU). 2) Remove any memmap entries (which is possible due to write lock on memmap_lock. Alternatively wait for an RCU grace period). Device should not be visible after that. 3) Lock dev->lock. 4) Wait until references are removed (no new references can be made since device is not visible). 5) Remove device. So its a combination of both dev->lock and reference counter. Note: a first step can be only parallel execution of MMIO lookups (actually that is a very good first target). dev->lock above would be qemu_big_lock in that first stage, then _only devices which are performance sensitive need to be converted_. > But the RCU critical section should not include the whole processing of > MMIO, only the walk of the memory map. Yes. > 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. > > 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