Re: [kvm-unit-tests RFC v1 2/5] lib/alloc_page: complete rewrite of the page allocator

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 14 Aug 2020 17:10:06 +0200
Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:

> This is a complete rewrite of the page allocator.
> 
> This will bring a few improvements:
> * no need to specify the size when freeing
> * allocate small areas with a large alignment without wasting memory
> * ability to initialize and use multiple memory areas (e.g. DMA)
> * more sanity checks
> 
> A few things have changed:
> * initialization cannot be done with free_pages like before,
>   page_alloc_init_area has to be used instead
> 
> Arch-specific changes:
> * arm and x86 have been adapted to put all the memory in just one big
>   area (or two, for x86_64 with highmem).
> * s390x instead creates one area below 2GiB and one above; the area
>   below 2GiB is used for SMP lowcore initialization.

I was just wondering why we did not run into that problem before for
the css control blocks, but these are statically allocated.

> 
> Details:
> Each memory area has metadata at the very beginning. The metadata is a
> byte array with one entry per usable page (so, excluding the metadata
> itself). Each entry indicates if the page is special (unused for now),
> if it is allocated, and the order of the block. Both free and allocated
> pages are part of larger blocks.
> 
> Some more fixed size metadata is present in a fixed-size static array.
> This metadata contains start and end page frame numbers, the pointer to
> the metadata array, and the array of freelists. The array of freelists
> has an entry for each possible order (indicated by the macro NLISTS,
> defined as BITS_PER_LONG - PAGE_SHIFT).
> 
> On allocation, if the free list for the needed size is empty, larger
> blocks are split. When a small allocation with a large alignment is
> requested, an appropriately large block is split, to guarantee the
> alignment.
> 
> When a block is freed, an attempt will be made to merge it into the
> neighbour, iterating the process as long as possible.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  lib/alloc_page.h |  64 ++++++-
>  lib/alloc_page.c | 451 ++++++++++++++++++++++++++++++++++++-----------
>  lib/arm/setup.c  |   2 +-
>  lib/s390x/sclp.c |  11 +-
>  lib/s390x/smp.c  |   2 +-
>  lib/vmalloc.c    |  13 +-
>  6 files changed, 427 insertions(+), 116 deletions(-)

(...)

> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 2860e9c..d954094 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -190,7 +190,7 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>  
>  	sigp_retry(cpu->addr, SIGP_INITIAL_CPU_RESET, 0, NULL);
>  
> -	lc = alloc_pages(1);
> +	lc = alloc_pages_area(_AREA(1), 1);

Add a comment describing what this _AREA(1) is? Or maybe add a wrapper
for allocating below 2G? We might want to use it in other places as
well.

>  	cpu->lowcore = lc;
>  	memset(lc, 0, PAGE_SIZE * 2);
>  	sigp_retry(cpu->addr, SIGP_SET_PREFIX, (unsigned long )lc, NULL);

(...)

I'll pass on reviewing the rest of this patch :)