Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-14 Thread Marcelo Tosatti
On Wed, Apr 14, 2010 at 10:14:29AM +0800, Xiao Guangrong wrote: Marcelo Tosatti wrote: On Tue, Apr 13, 2010 at 09:34:14AM +0800, Xiao Guangrong wrote: Marcelo Tosatti wrote: @@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-13 Thread Marcelo Tosatti
On Tue, Apr 13, 2010 at 09:34:14AM +0800, Xiao Guangrong wrote: Marcelo Tosatti wrote: @@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm, for_each_sp(pages, sp, parents, i) { kvm_mmu_zap_page(kvm, sp);

[PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Xiao Guangrong
- calculate zapped page number properly in mmu_zap_unsync_children() - calculate freeed page number properly kvm_mmu_change_mmu_pages() - restart list walking if have children page zapped Signed-off-by: Xiao Guangrong xiaoguangr...@cn.fujitsu.com --- arch/x86/kvm/mmu.c |7 --- 1 files

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Avi Kivity
On 04/12/2010 11:01 AM, Xiao Guangrong wrote: - calculate zapped page number properly in mmu_zap_unsync_children() - calculate freeed page number properly kvm_mmu_change_mmu_pages() - restart list walking if have children page zapped Signed-off-by: Xiao Guangrongxiaoguangr...@cn.fujitsu.com ---

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Xiao Guangrong
Avi Kivity wrote: kvm-arch.n_free_mmu_pages = 0; @@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) !sp-role.invalid) { pgprintk(%s: zap %lx %x\n, __func__, gfn, sp-role.word); -

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Avi Kivity
On 04/12/2010 11:53 AM, Xiao Guangrong wrote: kvm-arch.n_free_mmu_pages = 0; @@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) !sp-role.invalid) { pgprintk(%s: zap %lx %x\n, __func__, gfn, sp-role.word); -

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Xiao Guangrong
Hi Avi, Avi Kivity wrote: hlist_for_each_entry_safe() is supposed to be be safe against removal of the element that is pointed to by the iteration cursor. If we destroyed the next point, hlist_for_each_entry_safe() is unsafe. List hlist_for_each_entry_safe()'s code: |#define

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Avi Kivity
On 04/12/2010 12:22 PM, Xiao Guangrong wrote: Hi Avi, Avi Kivity wrote: hlist_for_each_entry_safe() is supposed to be be safe against removal of the element that is pointed to by the iteration cursor. If we destroyed the next point, hlist_for_each_entry_safe() is unsafe. List

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Xiao Guangrong
Avi Kivity wrote: On 04/12/2010 12:22 PM, Xiao Guangrong wrote: Hi Avi, Avi Kivity wrote: But kvm_mmu_zap_page() will only destroy sp == tpos == pos; n points at pos-next already, so it's safe. kvm_mmu_zap_page(sp) not only zaps sp but also zaps all sp's unsync children pages, if n is

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Avi Kivity
On 04/12/2010 03:22 PM, Xiao Guangrong wrote: But kvm_mmu_zap_page() will only destroy sp == tpos == pos; n points at pos-next already, so it's safe. kvm_mmu_zap_page(sp) not only zaps sp but also zaps all sp's unsync children pages, if n is just sp's unsyc child, just at the same

Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path

2010-04-12 Thread Marcelo Tosatti
On Mon, Apr 12, 2010 at 04:01:09PM +0800, Xiao Guangrong wrote: - calculate zapped page number properly in mmu_zap_unsync_children() - calculate freeed page number properly kvm_mmu_change_mmu_pages() - restart list walking if have children page zapped Signed-off-by: Xiao Guangrong