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, 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:23:59 -0800	[thread overview]
Message-ID: <A32A8A40-5581-4A3D-9DC8-4591C3A034C7@gmail.com> (raw)
In-Reply-To: <20201208101510.4e3866dc@ibm-vm>

> On Dec 8, 2020, at 1:15 AM, Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> 
> 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.

To be fair, I do not know which non-zeroed memory causes the failure, and
debugging these kind of failures is hard and sometimes non-deterministic. For
instance, the failure I got this time was:

	Test suite: vmenter
	VM-Fail on vmlaunch: error number is 7. See Intel 30.4.

And other VM-entry failures, which are not easy to debug, especially on
bare-metal.

Note that the failing test is not new, and unfortunately these kind of
errors (wrong assumption that memory is zeroed) are not rare, since KVM
indeed zeroes the memory (unlike other hypervisors and bare-metal).

The previous allocator had the behavior of zeroing the memory to avoid such
problems. I would argue that zeroing should be the default behavior, and if
someone wants to have the memory “untouched” for a specific test (which
one?) he should use an alternative function for this matter.