On Tue, May 21, 2013 at 11:39:03AM +0300, Gleb Natapov wrote: > > Any pages with stale information will be zapped by kvm_mmu_zap_all(). > > When that happens, page faults will take place which will automatically > > use the new generation number. > > > > So still not clear why is this necessary. > > > This is not, strictly speaking, necessary, but it is the sane thing to do. > You cannot update page's generation number to prevent it from been > destroyed since after kvm_mmu_zap_all() completes stale ptes in the > shadow page may point to now deleted memslot. So why build shadow page > table with a page that is in a process of been destroyed? OK, can this be introduced separately, in a later patch, with separate justification, then? Xiao please have the first patches of the patchset focus on the problem at hand: fix long mmu_lock hold times. > Not sure what you mean again. We flush TLB once before entering this function. > kvm_reload_remote_mmus() does this for us, no? kvm_reload_remote_mmus() is used as an optimization, its separate from the problem solution. > > > > What was suggested was... go to phrase which starts with "The only purpose > > of the generation number should be to". > > > > The comment quoted here does not match that description. > > > The comment describes what code does and in this it is correct. > > You propose to not reload roots right away and do it only when root sp > is encountered, right? So my question is what's the point? There are, > obviously, root sps with invalid generation number at this point, so > reload will happen regardless in kvm_mmu_prepare_zap_page(). So why not > do it here right away and avoid it in kvm_mmu_prepare_zap_page() for > invalid and obsolete sps as I proposed in one of my email? Sure. But Xiao please introduce that TLB collapsing optimization as a later patch, so we can reason about it in a more organized fashion. -- 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
