Hello Sudarshan,
On 09/10/2020 11:35 AM, Sudarshan Rajagopalan wrote:
> When section mappings are enabled, we allocate vmemmap pages from physically
> continuous memory of size PMD_SZIE using vmemmap_alloc_block_buf(). Section>
> mappings are good to reduce TLB pressure. But when system is highly fragmented
> and memory blocks are being hot-added at runtime, its possible that such
> physically continuous memory allocations can fail. Rather than failing the
Did you really see this happen on a system ?
> memory hot-add procedure, add a fallback option to allocate vmemmap pages from
> discontinuous pages using vmemmap_populate_basepages().
Which could lead to a mixed page size mapping in the VMEMMAP area.
Allocation failure in vmemmap_populate() should just cleanly fail
the memory hot add operation, which can then be retried. Why the
retry has to be offloaded to kernel ?
>
> Signed-off-by: Sudarshan Rajagopalan <sudar...@codeaurora.org>
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Cc: Will Deacon <w...@kernel.org>
> Cc: Anshuman Khandual <anshuman.khand...@arm.com>
> Cc: Mark Rutland <mark.rutl...@arm.com>
> Cc: Logan Gunthorpe <log...@deltatee.com>
> Cc: David Hildenbrand <da...@redhat.com>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: Steven Price <steven.pr...@arm.com>
> ---
> arch/arm64/mm/mmu.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 75df62f..a46c7d4 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1100,6 +1100,7 @@ int __meminit vmemmap_populate(unsigned long start,
> unsigned long end, int node,
> p4d_t *p4dp;
> pud_t *pudp;
> pmd_t *pmdp;
> + int ret = 0;
>
> do {
> next = pmd_addr_end(addr, end);
> @@ -1121,15 +1122,23 @@ int __meminit vmemmap_populate(unsigned long start,
> unsigned long end, int node,
> void *p = NULL;
>
> p = vmemmap_alloc_block_buf(PMD_SIZE, node, altmap);
> - if (!p)
> - return -ENOMEM;
> + if (!p) {
> +#ifdef CONFIG_MEMORY_HOTPLUG
> + vmemmap_free(start, end, altmap);
> +#endif
The mapping was never created in the first place, as the allocation
failed. vmemmap_free() here will free an unmapped area !
> + ret = -ENOMEM;
> + break;
> + }
>
> pmd_set_huge(pmdp, __pa(p), __pgprot(PROT_SECT_NORMAL));
> } else
> vmemmap_verify((pte_t *)pmdp, node, addr, next);
> } while (addr = next, addr != end);
>
> - return 0;
> + if (ret)
> + return vmemmap_populate_basepages(start, end, node, altmap);
> + else
> + return ret;
> }
> #endif /* !ARM64_SWAPPER_USES_SECTION_MAPS */
> void vmemmap_free(unsigned long start, unsigned long end,
>
