From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751893Ab2LDCgP (ORCPT ); Mon, 3 Dec 2012 21:36:15 -0500 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:55911 "EHLO e28smtp04.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751409Ab2LDCgN (ORCPT ); Mon, 3 Dec 2012 21:36:13 -0500 Message-ID: <1354588562.2649.11.camel@ThinkPad-T5421.cn.ibm.com> Subject: Re: [RFC PATCH v3] Add rcu user eqs exception hooks for async page fault From: Li Zhong To: Gleb Natapov Cc: Frederic Weisbecker , linux-next list , LKML , paulmck@linux.vnet.ibm.com, sasha.levin@oracle.com, avi@redhat.com Date: Tue, 04 Dec 2012 10:36:02 +0800 In-Reply-To: <20121203095708.GO8731@redhat.com> References: <1353993325.14050.49.camel@ThinkPad-T5421.cn.ibm.com> <1354090704.3054.13.camel@ThinkPad-T5421.cn.ibm.com> <20121128135345.GG928@redhat.com> <20121129110717.GA9711@redhat.com> <1354267121.2552.65.camel@ThinkPad-T5421.cn.ibm.com> <20121203095708.GO8731@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 12120402-5564-0000-0000-0000059F4829 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2012-12-03 at 11:57 +0200, Gleb Natapov wrote: > Please regenerate the patch against > git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue. Done. By the way, the included file is replaced with in latest next tree(91d1aa43 from rcu tree). Seems if they are merged, there won't be conflicts, but we need change the including file name after that. I don't know how to handle this kind of thing... Thanks, Zhong > > On Fri, Nov 30, 2012 at 05:18:41PM +0800, Li Zhong wrote: > > This patch adds user eqs exception hooks for async page fault page not > > present code path, to exit the user eqs and re-enter it as necessary. > > > > Async page fault is different from other exceptions that it may be > > triggered from idle process, so we still need rcu_irq_enter() and > > rcu_irq_exit() to exit cpu idle eqs when needed, to protect the code > > that needs use rcu. > > > > As Frederic pointed out it would be safest and simplest to protect the > > whole kvm_async_pf_task_wait(). Otherwise, "we need to check all the > > code there deeply for potential RCU uses and ensure it will never be > > extended later to use RCU.". > > > > However, We'd better re-enter the cpu idle eqs if we get the exception > > in cpu idle eqs, by calling rcu_irq_exit() before native_safe_halt(). > > > > So the patch does what Frederic suggested for rcu_irq_*() API usage > > here, except that I moved the rcu_irq_*() pair originally in > > do_async_page_fault() into kvm_async_pf_task_wait(). > > > > That's because, I think it's better to have rcu_irq_*() pairs to be in > > one function ( rcu_irq_exit() after rcu_irq_enter() ), especially here, > > kvm_async_pf_task_wait() has other callers, which might cause > > rcu_irq_exit() be called without a matching rcu_irq_enter() before it, > > which is illegal if the cpu happens to be in rcu idle state. > > > > Signed-off-by: Li Zhong > > --- > > arch/x86/kernel/kvm.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > > index 4180a87..342b00b 100644 > > --- a/arch/x86/kernel/kvm.c > > +++ b/arch/x86/kernel/kvm.c > > @@ -42,6 +42,7 @@ > > #include > > #include > > #include > > +#include > > > > static int kvmapf = 1; > > > > @@ -112,6 +113,8 @@ void kvm_async_pf_task_wait(u32 token) > > DEFINE_WAIT(wait); > > int cpu, idle; > > > > + rcu_irq_enter(); > > + > > cpu = get_cpu(); > > idle = idle_cpu(cpu); > > put_cpu(); > > @@ -123,6 +126,8 @@ void kvm_async_pf_task_wait(u32 token) > > hlist_del(&e->link); > > kfree(e); > > spin_unlock(&b->lock); > > + > > + rcu_irq_exit(); > > return; > > } > > > > @@ -147,13 +152,16 @@ void kvm_async_pf_task_wait(u32 token) > > /* > > * We cannot reschedule. So halt. > > */ > > + rcu_irq_exit(); > > native_safe_halt(); > > + rcu_irq_enter(); > > local_irq_disable(); > > } > > } > > if (!n.halted) > > finish_wait(&n.wq, &wait); > > > > + rcu_irq_exit(); > > return; > > } > > EXPORT_SYMBOL_GPL(kvm_async_pf_task_wait); > > @@ -247,10 +255,10 @@ do_async_page_fault(struct pt_regs *regs, unsigned long error_code) > > break; > > case KVM_PV_REASON_PAGE_NOT_PRESENT: > > /* page is swapped out by the host. */ > > - rcu_irq_enter(); > > + exception_enter(regs); > > exit_idle(); > > kvm_async_pf_task_wait((u32)read_cr2()); > > - rcu_irq_exit(); > > + exception_exit(regs); > > break; > > case KVM_PV_REASON_PAGE_READY: > > rcu_irq_enter(); > > -- > > 1.7.11.4 > > -- > Gleb. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >