* [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
* 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
* [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 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 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 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
* 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
* 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).
![[-- Attachment #1.2.2: compose-unknown-contact.jpg --] [-- Type: image/jpeg, Size: 770 bytes --]](../../532187C5.9050002@gmail.com/1.2.2-compose-unknown-contact.jpg){kind=link}