This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH][RFC]Remove lock on first guest table walk

To: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH][RFC]Remove lock on first guest table walk
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Thu, 21 Feb 2008 14:13:18 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 21 Feb 2008 06:14:58 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <D470B4E54465E3469E2ABBC5AFAC390F024D8FBB@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <D470B4E54465E3469E2ABBC5AFAC390F024D8FBB@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.13 (2006-08-11)

So, the idea seems sound, and avoids the shadow lock altogether on a
bunch more pagefaults, which is nice. 

I think that since PV pagetables are guaranteed to be read-only above
l1e, the guest_map_l1e and guest_get_eff_l1e functions can be allowed to
drop the shadow lock and call guest_walk_tables with shadow_op == 0.
That would mean that there are no callers left setting shadow_op to 1,
and then the shadow_op argument (and the whole mechanism for calling
remove_write_access from guest_walk_tables) could be removed.

I think that in guest_walk_tables, you need to add an rmb() after
reading the version number to stop the compiler from hoisting the
pagetable reads to before the version read.

Coding nits:
 - I'd like to see the "version" field called something more
   descriptive, and moved into the shadow-specific domain state, 
   since HAP won't be using it.
 - In shadow_check_gwalk, maybe return 1 at the top of the function if
   the version number hasn't changed, rather than putting most of the 
   function inside an if()?
 - You've added a second "not a shadow_fault" printout without removing
   the first.
 - We can remove the comment at the top of multi.c about reworking the
   guest_walk TLB flush logic. :)



Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>