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

["Luis R. Rodriguez" <mcgrof@xxxxxxxx>]

On Thu, Feb 05, 2015 at 12:41:17PM +0000, David Vrabel wrote:
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -982,6 +982,9 @@ ENTRY(xen_hypervisor_callback)
>  ENTRY(xen_do_upcall)
>  1:   mov %esp, %eax
>       call xen_evtchn_do_upcall
> +#ifndef CONFIG_PREEMPT
> +     call xen_maybe_preempt_hcall
> +#endif
>       jmp  ret_from_intr
>       CFI_ENDPROC
>  ENDPROC(xen_hypervisor_callback)

Oh good, did you get to test 32-bit? I was still trying to get
a good recent Xen build to test the domU suggestion you had.

> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 9ebaf63..9069401 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1198,6 +1198,9 @@ ENTRY(xen_do_hypervisor_callback)   # 
> do_hypervisor_callback(struct *pt_regs)
>       popq %rsp
>       CFI_DEF_CFA_REGISTER rsp
>       decl PER_CPU_VAR(irq_count)
> +#ifndef CONFIG_PREEMPT
> +     call xen_maybe_preempt_hcall
> +#endif
>       jmp  error_exit
>       CFI_ENDPROC
>  END(xen_do_hypervisor_callback)

> diff --git a/drivers/xen/preempt.c b/drivers/xen/preempt.c
> new file mode 100644
> index 0000000..e2bcc01
> --- /dev/null
> +++ b/drivers/xen/preempt.c

<-- ... -->

> +DEFINE_PER_CPU(bool, xen_in_preemptible_hcall);
> +EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
> +
> +void xen_maybe_preempt_hcall(void)
> +{
> +     if (__this_cpu_read(xen_in_preemptible_hcall)) {
> +             /*
> +              * Clear flag as we may be rescheduled on a different
> +              * cpu.
> +              */
> +             __this_cpu_write(xen_in_preemptible_hcall, false);

This read might be on CPU 3.

> +             _cond_resched();
> +             __this_cpu_write(xen_in_preemptible_hcall, true);

This CPU wright might happen on CPU 1 and as such it would
write true to another CPU. I had tested this approach and
it was my original approach and while it sort of works its
obviously broken due to the above. After considering
Andy's original request to use pt_regs instead it seemed a lot
safer than trying to fix this approach, so that is what I
went with.

  Luis

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