WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH][UPDATE]Remove lock on guest table walk

To: "Tim Deegan" <Tim.Deegan@xxxxxxxxxx>
Subject: [Xen-devel] [PATCH][UPDATE]Remove lock on guest table walk
From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
Date: Fri, 22 Feb 2008 10:33:19 +0800
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 22 Feb 2008 08:08:45 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20080221141318.GB21677@xxxxxxxxxxxxxxxxxxxxx>
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> <20080221141318.GB21677@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Ach0lF3aVzHh+xvyTk+ReM+BB2J7jAAZJjUg
Thread-topic: [PATCH][UPDATE]Remove lock on guest table walk
>From: Tim Deegan
>Sent: 2008年2月21日 22:13
>Hi,
>
>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.

Thanks for clarification.

>
>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.

Yes, my overlook and thanks for pointing out.

>
>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.

Done and renamed to gtable_dirty_version. Sorry if this is still not a
good name and please feel free help polish a better name. :-)

> - 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()?

Agree.

> - You've added a second "not a shadow_fault" printout without removing
>   the first.

The first one is branched with lock held, and some audit functions also
required lock held. To differentiate, I put a new label. Now I just removed
the new label since only few places use it.

> - We can remove the comment at the top of multi.c about reworking the
>   guest_walk TLB flush logic. :)

Done.

Then updated version attached, and please help check again.

Thanks,
Kevin

Attachment: gwalk_no_lock.patch
Description: gwalk_no_lock.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>