From mboxrd@z Thu Jan 1 00:00:00 1970 From: liu ping fan Subject: Re: [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views Date: Fri, 10 Aug 2012 14:42:58 +0800 Message-ID: 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=ISO-8859-1 Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, Anthony Liguori , Avi Kivity , Jan Kiszka , Marcelo Tosatti , Stefan Hajnoczi , Blue Swirl , =?ISO-8859-1?Q?Andreas_F=E4rber?= To: Paolo Bonzini Return-path: Received: from mail-wi0-f178.google.com ([209.85.212.178]:33654 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751447Ab2HJGnT (ORCPT ); Fri, 10 Aug 2012 02:43:19 -0400 Received: by wibhr14 with SMTP id hr14so1057161wib.1 for ; Thu, 09 Aug 2012 23:43:18 -0700 (PDT) In-Reply-To: <50236E10.8030709@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 Thanks and regards, pingfan > Paolo From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SziwM-0002y8-Kg for qemu-devel@nongnu.org; Fri, 10 Aug 2012 02:43:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SziwK-0005rB-0t for qemu-devel@nongnu.org; Fri, 10 Aug 2012 02:43:22 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:46159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SziwJ-0005qA-Pf for qemu-devel@nongnu.org; Fri, 10 Aug 2012 02:43:19 -0400 Received: by wibhm2 with SMTP id hm2so787225wib.10 for ; Thu, 09 Aug 2012 23:43:18 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <50236E10.8030709@redhat.com> 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> From: liu ping fan Date: Fri, 10 Aug 2012 14:42:58 +0800 Message-ID: Content-Type: text/plain; charset=ISO-8859-1 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 , Marcelo Tosatti , qemu-devel@nongnu.org, Blue Swirl , Avi Kivity , Anthony Liguori , Stefan Hajnoczi , =?ISO-8859-1?Q?Andreas_F=E4rber?= 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 Thanks and regards, pingfan > Paolo