On Wed, Sep 26, 2018 at 06:42:12AM -0700, Sean Christopherson wrote: >On Wed, Sep 26, 2018 at 10:42:39AM +0800, kvm-owner@xxxxxxxxxxxxxxx wrote: >> * nr_mmu_pages would be non-zero only if kvm->arch.n_requested_mmu_pages is >> non-zero. >> >> * nr_mmu_pages is always non-zero, since kvm_mmu_calculate_mmu_pages() >> never return zero. >> >> Based on these two reasons, we can merge the two *if* clause and use the >> return value from kvm_mmu_calculate_mmu_pages() directly. >> >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx> >> --- > Hi, Sean, Thanks for your comment. >Might be worth adding a blurb to the changelog to note that this also >eliminates code that might lead a reader to incorrectly believe it's >possible for nr_mmu_pages to be zero. Yep, I had this illusion when I read the code. > >Another idea would be to rename kvm_mmu_calculate_mmu_pages() to >kvm_mmu_calculate_default_mmu_pages() to help communicate that it's >designed to always return a sane value and that it's only intended to >be employed when userspace hasn't explicitly set the number of pages. > Will dress these two comment in v2. >Reviewed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> -- Wei Yang Help you, Help me
