Re: [RFC 1/2] arm64/mm: Fix pfn_valid() for ZONE_DEVICE based memory

From: David Hildenbrand
Date: Wed Jan 27 2021 - 04:33:54 EST


On 27.01.21 05:06, Anshuman Khandual wrote:


On 1/25/21 2:43 PM, David Hildenbrand wrote:
On 25.01.21 07:22, Anshuman Khandual wrote:

On 12/22/20 12:42 PM, Anshuman Khandual wrote:
pfn_valid() asserts that there is a memblock entry for a given pfn without
MEMBLOCK_NOMAP flag being set. The problem with ZONE_DEVICE based memory is
that they do not have memblock entries. Hence memblock_is_map_memory() will
invariably fail via memblock_search() for a ZONE_DEVICE based address. This
eventually fails pfn_valid() which is wrong. memblock_is_map_memory() needs
to be skipped for such memory ranges. As ZONE_DEVICE memory gets hotplugged
into the system via memremap_pages() called from a driver, their respective
memory sections will not have SECTION_IS_EARLY set.

Normal hotplug memory will never have MEMBLOCK_NOMAP set in their memblock
regions. Because the flag MEMBLOCK_NOMAP was specifically designed and set
for firmware reserved memory regions. memblock_is_map_memory() can just be
skipped as its always going to be positive and that will be an optimization
for the normal hotplug memory. Like ZONE_DEVIE based memory, all hotplugged
normal memory too will not have SECTION_IS_EARLY set for their sections.

Skipping memblock_is_map_memory() for all non early memory sections would
fix pfn_valid() problem for ZONE_DEVICE based memory and also improve its
performance for normal hotplug memory as well.

Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
Cc: Robin Murphy <robin.murphy@xxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Fixes: 73b20c84d42d ("arm64: mm: implement pte_devmap support")
Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>

Hello David/Mike,

Given that we would need to rework early sections, memblock semantics via a
new config i.e EARLY_SECTION_MEMMAP_HOLES and also some possible changes to
ARCH_KEEP_MEMBLOCK and HAVE_ARCH_PFN_VALID, wondering if these patches here
which fixes a problem (and improves performance) can be merged first. After
that, I could start working on the proposed rework. Could you please let me
know your thoughts on this. Thank you.

As I said, we might have to throw in an pfn_section_valid() check, to
catch not-section-aligned ZONE_DEVICE ranges (I assume this is possible
on arm64 as well, no?).

pfn_section_valid() should be called only for !early_section() i.e normal
hotplug and ZONE_DEVICE memory ? Because early boot memory should always
be section aligned.

Well, at least not on x86-64 you can have early sections intersect with ZONE_DEVICE memory.

E.g., have 64MB boot memory in a section. Later, we add ZONE_DEVICE memory which might cover the remaining 64MB. For pfn_valid() on x86-64, we always return "true" for such sections, because we always have the memmap for the whole early section allocated during boot. So, there it's "simple".

Now, arm64 seems to discard some parts of the vmemmap, so the remaining 64MB in such an early section might not have a memmap anymore? TBH, I don't know.

Most probably only performing the check for
!early_section() is sufficient on arm64, but I really can't tell as I don't know what we're actually discarding and if something as described for x86-64 is even possible on arm64.

We should really try to take the magic out of arm64 vmemmap handling.



Apart from that, I'm fine with a simple fix upfront, that can be more
easily backported if needed. (Q: do we? is this stable material?)


Right, an upfront fix here would help in backporting. AFAICS it should be
backported to the stable as pte_devmap and ZONE_DEVICE have been around
for some time now. Do you have a particular stable version which needs to
be tagged in the patch ?

I haven't looked yet TBH. I guess it is broken since ZONE_DEVICE was enabled on arm64?

--
Thanks,

David / dhildenb