From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 13/15] hotplug: introduce
 qdev_unplug_complete() to remove device from views
Date: Mon, 13 Aug 2012 15:53:43 -0300
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>
	<CAJnKYQ=xWDSAk=+8duE2xo3uGyN2j_rBpYPsTRt=OgVuAMFnbA@mail.gmail.com>
	<50236E10.8030709@redhat.com>
	<CAJnKYQmFkTM76dHF19y5GN7q88k8ZY2quH2_MGMF=MHQ4uE6fw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, Jan Kiszka <jan.kiszka@siemens.com>,
	qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
	Avi Kivity <avi@redhat.com>, Anthony Liguori <anthony@codemonkey.ws>,
	Stefan Hajnoczi <stefanha@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>
To: liu ping fan <qemulist@gmail.com>
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
Content-Disposition: inline
In-Reply-To: <CAJnKYQmFkTM76dHF19y5GN7q88k8ZY2quH2_MGMF=MHQ4uE6fw@mail.gmail.com>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=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 Fri, Aug 10, 2012 at 02:42:58PM +0800, liu ping fan wrote:
> On Thu, Aug 9, 2012 at 4:00 PM, Paolo Bonzini <pbonzini@redhat.com> 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