[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCHv3] x86/xen: allow privcmd hypercalls to be preempted



On Thu, Feb 27, 2014 at 03:07:55PM +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@xxxxxxxxxx>
> 
> Hypercalls submitted by user space tools via the privcmd driver can
> take a long time (potentially many 10s of seconds) if the hypercall
> has many sub-operations.
> 
> A fully preemptible kernel may deschedule such as task in any upcall
> called from a hypercall continuation.
> 
> However, in a kernel with voluntary or no preemption, hypercall
> continuations in Xen allow event handlers to be run but the task
> issuing the hypercall will not be descheduled until the hypercall is
> complete and the ioctl returns to user space.  These long running
> tasks may also trigger the kernel's soft lockup detection.
> 
> Add xen_preemptible_hcall_begin() and xen_preemptible_hcall_end() to
> bracket hypercalls that may be preempted.  Use these in the privcmd
> driver.
> 
> When returning from an upcall, call preempt_schedule_irq() if the
> current task was within a preemptible hypercall.
> 
> Since preempt_schedule_irq() can move the task to a different CPU,
> clear and set xen_in_preemptible_hcall around the call.
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
> Changes in v3:
> - Export xen_in_preemptible_hcall (to fix modular privcmd driver).

On 32-bit builds I get:

arch/x86/built-in.o: In function `xen_do_upcall':
/home/konrad/ssd/konrad/linux/arch/x86/kernel/entry_32.S:1016: undefined
reference to `TI_preempt_count'

