Re: [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap

From: Jan Beulich <jbeulich@suse.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jeff Kubascik <jeff.kubascik@dornerworks.com>,
	Stewart Hildebrand <stewart.hildebrand@dornerworks.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap
Date: Wed, 5 Feb 2020 11:49:09 +0100	[thread overview]
Message-ID: <df6f5f23-ee65-f289-7dc4-5a42da931bf9@suse.com> (raw)
In-Reply-To: <59013bb7dbf9b79daa6dd280c4448418aa1fc56b.camel@infradead.org>

On 05.02.2020 11:24, David Woodhouse wrote:
> On Wed, 2020-02-05 at 11:02 +0100, Jan Beulich wrote:
>>> +    /* Pages from the boot allocator need to pass through init_heap_pages() */
>>> +    if ( unlikely(!pg->count_info) )
>>
>> ... while I think this check may be fine here, no similar one
>> can be used in free_domheap_pages(), yet pages getting handed
>> there isn't less likely than ones getting handed to
>> free_xenheap_pages() (if we already fear mismatch).
> 
> Do we care about that?
> 
> ICBW but I don't think I've seen a case where boot-allocated pages get
> handed to free_domheap_pages() later. I've only seen them handed to
> free_xenheap_pages(). These are pages which are mapped to Xen, not
> domheap pages.
> 
> You are already expected *not* to conflate free_xenheap_pages() and
> free_domheap_pages().

That's the case now, but with us wanting to get rid of the direct map,
to sole difference will be whether a mapping gets established.
There'll likely be no need for a PGC_xen_heap flag anymore, as we
don't need to tell apart the underlying pages (at least as far as the
allocator is concerned).

> I think it should be OK to declare that freeing boot-allocated pages
> with free_xenheap_pages() is permitted, but freeing them with
> free_domheap_pages() isn't.

I don't think this is going to be a good idea. free_xenheap_pages()
right now expects an address within the direct map to be passed.
alloc_boot_pages(), however, returns an MFN, which may get mapped
to arbitrary linear addresses (or none at all). There's quite a bit
more similarity to alloc_domheap_pages(), as that returns
struct page_info *, which in turn has a reliable translation
to/from MFN. Yet, as you say elsewhere, whether an MFN has an
entry in frame_table[] is entirely unclear, so permitting boot-
allocator pages to be freed via alloc_domheap_pages() nevertheless
still doesn't look any better an idea to me.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel