xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] make hypercall preemption checks consistent
@ 2014-03-04 11:21 Jan Beulich
  2014-03-04 11:24 ` [PATCH 1/2] common: " Jan Beulich
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 11:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Keir Fraser

- never preempt on the first iteration (ensure forward progress)
- never preempt on the last iteration (pointless/wasteful)
- do cheap checks first

1: common: make hypercall preemption checks consistent
2: x86: make hypercall preemption checks consistent

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] common: make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
@ 2014-03-04 11:24 ` Jan Beulich
  2014-03-04 11:29   ` Andrew Cooper
  2014-03-04 11:26 ` [PATCH 2/2] x86: " Jan Beulich
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 11:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 2465 bytes --]

- never preempt on the first iteration (ensure forward progress)
- do cheap checks first

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

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -63,7 +63,7 @@ static void increase_reservation(struct 
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -109,7 +109,7 @@ static void populate_physmap(struct memo
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -268,7 +268,7 @@ static void decrease_reservation(struct 
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() && i != a->nr_done )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -398,7 +398,8 @@ static long memory_exchange(XEN_GUEST_HA
           i < (exch.in.nr_extents >> in_chunk_order);
           i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != (exch.nr_exchanged >> in_chunk_order) &&
+             hypercall_preempt_check() )
         {
             exch.nr_exchanged = i << in_chunk_order;
             rcu_unlock_domain(d);
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -52,7 +52,7 @@ do_multicall(
 
     for ( i = 0; !rc && i < nr_calls; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i && hypercall_preempt_check() )
             goto preempted;
 
         if ( unlikely(__copy_from_guest(&mcs->call, call_list, 1)) )
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -375,12 +375,12 @@ static DECLARE_SOFTIRQ_TASKLET(notify_do
 static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
 {
     char kbuf[128];
-    int kcount;
+    int kcount = 0;
     struct domain *cd = current->domain;
 
     while ( count > 0 )
     {
-        if ( hypercall_preempt_check() )
+        if ( kcount && hypercall_preempt_check() )
             return hypercall_create_continuation(
                 __HYPERVISOR_console_io, "iih",
                 CONSOLEIO_write, count, buffer);




[-- Attachment #2: preempt-progress-common.patch --]
[-- Type: text/plain, Size: 2514 bytes --]

common: make hypercall preemption checks consistent

- never preempt on the first iteration (ensure forward progress)
- do cheap checks first

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

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -63,7 +63,7 @@ static void increase_reservation(struct 
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -109,7 +109,7 @@ static void populate_physmap(struct memo
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -268,7 +268,7 @@ static void decrease_reservation(struct 
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
-        if ( hypercall_preempt_check() && i != a->nr_done )
+        if ( i != a->nr_done && hypercall_preempt_check() )
         {
             a->preempted = 1;
             goto out;
@@ -398,7 +398,8 @@ static long memory_exchange(XEN_GUEST_HA
           i < (exch.in.nr_extents >> in_chunk_order);
           i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i != (exch.nr_exchanged >> in_chunk_order) &&
+             hypercall_preempt_check() )
         {
             exch.nr_exchanged = i << in_chunk_order;
             rcu_unlock_domain(d);
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -52,7 +52,7 @@ do_multicall(
 
     for ( i = 0; !rc && i < nr_calls; i++ )
     {
-        if ( hypercall_preempt_check() )
+        if ( i && hypercall_preempt_check() )
             goto preempted;
 
         if ( unlikely(__copy_from_guest(&mcs->call, call_list, 1)) )
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -375,12 +375,12 @@ static DECLARE_SOFTIRQ_TASKLET(notify_do
 static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
 {
     char kbuf[128];
-    int kcount;
+    int kcount = 0;
     struct domain *cd = current->domain;
 
     while ( count > 0 )
     {
-        if ( hypercall_preempt_check() )
+        if ( kcount && hypercall_preempt_check() )
             return hypercall_create_continuation(
                 __HYPERVISOR_console_io, "iih",
                 CONSOLEIO_write, count, buffer);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] x86: make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
  2014-03-04 11:24 ` [PATCH 1/2] common: " Jan Beulich
@ 2014-03-04 11:26 ` Jan Beulich
  2014-03-04 11:35   ` Andrew Cooper
  2014-03-04 11:30 ` [PATCH 0/2] " Jan Beulich
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 11:26 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim Deegan, Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 4673 bytes --]

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

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



[-- Attachment #2: preempt-progress-x86.patch --]
[-- Type: text/plain, Size: 4721 bytes --]

x86: make hypercall preemption checks consistent

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

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

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] common: make hypercall preemption checks consistent
  2014-03-04 11:24 ` [PATCH 1/2] common: " Jan Beulich
@ 2014-03-04 11:29   ` Andrew Cooper
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-03-04 11:29 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Keir Fraser


[-- Attachment #1.1: Type: text/plain, Size: 2769 bytes --]

On 04/03/14 11:24, Jan Beulich wrote:
> - never preempt on the first iteration (ensure forward progress)
> - do cheap checks first
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

>
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -63,7 +63,7 @@ static void increase_reservation(struct 
>  
>      for ( i = a->nr_done; i < a->nr_extents; i++ )
>      {
> -        if ( hypercall_preempt_check() )
> +        if ( i != a->nr_done && hypercall_preempt_check() )
>          {
>              a->preempted = 1;
>              goto out;
> @@ -109,7 +109,7 @@ static void populate_physmap(struct memo
>  
>      for ( i = a->nr_done; i < a->nr_extents; i++ )
>      {
> -        if ( hypercall_preempt_check() )
> +        if ( i != a->nr_done && hypercall_preempt_check() )
>          {
>              a->preempted = 1;
>              goto out;
> @@ -268,7 +268,7 @@ static void decrease_reservation(struct 
>  
>      for ( i = a->nr_done; i < a->nr_extents; i++ )
>      {
> -        if ( hypercall_preempt_check() && i != a->nr_done )
> +        if ( i != a->nr_done && hypercall_preempt_check() )
>          {
>              a->preempted = 1;
>              goto out;
> @@ -398,7 +398,8 @@ static long memory_exchange(XEN_GUEST_HA
>            i < (exch.in.nr_extents >> in_chunk_order);
>            i++ )
>      {
> -        if ( hypercall_preempt_check() )
> +        if ( i != (exch.nr_exchanged >> in_chunk_order) &&
> +             hypercall_preempt_check() )
>          {
>              exch.nr_exchanged = i << in_chunk_order;
>              rcu_unlock_domain(d);
> --- a/xen/common/multicall.c
> +++ b/xen/common/multicall.c
> @@ -52,7 +52,7 @@ do_multicall(
>  
>      for ( i = 0; !rc && i < nr_calls; i++ )
>      {
> -        if ( hypercall_preempt_check() )
> +        if ( i && hypercall_preempt_check() )
>              goto preempted;
>  
>          if ( unlikely(__copy_from_guest(&mcs->call, call_list, 1)) )
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -375,12 +375,12 @@ static DECLARE_SOFTIRQ_TASKLET(notify_do
>  static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
>  {
>      char kbuf[128];
> -    int kcount;
> +    int kcount = 0;
>      struct domain *cd = current->domain;
>  
>      while ( count > 0 )
>      {
> -        if ( hypercall_preempt_check() )
> +        if ( kcount && hypercall_preempt_check() )
>              return hypercall_create_continuation(
>                  __HYPERVISOR_console_io, "iih",
>                  CONSOLEIO_write, count, buffer);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3698 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
  2014-03-04 11:24 ` [PATCH 1/2] common: " Jan Beulich
  2014-03-04 11:26 ` [PATCH 2/2] x86: " Jan Beulich
@ 2014-03-04 11:30 ` Jan Beulich
  2014-03-11 11:38   ` Ian Campbell
  2014-03-04 11:46 ` Tim Deegan
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 11:30 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

>>> On 04.03.14 at 12:21, "Jan Beulich" <JBeulich@suse.com> wrote:
> - never preempt on the first iteration (ensure forward progress)
> - never preempt on the last iteration (pointless/wasteful)
> - do cheap checks first
> 
> 1: common: make hypercall preemption checks consistent
> 2: x86: make hypercall preemption checks consistent

And just to clarify - this time I did check ARM, and the two uses
of hypercall_preempt_check() it currently has look okay.

Jan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] x86: make hypercall preemption checks consistent
  2014-03-04 11:26 ` [PATCH 2/2] x86: " Jan Beulich
@ 2014-03-04 11:35   ` Andrew Cooper
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-03-04 11:35 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Tim Deegan


[-- Attachment #1.1: Type: text/plain, Size: 5052 bytes --]

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


[-- Attachment #1.2: Type: text/html, Size: 5825 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
                   ` (2 preceding siblings ...)
  2014-03-04 11:30 ` [PATCH 0/2] " Jan Beulich
@ 2014-03-04 11:46 ` Tim Deegan
  2014-03-04 11:52 ` Andrew Cooper
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Tim Deegan @ 2014-03-04 11:46 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Keir Fraser

At 11:21 +0000 on 04 Mar (1393928471), Jan Beulich wrote:
> - never preempt on the first iteration (ensure forward progress)
> - never preempt on the last iteration (pointless/wasteful)
> - do cheap checks first
> 
> 1: common: make hypercall preemption checks consistent
> 2: x86: make hypercall preemption checks consistent
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Tim Deegan <tim@xen.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
                   ` (3 preceding siblings ...)
  2014-03-04 11:46 ` Tim Deegan
@ 2014-03-04 11:52 ` Andrew Cooper
  2014-03-04 12:00   ` Jan Beulich
  2014-03-12  9:35 ` Ping: " Jan Beulich
  2014-03-13 10:26 ` Keir Fraser
  6 siblings, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2014-03-04 11:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Keir Fraser

On 04/03/14 11:21, Jan Beulich wrote:
> - never preempt on the first iteration (ensure forward progress)
> - never preempt on the last iteration (pointless/wasteful)
> - do cheap checks first
>
> 1: common: make hypercall preemption checks consistent
> 2: x86: make hypercall preemption checks consistent
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

All in all, this is a good improvement over what is currently present.

However, given the overhead of creating continuations (particularly for
32bit HVM guests, which have been seen to unconditionally fail the
preemption check by the time the compat layer has run), some of these
operations would probably be better having more than a single guaranteed
operation.

~Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:52 ` Andrew Cooper
@ 2014-03-04 12:00   ` Jan Beulich
  2014-03-04 12:10     ` David Vrabel
  0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 12:00 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, Keir Fraser

>>> On 04.03.14 at 12:52, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 04/03/14 11:21, Jan Beulich wrote:
>> - never preempt on the first iteration (ensure forward progress)
>> - never preempt on the last iteration (pointless/wasteful)
>> - do cheap checks first
>>
>> 1: common: make hypercall preemption checks consistent
>> 2: x86: make hypercall preemption checks consistent
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> All in all, this is a good improvement over what is currently present.
> 
> However, given the overhead of creating continuations (particularly for
> 32bit HVM guests, which have been seen to unconditionally fail the
> preemption check by the time the compat layer has run), some of these
> operations would probably be better having more than a single guaranteed
> operation.

I agree, but I wanted to do one step at a time. Judging how much
work we want to permit done between preemption points will be
either heavy guess work, or require quite a bit of performance
measurement...

Jan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 12:00   ` Jan Beulich
@ 2014-03-04 12:10     ` David Vrabel
  2014-03-04 13:06       ` Jan Beulich
  0 siblings, 1 reply; 15+ messages in thread
From: David Vrabel @ 2014-03-04 12:10 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Keir Fraser, xen-devel

On 04/03/14 12:00, Jan Beulich wrote:
>>>> On 04.03.14 at 12:52, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 04/03/14 11:21, Jan Beulich wrote:
>>> - never preempt on the first iteration (ensure forward progress)
>>> - never preempt on the last iteration (pointless/wasteful)
>>> - do cheap checks first
>>>
>>> 1: common: make hypercall preemption checks consistent
>>> 2: x86: make hypercall preemption checks consistent
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> All in all, this is a good improvement over what is currently present.
>>
>> However, given the overhead of creating continuations (particularly for
>> 32bit HVM guests, which have been seen to unconditionally fail the
>> preemption check by the time the compat layer has run), some of these
>> operations would probably be better having more than a single guaranteed
>> operation.
> 
> I agree, but I wanted to do one step at a time. Judging how much
> work we want to permit done between preemption points will be
> either heavy guess work, or require quite a bit of performance
> measurement...

Perhaps something time-based?  Record the time at start and make
hypercall_preempt_check() return true if more than T time has elapsed?

David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 12:10     ` David Vrabel
@ 2014-03-04 13:06       ` Jan Beulich
  2014-03-04 14:07         ` David Vrabel
  0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-03-04 13:06 UTC (permalink / raw)
  To: David Vrabel; +Cc: Andrew Cooper, Keir Fraser, xen-devel

>>> On 04.03.14 at 13:10, David Vrabel <david.vrabel@citrix.com> wrote:
> On 04/03/14 12:00, Jan Beulich wrote:
>>>>> On 04.03.14 at 12:52, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> On 04/03/14 11:21, Jan Beulich wrote:
>>>> - never preempt on the first iteration (ensure forward progress)
>>>> - never preempt on the last iteration (pointless/wasteful)
>>>> - do cheap checks first
>>>>
>>>> 1: common: make hypercall preemption checks consistent
>>>> 2: x86: make hypercall preemption checks consistent
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> All in all, this is a good improvement over what is currently present.
>>>
>>> However, given the overhead of creating continuations (particularly for
>>> 32bit HVM guests, which have been seen to unconditionally fail the
>>> preemption check by the time the compat layer has run), some of these
>>> operations would probably be better having more than a single guaranteed
>>> operation.
>> 
>> I agree, but I wanted to do one step at a time. Judging how much
>> work we want to permit done between preemption points will be
>> either heavy guess work, or require quite a bit of performance
>> measurement...
> 
> Perhaps something time-based?  Record the time at start and make
> hypercall_preempt_check() return true if more than T time has elapsed?

That's certainly an interesting idea. But it doesn't remove the need
to determine the actual value of the parameter to use (T in this case).

Jan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 13:06       ` Jan Beulich
@ 2014-03-04 14:07         ` David Vrabel
  0 siblings, 0 replies; 15+ messages in thread
From: David Vrabel @ 2014-03-04 14:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Keir Fraser, xen-devel

On 04/03/14 13:06, Jan Beulich wrote:
>>>> On 04.03.14 at 13:10, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 04/03/14 12:00, Jan Beulich wrote:
>>>>>> On 04.03.14 at 12:52, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 04/03/14 11:21, Jan Beulich wrote:
>>>>> - never preempt on the first iteration (ensure forward progress)
>>>>> - never preempt on the last iteration (pointless/wasteful)
>>>>> - do cheap checks first
>>>>>
>>>>> 1: common: make hypercall preemption checks consistent
>>>>> 2: x86: make hypercall preemption checks consistent
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> All in all, this is a good improvement over what is currently present.
>>>>
>>>> However, given the overhead of creating continuations (particularly for
>>>> 32bit HVM guests, which have been seen to unconditionally fail the
>>>> preemption check by the time the compat layer has run), some of these
>>>> operations would probably be better having more than a single guaranteed
>>>> operation.
>>>
>>> I agree, but I wanted to do one step at a time. Judging how much
>>> work we want to permit done between preemption points will be
>>> either heavy guess work, or require quite a bit of performance
>>> measurement...
>>
>> Perhaps something time-based?  Record the time at start and make
>> hypercall_preempt_check() return true if more than T time has elapsed?
> 
> That's certainly an interesting idea. But it doesn't remove the need
> to determine the actual value of the parameter to use (T in this case).

Sure, but it should be just one parameter for all hypercalls instead of
having to consider each one separately.

David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:30 ` [PATCH 0/2] " Jan Beulich
@ 2014-03-11 11:38   ` Ian Campbell
  0 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2014-03-11 11:38 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

On Tue, 2014-03-04 at 11:30 +0000, Jan Beulich wrote:
> >>> On 04.03.14 at 12:21, "Jan Beulich" <JBeulich@suse.com> wrote:
> > - never preempt on the first iteration (ensure forward progress)
> > - never preempt on the last iteration (pointless/wasteful)
> > - do cheap checks first
> > 
> > 1: common: make hypercall preemption checks consistent
> > 2: x86: make hypercall preemption checks consistent
> 
> And just to clarify - this time I did check ARM, and the two uses
> of hypercall_preempt_check() it currently has look okay.

Thanks!

Ian.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Ping: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
                   ` (4 preceding siblings ...)
  2014-03-04 11:52 ` Andrew Cooper
@ 2014-03-12  9:35 ` Jan Beulich
  2014-03-13 10:26 ` Keir Fraser
  6 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2014-03-12  9:35 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel

>>> On 04.03.14 at 12:21, "Jan Beulich" <JBeulich@suse.com> wrote:
> - never preempt on the first iteration (ensure forward progress)
> - never preempt on the last iteration (pointless/wasteful)
> - do cheap checks first
> 
> 1: common: make hypercall preemption checks consistent
> 2: x86: make hypercall preemption checks consistent
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] make hypercall preemption checks consistent
  2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
                   ` (5 preceding siblings ...)
  2014-03-12  9:35 ` Ping: " Jan Beulich
@ 2014-03-13 10:26 ` Keir Fraser
  6 siblings, 0 replies; 15+ messages in thread
From: Keir Fraser @ 2014-03-13 10:26 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 420 bytes --]


> Jan Beulich <mailto:JBeulich@suse.com>
> 4 March 2014 11:21
> - never preempt on the first iteration (ensure forward progress)
> - never preempt on the last iteration (pointless/wasteful)
> - do cheap checks first
>
> 1: common: make hypercall preemption checks consistent
> 2: x86: make hypercall preemption checks consistent
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
Acked-by: Keir Fraser <keir@xen.org>

[-- Attachment #1.2.1: Type: text/html, Size: 1789 bytes --]

[-- Attachment #1.2.2: compose-unknown-contact.jpg --]
[-- Type: image/jpeg, Size: 770 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-03-13 10:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-04 11:21 [PATCH 0/2] make hypercall preemption checks consistent Jan Beulich
2014-03-04 11:24 ` [PATCH 1/2] common: " Jan Beulich
2014-03-04 11:29   ` Andrew Cooper
2014-03-04 11:26 ` [PATCH 2/2] x86: " Jan Beulich
2014-03-04 11:35   ` Andrew Cooper
2014-03-04 11:30 ` [PATCH 0/2] " Jan Beulich
2014-03-11 11:38   ` Ian Campbell
2014-03-04 11:46 ` Tim Deegan
2014-03-04 11:52 ` Andrew Cooper
2014-03-04 12:00   ` Jan Beulich
2014-03-04 12:10     ` David Vrabel
2014-03-04 13:06       ` Jan Beulich
2014-03-04 14:07         ` David Vrabel
2014-03-12  9:35 ` Ping: " Jan Beulich
2014-03-13 10:26 ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).