On Wed, May 29, 2013 at 11:03:19AM +0800, Xiao Guangrong wrote: > >>> the pages since other vcpus may be doing locklessly shadow > >>> page walking > > > > Ah, yes, i agree with you. > > > > We can introduce a list, say kvm->arch.obsolte_pages, to link all of the > > zapped-page, the page-shrink will free the page on that list first. > > > > Marcelo, if you do not have objection on patch 1 ~ 8 and 11, could you please > > let them merged first, and do add some comments and tlb optimization later? > > Exclude patch 11 please, since it depends on the "collapse" optimization. I'm fine with patch 1 being merged. I think the remaining patches need better understanding or explanation. The problems i see are: 1) The magic number "10" to zap before considering reschedule is annoying. It would be good to understand why it is needed at all. But then again, the testcase is measuring kvm_mmu_zap_all performance alone which we know is not a common operation, so perhaps there is no need for that minimum-pages-to-zap-before-reschedule. 2) The problem above (retention of large number of pages while zapping) can be fatal, it can lead to OOM and host crash. 3) Make sure that introduction of obsolete pages can not lead to a huge number of shadow pages around (the correct reason you gave for not merging https://patchwork.kernel.org/patch/2309641/ is not true anymore obsolete pages). Other than these points, i'm fine with obsolete pages optimization to speed up kvm_mmu_zap_all and the rest of the patchset. -- 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
