On Tue, Jun 15, 2010 at 06:55:22AM -0700, Dave Hansen wrote:
>
> I think doing this makes the code much more readable. That's
> borne out by the fact that this patch removes code. "used"
> also happens to be the number that we need to return back to
> the slab code when our shrinker gets called. Keeping this
> value as opposed to free makes the next patch simpler.
>
> So, 'struct kvm' is kzalloc()'d. 'struct kvm_arch' is a
> structure member (and not a pointer) of 'struct kvm'. That
> means they start out zeroed. I _think_ they get initialized
> properly by kvm_mmu_change_mmu_pages(). But, that only happens
> via kvm ioctls.
>
> I have a suspicion that they values are actually inconsistent
> until those ioctls get called; "free" and "alloc" are both zero.
> But, the VM can't really get run until these ioctl()s get called
> anyway. There are also some checks for negative "used_pages"
> values which confused me. It might all tie together.
>
> Anyway, another benefit of storing 'used' intead of 'free' is
> that the values are consistent from the moment the structure is
> allocated: no negative "used" value.
>
> Signed-off-by: Dave Hansen <dave@xxxxxxxxxxxxxxxxxx>
> ---
>
> linux-2.6.git-dave/arch/x86/include/asm/kvm_host.h | 2 -
> linux-2.6.git-dave/arch/x86/kvm/mmu.c | 29 +++++++--------------
> linux-2.6.git-dave/arch/x86/kvm/mmu.h | 3 +-
> 3 files changed, 13 insertions(+), 21 deletions(-)
>
> diff -puN arch/x86/include/asm/kvm_host.h~replace-free-with-used arch/x86/include/asm/kvm_host.h
> --- linux-2.6.git/arch/x86/include/asm/kvm_host.h~replace-free-with-used 2010-06-09 15:14:29.000000000 -0700
> +++ linux-2.6.git-dave/arch/x86/include/asm/kvm_host.h 2010-06-09 15:14:29.000000000 -0700
> @@ -380,7 +380,7 @@ struct kvm_mem_aliases {
> struct kvm_arch {
> struct kvm_mem_aliases *aliases;
>
> - unsigned int n_free_mmu_pages;
> + unsigned int n_used_mmu_pages;
> unsigned int n_requested_mmu_pages;
> unsigned int n_max_mmu_pages;
> atomic_t invlpg_counter;
> diff -puN arch/x86/kvm/mmu.c~replace-free-with-used arch/x86/kvm/mmu.c
> --- linux-2.6.git/arch/x86/kvm/mmu.c~replace-free-with-used 2010-06-09 15:14:29.000000000 -0700
> +++ linux-2.6.git-dave/arch/x86/kvm/mmu.c 2010-06-09 15:14:29.000000000 -0700
> @@ -898,7 +898,7 @@ static void kvm_mmu_free_page(struct kvm
> __free_page(virt_to_page(sp->spt));
> __free_page(virt_to_page(sp->gfns));
> kfree(sp);
> - ++kvm->arch.n_free_mmu_pages;
> + --kvm->arch.n_used_mmu_pages;
> }
>
> static unsigned kvm_page_table_hashfn(gfn_t gfn)
> @@ -919,7 +919,7 @@ static struct kvm_mmu_page *kvm_mmu_allo
> bitmap_zero(sp->slot_bitmap, KVM_MEMORY_SLOTS + KVM_PRIVATE_MEM_SLOTS);
> sp->multimapped = 0;
> sp->parent_pte = parent_pte;
> - --vcpu->kvm->arch.n_free_mmu_pages;
> + ++vcpu->kvm->arch.n_used_mmu_pages;
> return sp;
> }
>
> @@ -1516,39 +1516,30 @@ static int kvm_mmu_zap_page(struct kvm *
>
> /*
> * Changing the number of mmu pages allocated to the vm
> - * Note: if kvm_nr_mmu_pages is too small, you will get dead lock
> + * Note: if goal_nr_mmu_pages is too small, you will get dead lock
> */
> -void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages)
> +void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int goal_nr_mmu_pages)
> {
> - int used_pages;
> -
> - used_pages = kvm->arch.n_max_mmu_pages - kvm_mmu_available_pages(kvm);
> - used_pages = max(0, used_pages);
> -
> /*
> * If we set the number of mmu pages to be smaller be than the
> * number of actived pages , we must to free some mmu pages before we
> * change the value
> */
>
> - if (used_pages > kvm_nr_mmu_pages) {
> - while (used_pages > kvm_nr_mmu_pages &&
> + if (kvm->arch.n_used_mmu_pages > goal_nr_mmu_pages) {
> + while (kvm->arch.n_used_mmu_pages > goal_nr_mmu_pages &&
> !list_empty(&kvm->arch.active_mmu_pages)) {
> struct kvm_mmu_page *page;
>
> page = container_of(kvm->arch.active_mmu_pages.prev,
> struct kvm_mmu_page, link);
> - used_pages -= kvm_mmu_zap_page(kvm, page);
> - used_pages--;
> + kvm->arch.n_used_mmu_pages -= kvm_mmu_zap_page(kvm, page);
> + kvm->arch.n_used_mmu_pages--;
kvm->arch.n_used_mmu_pages is deaccounted for in kvm_mmu_zap_page -> kvm_mmu_free_page
already.
--
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
