On Wed, Aug 15, 2012 at 11:11:51AM +0900, Takuya Yoshikawa wrote:
> On Tue, 14 Aug 2012 12:17:12 -0300
> Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
>
> > - if (kvm->arch.n_used_mmu_pages > 0) {
> > - if (!nr_to_scan--)
> > - break;
>
> -- (*1)
>
> > + if (!kvm->arch.n_used_mmu_pages)
> > continue;
>
> -- (*2)
>
> > - }
> >
> > idx = srcu_read_lock(&kvm->srcu);
> > spin_lock(&kvm->mmu_lock);
> >
> > This patch removes the maximum (successful) loops, which is nr_scan ==
> > sc->nr_to_scan.
>
> IIUC, there was no successful loop from the beginning:
>
> if (kvm->arch.n_used_mmu_pages > 0) {
> if (!nr_to_scan--)
> break; -- (*1)
> continue; -- (*2)
> }
>
> Before the patch even when we find a VM with kvm->arch.n_used_mmu_pages
> greater than 0, we just do either:
> skip it (*2) or
> break (*1) if nr_to_scan becomes 0.
>
> We only reach to
> kvm_mmu_remove_some_alloc_mmu_pages(kvm, &invalid_list);
> kvm_mmu_commit_zap_page(kvm, &invalid_list);
> when (kvm->arch.n_used_mmu_pages == 0) that is probably why
> commit 85b7059169e128c57a3a8a3e588fb89cb2031da1
> KVM: MMU: fix shrinking page from the empty mmu
>
> could hit the very unlikely condition so easily.
>
> So we are just looping for trying to free from empty MMUs.
>
> > The description above where you say 'possibility that we see
> > "n_used_mmu_pages == 0" 128 times' does not match the patch above.
>
> Sorry about that.
>
> > If the patch is correct, then please explain it clearly in the
> > changelog.
>
> Yes, I will do so.
>
> > What is the reasoning to remove nr_to_scan? What tests did you perform?
>
> I just confirmed:
> - mmu_shrink() did not free any pages:
> just checked all VMs and did "continue"
> - with my patch, it could free from the first VM with (n_used_mmu_pages > 0)
>
> About nr_to_scan:
> If my explanation above is right, this is not functioning at all.
> But since it will not hurt anyone and may help us when we change
> our batch size, I won't remove it in the next version.
>
> Thanks,
> Takuya
Ouch.
OK, please resend with dumb-proof changelog.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
