From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [kvm-unit-tests PATCH v2 6/6] lib/x86/vm: enable malloc and friends Date: Thu, 3 Nov 2016 17:00:01 +0100 Message-ID: References: <1478119966-13252-1-git-send-email-drjones@redhat.com> <1478119966-13252-7-git-send-email-drjones@redhat.com> <20161103145856.weexgnucvpz3ul6q@kamzik.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, lvivier@redhat.com, thuth@redhat.com To: Andrew Jones Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:33461 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758611AbcKCQAG (ORCPT ); Thu, 3 Nov 2016 12:00:06 -0400 Received: by mail-wm0-f65.google.com with SMTP id u144so8719315wmu.0 for ; Thu, 03 Nov 2016 09:00:05 -0700 (PDT) In-Reply-To: <20161103145856.weexgnucvpz3ul6q@kamzik.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 03/11/2016 15:58, Andrew Jones wrote: > Actually I have it envisioned the other way around. phys_alloc should > only be for early allocations, before enabling the MMU (which is why > it's called "phys"_alloc). Yes, I agree. Putting your other suggestions together, you'd have for early allocation: phys // provide contiguous physical addresses | morecore // provide contiguous RAM | malloc // full API Later, you'd switch morecore to also take care of installing PTEs: page virt \ / morecore | malloc Now morecore talks to both page and virt. page (alloc_page) is initialized with whatever memory wasn't allocated by phys, and takes up its job. virt (alloc_vpages) allocates virtual address space. morecore is now taking not necessarily consecutive physical memory at page granularity, assigning consecutive virtual address to it, and if needed slicing pages into smaller allocations. morecore is not an external API, it's just malloc's pluggable interface. page/virt/malloc are all external APIs. phys could be, but nothing uses phys_alloc_aligned and phys_zalloc_aligned so we might as well zap it. Note that you can choose separately at each layer whether to implement freeing or not. I think we only need to have it in "page" and "malloc" (though in the latter it can be dummy, as it is now, since "malloc" is just a small veneer around morecore). >> addr = malloc_free; >> available = malloc_top - addr; >> if (available < size) { >> if (vfree_bottom == malloc_top + 1) { >> // can reuse top of previous segment >> vsize = PAGE_ALIGN(size - available); >> vaddr = alloc_vpages(vsize >> PAGE_SHIFT); >> assert(vaddr == malloc_top + 1); >> } else { >> // ignore free space at end of previous segment >> vsize = PAGE_ALIGN(size); >> addr = vaddr = >> alloc_vpages(vsize >> PAGE_SHIFT); >> } >> malloc_top = vaddr + vsize - 1; >> } >> malloc_free = addr + size; >> return addr; > > I agree lib/x86/vm.c needs to change it's virtual address allocation > scheme. It allocates down and never allows an address to be reused > (but maybe that's by design to avoid TLB flushes?) I didn't feel like > tackling that right now though. I'm not sure what the function you've > written should be used for. To me, it looks like it's an extension of > the virtual address management already available. Where do alloc_page() > and install_page() come in, like vmalloc has? The alloc_page()/install_page() loop would be before setting malloc_top. Paolo