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

From: Claudio Imbrenda <imbrenda@linux.ibm.com>
To: Nadav Amit <nadav.amit@gmail.com>
Cc: KVM <kvm@vger.kernel.org>,
	pbonzini@redhat.com, frankja@linux.ibm.com, david@redhat.com,
	thuth@redhat.com, cohuck@redhat.com, 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 10:15:10 +0100	[thread overview]
Message-ID: <20201208101510.4e3866dc@ibm-vm> (raw)
In-Reply-To: <11863F45-D4E5-4192-9541-EC4D26AC3634@gmail.com>

On Mon, 7 Dec 2020 17:10:13 -0800
Nadav Amit <nadav.amit@gmail.com> wrote:

> > On Dec 7, 2020, at 4:41 PM, 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.
> > 
> > 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.  
> 
> One more thing: I changed the previous allocator to zero any
> allocated page. Without it, I get strange failures when I do not run
> the tests on KVM, which are presumably caused by some intentional or
> unintentional hidden assumption of kvm-unit-tests that the memory is
> zeroed.
> 
> Can you restore this behavior? I can also send this one-line fix, but
> I do not want to overstep on your (hopeful) fix for the previous
> problem that I mentioned (AREA_ANY_NUMBER).

no. Some tests depend on the fact that the memory is being touched for
the first time.

if your test depends on memory being zeroed on allocation, maybe you
can zero the memory yourself in the test?

otherwise I can try adding a function to explicitly allocate a zeroed
page.

Claudio