From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2/2] x86: make hypercall preemption checks consistent Date: Tue, 4 Mar 2014 11:35:46 +0000 Message-ID: <5315BA92.8080902@citrix.com> References: <5315C5370200007800120CE4@nat28.tlf.novell.com> <5315C65E0200007800120CF7@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4348964631047093024==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WKndX-0000iD-Hb for xen-devel@lists.xenproject.org; Tue, 04 Mar 2014 11:35:51 +0000 In-Reply-To: <5315C65E0200007800120CF7@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser , Tim Deegan List-Id: xen-devel@lists.xenproject.org --===============4348964631047093024== Content-Type: multipart/alternative; boundary="------------000904080202050307040005" --------------000904080202050307040005 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 04/03/14 11:26, Jan Beulich wrote: > - never preempt on the first iteration (ensure forward progress) > - never preempt on the last iteration (pointless/wasteful) > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -2934,7 +2934,7 @@ long do_mmuext_op( > > for ( i = 0; i < count; i++ ) > { > - if ( curr->arch.old_guest_table || hypercall_preempt_check() ) > + if ( curr->arch.old_guest_table || (i && hypercall_preempt_check()) ) > { > rc = -EAGAIN; > break; > @@ -3481,7 +3481,7 @@ long do_mmu_update( > > for ( i = 0; i < count; i++ ) > { > - if ( curr->arch.old_guest_table || hypercall_preempt_check() ) > + if ( curr->arch.old_guest_table || (i && hypercall_preempt_check()) ) > { > rc = -EAGAIN; > break; > --- a/xen/arch/x86/mm/hap/hap.c > +++ b/xen/arch/x86/mm/hap/hap.c > @@ -326,7 +326,7 @@ hap_set_allocation(struct domain *d, uns > else > pages -= d->arch.paging.hap.p2m_pages; > > - while ( d->arch.paging.hap.total_pages != pages ) > + for ( ; ; ) > { > if ( d->arch.paging.hap.total_pages < pages ) > { > @@ -355,6 +355,8 @@ hap_set_allocation(struct domain *d, uns > d->arch.paging.hap.total_pages--; > free_domheap_page(pg); > } > + else > + break; > > /* Check to see if we need to yield and try again */ > if ( preempted && hypercall_preempt_check() ) > --- a/xen/arch/x86/mm/p2m-pod.c > +++ b/xen/arch/x86/mm/p2m-pod.c > @@ -242,7 +242,8 @@ p2m_pod_set_cache_target(struct p2m_doma > > p2m_pod_cache_add(p2m, page, order); > > - if ( hypercall_preempt_check() && preemptible ) > + if ( preemptible && pod_target != p2m->pod.count && > + hypercall_preempt_check() ) > { > ret = -EAGAIN; > goto out; > @@ -286,7 +287,8 @@ p2m_pod_set_cache_target(struct p2m_doma > > put_page(page+i); > > - if ( hypercall_preempt_check() && preemptible ) > + if ( preemptible && pod_target != p2m->pod.count && > + hypercall_preempt_check() ) > { > ret = -EAGAIN; > goto out; > --- a/xen/arch/x86/mm/shadow/common.c > +++ b/xen/arch/x86/mm/shadow/common.c > @@ -1672,7 +1672,7 @@ static unsigned int sh_set_allocation(st > SHADOW_PRINTK("current %i target %i\n", > d->arch.paging.shadow.total_pages, pages); > > - while ( d->arch.paging.shadow.total_pages != pages ) > + for ( ; ; ) > { > if ( d->arch.paging.shadow.total_pages < pages ) > { > @@ -1707,6 +1707,8 @@ static unsigned int sh_set_allocation(st > d->arch.paging.shadow.total_pages--; > free_domheap_page(sp); > } > + else > + break; > > /* Check to see if we need to yield and try again */ > if ( preempted && hypercall_preempt_check() ) > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -3596,13 +3596,6 @@ long do_set_trap_table(XEN_GUEST_HANDLE_ > > for ( ; ; ) > { > - if ( hypercall_preempt_check() ) > - { > - rc = hypercall_create_continuation( > - __HYPERVISOR_set_trap_table, "h", traps); > - break; > - } > - > if ( copy_from_guest(&cur, traps, 1) ) > { > rc = -EFAULT; > @@ -3623,6 +3616,13 @@ long do_set_trap_table(XEN_GUEST_HANDLE_ > init_int80_direct_trap(curr); > > guest_handle_add_offset(traps, 1); > + > + if ( hypercall_preempt_check() ) > + { > + rc = hypercall_create_continuation( > + __HYPERVISOR_set_trap_table, "h", traps); > + break; > + } > } > > return rc; > --- a/xen/arch/x86/x86_64/compat/traps.c > +++ b/xen/arch/x86/x86_64/compat/traps.c > @@ -329,13 +329,6 @@ int compat_set_trap_table(XEN_GUEST_HAND > > for ( ; ; ) > { > - if ( hypercall_preempt_check() ) > - { > - rc = hypercall_create_continuation( > - __HYPERVISOR_set_trap_table, "h", traps); > - break; > - } > - > if ( copy_from_guest(&cur, traps, 1) ) > { > rc = -EFAULT; > @@ -353,6 +346,13 @@ int compat_set_trap_table(XEN_GUEST_HAND > init_int80_direct_trap(current); > > guest_handle_add_offset(traps, 1); > + > + if ( hypercall_preempt_check() ) > + { > + rc = hypercall_create_continuation( > + __HYPERVISOR_set_trap_table, "h", traps); > + break; > + } > } > > return rc; > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------000904080202050307040005 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 04/03/14 11:26, Jan Beulich wrote:
- never preempt on the first iteration (ensure forward progress)
- never preempt on the last iteration (pointless/wasteful)

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2934,7 +2934,7 @@ long do_mmuext_op(
 
     for ( i = 0; i < count; i++ )
     {
-        if ( curr->arch.old_guest_table || hypercall_preempt_check() )
+        if ( curr->arch.old_guest_table || (i && hypercall_preempt_check()) )
         {
             rc = -EAGAIN;
             break;
@@ -3481,7 +3481,7 @@ long do_mmu_update(
 
     for ( i = 0; i < count; i++ )
     {
-        if ( curr->arch.old_guest_table || hypercall_preempt_check() )
+        if ( curr->arch.old_guest_table || (i && hypercall_preempt_check()) )
         {
             rc = -EAGAIN;
             break;
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -326,7 +326,7 @@ hap_set_allocation(struct domain *d, uns
     else
         pages -= d->arch.paging.hap.p2m_pages;
 
-    while ( d->arch.paging.hap.total_pages != pages )
+    for ( ; ; )
     {
         if ( d->arch.paging.hap.total_pages < pages )
         {
@@ -355,6 +355,8 @@ hap_set_allocation(struct domain *d, uns
             d->arch.paging.hap.total_pages--;
             free_domheap_page(pg);
         }
+        else
+            break;
 
         /* Check to see if we need to yield and try again */
         if ( preempted && hypercall_preempt_check() )
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -242,7 +242,8 @@ p2m_pod_set_cache_target(struct p2m_doma
 
         p2m_pod_cache_add(p2m, page, order);
 
-        if ( hypercall_preempt_check() && preemptible )
+        if ( preemptible && pod_target != p2m->pod.count &&
+             hypercall_preempt_check() )
         {
             ret = -EAGAIN;
             goto out;
@@ -286,7 +287,8 @@ p2m_pod_set_cache_target(struct p2m_doma
 
             put_page(page+i);
 
-            if ( hypercall_preempt_check() && preemptible )
+            if ( preemptible && pod_target != p2m->pod.count &&
+                 hypercall_preempt_check() )
             {
                 ret = -EAGAIN;
                 goto out;
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1672,7 +1672,7 @@ static unsigned int sh_set_allocation(st
     SHADOW_PRINTK("current %i target %i\n", 
                    d->arch.paging.shadow.total_pages, pages);
 
-    while ( d->arch.paging.shadow.total_pages != pages ) 
+    for ( ; ; )
     {
         if ( d->arch.paging.shadow.total_pages < pages ) 
         {
@@ -1707,6 +1707,8 @@ static unsigned int sh_set_allocation(st
             d->arch.paging.shadow.total_pages--;
             free_domheap_page(sp);
         }
+        else
+            break;
 
         /* Check to see if we need to yield and try again */
         if ( preempted && hypercall_preempt_check() )
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3596,13 +3596,6 @@ long do_set_trap_table(XEN_GUEST_HANDLE_
 
     for ( ; ; )
     {
-        if ( hypercall_preempt_check() )
-        {
-            rc = hypercall_create_continuation(
-                __HYPERVISOR_set_trap_table, "h", traps);
-            break;
-        }
-
         if ( copy_from_guest(&cur, traps, 1) )
         {
             rc = -EFAULT;
@@ -3623,6 +3616,13 @@ long do_set_trap_table(XEN_GUEST_HANDLE_
             init_int80_direct_trap(curr);
 
         guest_handle_add_offset(traps, 1);
+
+        if ( hypercall_preempt_check() )
+        {
+            rc = hypercall_create_continuation(
+                __HYPERVISOR_set_trap_table, "h", traps);
+            break;
+        }
     }
 
     return rc;
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -329,13 +329,6 @@ int compat_set_trap_table(XEN_GUEST_HAND
 
     for ( ; ; )
     {
-        if ( hypercall_preempt_check() )
-        {
-            rc = hypercall_create_continuation(
-                __HYPERVISOR_set_trap_table, "h", traps);
-            break;
-        }
-
         if ( copy_from_guest(&cur, traps, 1) )
         {
             rc = -EFAULT;
@@ -353,6 +346,13 @@ int compat_set_trap_table(XEN_GUEST_HAND
             init_int80_direct_trap(current);
 
         guest_handle_add_offset(traps, 1);
+
+        if ( hypercall_preempt_check() )
+        {
+            rc = hypercall_create_continuation(
+                __HYPERVISOR_set_trap_table, "h", traps);
+            break;
+        }
     }
 
     return rc;





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------000904080202050307040005-- --===============4348964631047093024== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4348964631047093024==--