> 
> Changes in v2:
> - Use per-cpu variable to mark preemptible regions
> - Call preempt_schedule_irq() from the correct place in
>   xen_hypervisor_callback
> ---
>  arch/x86/kernel/entry_32.S |   23 +++++++++++++++++++++++
>  arch/x86/kernel/entry_64.S |   19 +++++++++++++++++++
>  drivers/xen/Makefile       |    2 +-
>  drivers/xen/preempt.c      |   17 +++++++++++++++++
>  drivers/xen/privcmd.c      |    2 ++
>  include/xen/xen-ops.h      |   27 +++++++++++++++++++++++++++
>  6 files changed, 89 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/xen/preempt.c
> 
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index a2a4f46..b99bc9c 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -998,7 +998,30 @@ ENTRY(xen_hypervisor_callback)
>  ENTRY(xen_do_upcall)
>  1:   mov %esp, %eax
>       call xen_evtchn_do_upcall
> +#ifdef CONFIG_PREEMPT
>       jmp  ret_from_intr
> +#else
> +     GET_THREAD_INFO(%ebp)
> +#ifdef CONFIG_VM86
> +     movl PT_EFLAGS(%esp), %eax      # mix EFLAGS and CS
> +     movb PT_CS(%esp), %al
> +     andl $(X86_EFLAGS_VM | SEGMENT_RPL_MASK), %eax
> +#else
> +     movl PT_CS(%esp), %eax
> +     andl $SEGMENT_RPL_MASK, %eax
> +#endif
> +     cmpl $USER_RPL, %eax
> +     jae resume_userspace            # returning to v8086 or userspace
> +     DISABLE_INTERRUPTS(CLBR_ANY)
> +     cmpl $0,TI_preempt_count(%ebp)  # non-zero preempt_count ?
> +     jnz resume_kernel
> +     cmpb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     jz resume_kernel
> +     movb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     call preempt_schedule_irq
> +     movb $1,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     jmp resume_kernel
> +#endif /* CONFIG_PREEMPT */
>       CFI_ENDPROC
>  ENDPROC(xen_hypervisor_callback)
>  
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 1e96c36..d8f4fd8 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1404,7 +1404,25 @@ ENTRY(xen_do_hypervisor_callback)   # 
> do_hypervisor_callback(struct *pt_regs)
>       popq %rsp
>       CFI_DEF_CFA_REGISTER rsp
>       decl PER_CPU_VAR(irq_count)
> +#ifdef CONFIG_PREEMPT
>       jmp  error_exit
> +#else
> +     movl %ebx, %eax
> +     RESTORE_REST
> +     DISABLE_INTERRUPTS(CLBR_NONE)
> +     TRACE_IRQS_OFF
> +     GET_THREAD_INFO(%rcx)
> +     testl %eax, %eax
> +     je error_exit_user
> +     cmpl $0,PER_CPU_VAR(__preempt_count)
> +     jnz retint_kernel
> +     cmpb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     jz retint_kernel
> +     movb $0,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     call preempt_schedule_irq
> +     movb $1,PER_CPU_VAR(xen_in_preemptible_hcall)
> +     jmp retint_kernel
> +#endif
>       CFI_ENDPROC
>  END(xen_do_hypervisor_callback)
>  
> @@ -1629,6 +1647,7 @@ ENTRY(error_exit)
>       GET_THREAD_INFO(%rcx)
>       testl %eax,%eax
>       jne retint_kernel
> +error_exit_user:
>       LOCKDEP_SYS_EXIT_IRQ
>       movl TI_flags(%rcx),%edx
>       movl $_TIF_WORK_MASK,%edi
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 45e00af..6b867e9 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -2,7 +2,7 @@ ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
>  obj-$(CONFIG_HOTPLUG_CPU)            += cpu_hotplug.o
>  endif
>  obj-$(CONFIG_X86)                    += fallback.o
> -obj-y        += grant-table.o features.o balloon.o manage.o
> +obj-y        += grant-table.o features.o balloon.o manage.o preempt.o
>  obj-y        += events/
>  obj-y        += xenbus/
>  
> diff --git a/drivers/xen/preempt.c b/drivers/xen/preempt.c
> new file mode 100644
> index 0000000..b5a3e98
> --- /dev/null
> +++ b/drivers/xen/preempt.c
> @@ -0,0 +1,17 @@
> +/*
> + * Preemptible hypercalls
> + *
> + * Copyright (C) 2014 Citrix Systems R&D ltd.
> + *
> + * This source code is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + */
> +
> +#include <xen/xen-ops.h>
> +
> +#ifndef CONFIG_PREEMPT
> +DEFINE_PER_CPU(bool, xen_in_preemptible_hcall);
> +EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
> +#endif
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 569a13b..59ac71c 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -56,10 +56,12 @@ static long privcmd_ioctl_hypercall(void __user *udata)
>       if (copy_from_user(&hypercall, udata, sizeof(hypercall)))
>               return -EFAULT;
>  
> +     xen_preemptible_hcall_begin();
>       ret = privcmd_call(hypercall.op,
>                          hypercall.arg[0], hypercall.arg[1],
>                          hypercall.arg[2], hypercall.arg[3],
>                          hypercall.arg[4]);
> +     xen_preemptible_hcall_end();
>  
>       return ret;
>  }
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index fb2ea8f..6d8c042 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -35,4 +35,31 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
>                              int numpgs, struct page **pages);
>  
>  bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
> +
> +#ifdef CONFIG_PREEMPT
> +
> +static inline void xen_preemptible_hcall_begin(void)
> +{
> +}
> +
> +static inline void xen_preemptible_hcall_end(void)
> +{
> +}
> +
> +#else
> +
> +DECLARE_PER_CPU(bool, xen_in_preemptible_hcall);
> +
> +static inline void xen_preemptible_hcall_begin(void)
> +{
> +     __this_cpu_write(xen_in_preemptible_hcall, true);
> +}
> +
> +static inline void xen_preemptible_hcall_end(void)
> +{
> +     __this_cpu_write(xen_in_preemptible_hcall, false);
> +}
> +
> +#endif /* CONFIG_PREEMPT */
> +
>  #endif /* INCLUDE_XEN_OPS_H */
> -- 
> 1.7.2.5
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.