Re: [kvm-unit-tests PATCH v2 4/7] lib/alloc_page: complete rewrite of the page allocator

From: Nadav Amit <nadav.amit@gmail.com>
To: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: KVM <kvm@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	David Hildenbrand <david@redhat.com>,
	Thomas Huth <thuth@redhat.com>,
	cohuck@redhat.com, Laurent Vivier <lvivier@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v2 4/7] lib/alloc_page: complete rewrite of the page allocator
Date: Tue, 8 Dec 2020 01:16:59 -0800	[thread overview]
Message-ID: <17D911C4-2A11-4F37-810F-48F8E3226305@gmail.com> (raw)
In-Reply-To: <20201208101155.0e2de3c9@ibm-vm>

> On Dec 8, 2020, at 1:11 AM, Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> 
> On Mon, 7 Dec 2020 16:41:18 -0800
> Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>>> On Oct 2, 2020, at 8:44 AM, Claudio Imbrenda
>>> <imbrenda@linux.ibm.com> wrote:
>>> 
>>> This is a complete rewrite of the page allocator.  
>> 
>> This patch causes me crashes:
>> 
>>  lib/alloc_page.c:433: assert failed: !(areas_mask & BIT(n))
>> 
>> It appears that two areas are registered on AREA_LOW_NUMBER, as
>> setup_vm() can call (and calls on my system) page_alloc_init_area()
>> twice.
> 
> which system is that? page_alloc_init_area should not be called twice
> on the same area!

It is not the “same area”, it just uses the same id, see setup_vm():

        if (!page_alloc_initialized()) {
                base = PAGE_ALIGN(base) >> PAGE_SHIFT;
                top = top >> PAGE_SHIFT;

		// FIRST
                page_alloc_init_area(AREA_ANY_NUMBER, base, top);
                page_alloc_ops_enable();
        }

        find_highmem();
        phys_alloc_get_unused(&base, &top);
        page_root = setup_mmu(top);
        if (base != top) {
                base = PAGE_ALIGN(base) >> PAGE_SHIFT;
                top = top >> PAGE_SHIFT;

		// SECOND
                page_alloc_init_area(AREA_ANY_NUMBER, base, top);
        }

The problem occurs when I run KVM-unit-tests on VMware, but would presumably
also happen on bare-metal.

> 
>> setup_vm() uses AREA_ANY_NUMBER as the area number argument but
>> eventually this means, according to the code, that
>> __page_alloc_init_area() would use AREA_LOW_NUMBER.
>> 
>> I do not understand the rationale behind these areas well enough to
>> fix it.
> 
> I'll see what I can fix

Thanks,
Nadav