From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCHv5] x86/xen: allow privcmd hypercalls to be preempted Date: Thu, 5 Feb 2015 18:23:28 +0000 Message-ID: <54D3B520.7030209@citrix.com> References: <1423140077-32356-1-git-send-email-david.vrabel@citrix.com> <20150205181445.GV19988@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YJR5R-0000Ua-D4 for xen-devel@lists.xenproject.org; Thu, 05 Feb 2015 18:23:33 +0000 In-Reply-To: <20150205181445.GV19988@wotan.suse.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Luis R. Rodriguez" , David Vrabel Cc: xen-devel@lists.xenproject.org, Boris Ostrovsky , Borislav Petkov , Steven Rostedt , Andy Lutomirski List-Id: xen-devel@lists.xenproject.org On 05/02/15 18:14, Luis R. Rodriguez wrote: > 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. That is the entire point, because the hypercall is now being continued on CPU1 rather than CPU3. The "preemptibleness" follows the task which initiated the hypercall, not the cpu which it happened to start executing on. ~Andrew > 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@lists.xen.org > http://lists.xen.org/xen-devel