On Thu, Nov 03, 2016 at 03:12:15PM +0100, Paolo Bonzini wrote:
>
>
> On 02/11/2016 21:52, Andrew Jones wrote:
> > We've had malloc, calloc, free, and memalign available for quite a
> > while, and arm and powerpc make use of them. x86 hasn't yet, which
> > is fine, but could lead to problems if common lib code wants to make
> > use of them. While arm and powerpc currently use early_alloc_ops,
> > built on phys_alloc, x86 does not initialize phys_alloc and has its
> > own memory management, vmalloc. This patch enables vmalloc to be
> > used as the underlying allocator, but there are two drawbacks:
> >
> > 1) Every allocation will allocate at least one page.
> > 2) It can only be used with the MMU enabled.
> >
> > Drawback (1) is probably fine for kvm-unit-tests. Drawback (2) may
> > not be, as common lib code may get invoked at any time by any unit
> > test, including ones that don't have the MMU enabled. However, as
> > we only switch alloc_ops to the vmalloc based implementations in
> > setup_vm, where the MMU is enabled, then they'll be safe to use for
> > any unit test that invokes setup_vm first. If the unit test does
> > not invoke setup_vm first then alloc_ops will use the default
> > implementations, the ones based on phys_alloc, and will result
> > in asserts firing if phys_alloc_init has not be called first.
> >
> > While this patch may not enable anything right now, I think it
> > makes sense to enable these alloc ops before they're needed, because
> > then everything will most likely just work when a future common lib
> > function that uses malloc is introduced. If that common function
> > results in a unit test hitting an assert, then the test writer will
> > just need to decide if they want to use phys_alloc or vmalloc and
> > call the appropriate init function first.
>
> So it seems to me that phys_alloc should be implemented _on top_ of
> virtual memory.
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). The only reason phys_alloc is still being
used for ARM after enabling the MMU is because, thanks to using an
identity map, it doesn't really matter. The only drawback of using it
is that I didn't want it to be too complicated, so I didn't implement
a way to free memory. If at some point we want a region of memory that
isn't identity mapped, and/or can also be freed, then we'll need a new
allocator. That new allocator would get its memory from a phys_alloc
allocation though. For example, we can now use the alloc/free_page
allocator like this
phys_alloc_init(phys_base, total_size);
heap_phys_base = phys_alloc_aligned(heap_size, PAGE_SIZE);
mmu_enable(pgtable);
mmu_set_range_ptes(pgtable, heap_virt_base, heap_phys_base,
heap_phys_base + heap_size, prot);
heap_init(heap_virt_base, heap_size);
alloc_ops = &heap_based_ops;
> Basically, the alloc_ops should not be
> malloc/calloc/free/memalign. The current
> early_malloc/early_calloc/early_free/early_memalign should just be
> renamed to malloc/calloc/free/memalign.
>
> Instead, you only need an op which is
>
> void *morecore(unsigned long size);
It makes sense to try to reduce the number of ops we have. Certainly
malloc/calloc/memalign could all be based on a single op (they're
already all based on phys_alloc_aligned_safe). We still need one more
though for free(). I can fix that up, i.e. change alloc_ops to only
an alloc and a free. early_alloc will be phys_alloc_aligned_safe and
early_free will still just be {}.
>
> The default implementation is basically "base += size; return base -
> size;". When you turn on virtual memory (hopefully, that's before any
> allocation!) map_core is changed to point to a function that calls
> install_page to fetch memory if needed.
Yes, this is the idea we have today; before MMU, alloc_ops point to
phys_alloc (the simple default imp. you describe). After MMU, we can
remap alloc_ops to point to a new implementation (as I show above).
>
> Assuming lib/x86/vm.c is changed to allocate upwards (not downwards),
> something like this would do:
>
> 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?
Thanks,
drew
--
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
