From patchwork Wed Apr 17 21:55:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 2456661 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 8297F3FC64 for ; Wed, 17 Apr 2013 21:55:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966927Ab3DQVzO (ORCPT ); Wed, 17 Apr 2013 17:55:14 -0400 Received: from mail-ee0-f50.google.com ([74.125.83.50]:47674 "EHLO mail-ee0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966683Ab3DQVzN (ORCPT ); Wed, 17 Apr 2013 17:55:13 -0400 Received: by mail-ee0-f50.google.com with SMTP id e53so998372eek.37 for ; Wed, 17 Apr 2013 14:55:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:x-enigmail-version:content-type :content-transfer-encoding; bh=v47TKE593goaoaf9T4nlLw5YYCGhL9gHKmoejUSFneU=; b=n0c0KHAxaLQzNngnY8HkfPsi2mohy3YGR95W0m25xwsMs4bDvleEXNfLhziy+djCgC bA4BNDjHdtuj8JalCDnfRoU6DCVR2851dpnPmpeTeIcEb6eYM+giKqrN9QkzTCHHNXj3 pnUBIrnQoKo4yLOPUkNi/s4dtYJo1pyNb0N6M+QpOopiHKQCBxapW1L6MX/O6sZ5LDcD 6hf2Y309RqJVECBsZcgipd4b/PTHbHu+i5WY7oxoRWTMgmW1XI4FILQstr+c6k53PV4F 77PZOH6nnEBPVB8bExuvAQawUqGGUoPz8blPKIeCBptyBhPPt2IvGA+hE13BGqQTfUry CxkQ== X-Received: by 10.15.83.73 with SMTP id b49mr22571891eez.25.1366235711980; Wed, 17 Apr 2013 14:55:11 -0700 (PDT) Received: from yakj.usersys.redhat.com (93-34-176-20.ip50.fastwebnet.it. [93.34.176.20]) by mx.google.com with ESMTPS id bk42sm11667920eeb.3.2013.04.17.14.55.09 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 17 Apr 2013 14:55:10 -0700 (PDT) Message-ID: <516F1A35.5090106@redhat.com> Date: Wed, 17 Apr 2013 23:55:01 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Andrew Honig CC: kvm@vger.kernel.org Subject: Re: [PATCHv2] KVM: x86: Fix memory leak in vmx.c References: In-Reply-To: X-Enigmail-Version: 1.5.1 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Il 17/04/2013 19:54, Andrew Honig ha scritto: > > If userspace creates and destroys multiple VMs within the same process > we leak 20k of memory in the userspace process context per VM. This > patch frees the memory in kvm_arch_destroy_vm. If the process exits > without closing the VM file descriptor or the file descriptor has been > shared with another process then we don't need to free the memory. > > Signed-off-by: Andrew Honig > --- > arch/x86/kvm/x86.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) What about something like this (uncompiled/untested) --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index 8b3a9c0..6706134 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -1563,7 +1563,8 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) return VM_FAULT_SIGBUS; } -void kvm_arch_free_memslot(struct kvm_memory_slot *free, +void kvm_arch_free_memslot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont) { } diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 4d213b8..a654580 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -299,7 +299,8 @@ long kvm_arch_dev_ioctl(struct file *filp, return -EINVAL; } -void kvm_arch_free_memslot(struct kvm_memory_slot *free, +void kvm_arch_free_memslot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont) { if (!dont || free->arch.rmap != dont->arch.rmap) { diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index ecced9d..e2159c1 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -912,7 +912,8 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) return VM_FAULT_SIGBUS; } -void kvm_arch_free_memslot(struct kvm_memory_slot *free, +void kvm_arch_free_memslot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont) { } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 224a7e7..f9fa0d1 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6357,11 +6367,26 @@ void kvm_arch_destroy_vm(struct kvm *kvm) kfree(rcu_dereference_check(kvm->arch.apic_map, 1)); } -void kvm_arch_free_memslot(struct kvm_memory_slot *free, +void kvm_arch_free_memslot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont) { int i; + if (current->mm == kvm->mm && free->user_alloc) { + if (!dont || !dont->user_alloc || + free->userspace_addr != dont->userspace_addr) { + int ret; + + ret = vm_munmap(free->userspace_addr, + free->npages * PAGE_SIZE); + if (ret < 0) + printk(KERN_WARNING + "kvm_vm_ioctl_set_memory_region: " + "failed to munmap memory\n"); + } + } + for (i = 0; i < KVM_NR_PAGE_SIZES; ++i) { if (!dont || free->arch.rmap[i] != dont->arch.rmap[i]) { kvm_kvfree(free->arch.rmap[i]); @@ -6453,7 +6478,7 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, *x86 needs to handle !user_alloc case. */ if (!user_alloc) { - if (npages && !old.npages) { + if (npages != old.npages) { unsigned long userspace_addr; userspace_addr = vm_mmap(NULL, 0, @@ -6466,7 +6491,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, return PTR_ERR((void *)userspace_addr); memslot->userspace_addr = userspace_addr; - } + } else + memslot->userspace_addr = old.userspace_addr; } @@ -6481,17 +6507,6 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, int nr_mmu_pages = 0, npages = mem->memory_size >> PAGE_SHIFT; - if (!user_alloc && !old.user_alloc && old.npages && !npages) { - int ret; - - ret = vm_munmap(old.userspace_addr, - old.npages * PAGE_SIZE); - if (ret < 0) - printk(KERN_WARNING - "kvm_vm_ioctl_set_memory_region: " - "failed to munmap memory\n"); - } - if (!kvm->arch.n_requested_mmu_pages) nr_mmu_pages = kvm_mmu_calculate_mmu_pages(kvm); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index ecc5543..8f2a863 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -436,7 +436,8 @@ int kvm_set_memory_region(struct kvm *kvm, int __kvm_set_memory_region(struct kvm *kvm, struct kvm_userspace_memory_region *mem, int user_alloc); -void kvm_arch_free_memslot(struct kvm_memory_slot *free, +void kvm_arch_free_memslot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont); int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages); int kvm_arch_prepare_memory_region(struct kvm *kvm, diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index be70035..ea63b9c 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -546,13 +546,14 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot) /* * Free any memory in @free but not in @dont. */ -static void kvm_free_physmem_slot(struct kvm_memory_slot *free, +static void kvm_free_physmem_slot(struct kvm *kvm, + struct kvm_memory_slot *free, struct kvm_memory_slot *dont) { if (!dont || free->dirty_bitmap != dont->dirty_bitmap) kvm_destroy_dirty_bitmap(free); - kvm_arch_free_memslot(free, dont); + kvm_arch_free_memslot(kvm, free, dont); free->npages = 0; } @@ -563,7 +564,7 @@ void kvm_free_physmem(struct kvm *kvm) struct kvm_memory_slot *memslot; kvm_for_each_memslot(memslot, slots) - kvm_free_physmem_slot(memslot, NULL); + kvm_free_physmem_slot(kvm, memslot, NULL); kfree(kvm->memslots); } @@ -851,13 +852,13 @@ int __kvm_set_memory_region(struct kvm *kvm, kvm_arch_commit_memory_region(kvm, mem, old, user_alloc); - kvm_free_physmem_slot(&old, &new); + kvm_free_physmem_slot(kvm, &old, &new); kfree(old_memslots); return 0; out_free: - kvm_free_physmem_slot(&new, &old); + kvm_free_physmem_slot(kvm, &new, &old); out: return r;