From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [patch 09/10] KVM: MMU: out of sync shadow core v2
Date: Mon, 22 Sep 2008 19:51:37 -0300
Message-ID: <20080922225137.GA1257@dmt.cnet>
References: <20080918212749.800177179@localhost.localdomain> <20080918213337.148804603@localhost.localdomain> <48D4506C.5070804@redhat.com> <20080921004515.GC10120@dmt.cnet> <48D802EA.9070807@redhat.com> <20080922215503.GA27744@dmt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, "David S. Ahern" <daahern@cisco.com>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:43971 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753167AbYIVWw4 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 22 Sep 2008 18:52:56 -0400
Content-Disposition: inline
In-Reply-To: <20080922215503.GA27744@dmt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Sep 22, 2008 at 06:55:03PM -0300, Marcelo Tosatti wrote:
> On Mon, Sep 22, 2008 at 11:41:14PM +0300, Avi Kivity wrote:
> > Marcelo Tosatti wrote:
> >>>> +	while (parent->unsync_children) {
> >>>> +		for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
> >>>> +			u64 ent = sp->spt[i];
> >>>> +
> >>>> +			if (is_shadow_present_pte(ent)) {
> >>>> +				struct kvm_mmu_page *child;
> >>>> +				child = page_header(ent & PT64_BASE_ADDR_MASK);
> >>>>       
> >>> What does this do?
> >>>     
> >>
> >> Walks all children of given page with no efficiency. Its replaced later
> >> by the bitmap version.
> >>
> >>   
> >
> > I don't understand how the variables sp, child, and parent interact. You  
> > either need recursion or an explicit stack?
> 
> It restarts at parent level whenever finishing any children:
> 
> +               if (i == PT64_ENT_PER_PAGE) {
> +                       sp->unsync_children = 0;
> +                       sp = parent;
> +               }
> 
> No efficiency.

Do you prefer a recursive version for this one too?