Xiao Guangrong wrote: > > Gui Jianfeng wrote: >> Currently, in kvm_mmu_change_mmu_pages(kvm, page), "used_pages--" is performed after calling >> kvm_mmu_zap_page() in spite of that whether "page" is actually reclaimed. Because root sp won't be >> reclaimed by kvm_mmu_zap_page(). So making kvm_mmu_zap_page() return total number of reclaimed sp >> makes more sense. A new flag is put into kvm_mmu_zap_page() to indicate whether the top page is reclaimed. >> > > This bug only hurts kvm_mmu_change_mmu_pages() function, we'd better allow 'self_deleted' is > NULL, then we can pass NULL at other place. Ok, will change. Will send a updated version. > >> @@ -1571,7 +1584,8 @@ restart: >> pgprintk("%s: gfn %lx role %x\n", __func__, gfn, >> sp->role.word); >> r = 1; >> - if (kvm_mmu_zap_page(kvm, sp)) >> + ret = kvm_mmu_zap_page(kvm, sp, &self_deleted); >> + if (ret > 1 || (ret == 1 && self_deleted == 0)) >> goto restart; > > Maybe we can keep kvm_mmu_zap_page() returns the number of zapped children, > and 'self_deleted' indicates whether self is zapped, then we no need modify > those function, just fix kvm_mmu_change_mmu_pages() that is if 'self_deleted == 1', > inc 'used_pages' I think kvm_mmu_zap_page() returning the total zapped number is more intuitive, so i'd prefer to retain the original code. Thanks. Gui, Thanks > > Xiao > > > -- 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