From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCHv2] KVM: x86: Fix memory leak in vmx.c Date: Thu, 18 Apr 2013 10:20:05 +0200 Message-ID: <516FACB5.2040000@redhat.com> References: <516F1A35.5090106@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit To: Andrew Honig , "kvm@vger.kernel.org" Return-path: Received: from mail-ee0-f49.google.com ([74.125.83.49]:45896 "EHLO mail-ee0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752652Ab3DRIUQ (ORCPT ); Thu, 18 Apr 2013 04:20:16 -0400 Received: by mail-ee0-f49.google.com with SMTP id b47so133726eek.22 for ; Thu, 18 Apr 2013 01:20:15 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Il 18/04/2013 01:03, Andrew Honig ha scritto: > I don't have a significant objection to freeing the memory in > kvm_arch_free_memslot, although I think it's a little harder to > understand. I like the idea of being symmetric (memory is allocated > by calling kvm_set_memory_region and freed using the same technique). > That way if someone changes from vm_mmap to something else it will be > obvious that they need to change both. > > Also, it looks like your patch is based on something several commits > behind HEAD on virt/kvm/kvm.git, Yeah, it was just whatever version I had checked out on the laptop. :) So that maintainers can look at both approaches and see what they prefer. Gleb, Marcelo, wdyt? Paolo > which significantly affect your > patch. In the HEAD version it assumes that user_alloc is always set > unless it's a private memslot. This appears to already have been the > case and allows a bunch of simplifications, some of which would apply > to your patch.