On Sun, Jun 08, 2008 at 01:48:53AM -0300, Marcelo Tosatti wrote: > > writeable sptes (I did exactly the same mistake once). > > Why not posting it at the time?
See the comments in the mail I posted a few days ago with subject "[patch] kvm with mmu notifier v18". I posted the incompatibility with rmap_next and rmap_remove in the same loop, the day after I fixed it in ksm. > This makes no difference since rmap_next() still can't handle the case > where rmap_remove() left a single entry in the array and "spte" has been > passed as non-NULL: I don't see the problem, with the fixed code we never pass spte not-NULL after any rmap_remove run. > How bad you think restarting is? Unless there are workloads with very It probably worth to optimize it with rmap_next_safe in the long run, but it may not be urgent as it may be lost in the noise even if the chains are fairly long. To be sure you could try to profile it and see if rmap_write_protect goes up in the opreport. > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index aaccc40..d01d098 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -640,6 +640,7 @@ static void rmap_write_protect(struct kvm *kvm, u64 > gfn) > rmap_remove(kvm, spte); > --kvm->stat.lpages; > set_shadow_pte(spte,shadow_trap_nonpresent_pte); > + spte = NULL; > write_protected = 1; > } > spte = rmap_next(kvm, rmapp, spte); This is shorter and functionally equivalent to my proposed fix, so you seem to agree this makes a difference by posting this patch, and I sure agree with the above fix too! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
