Xen 
 
Home About Xen.org Xen Xen Summit Wiki Mailing List Bug Tracker Xen Downloads
 
   
 

xen-devel

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

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] FW: [PATCH][UPDATE]Remove lock on guest table walk
From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
Date: Fri, 22 Feb 2008 17:29:29 +0800
Delivery-date: Fri, 22 Feb 2008 08:26:15 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Ach0lF3aVzHh+xvyTk+ReM+BB2J7jAAZJjUgAA8SW8A=
Thread-topic: [PATCH][UPDATE]Remove lock on guest table walk
Is there any trouble with mailing list? I didn't see my mail sent back
after almost 8hrs. :-(

Thanks,
Kevin 

-----Original Message-----
From: Tian, Kevin 
Sent: 2008年2月22日 10:33
To: 'Tim Deegan'
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [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>