On Tue, Sep 03, 2019 at 03:15:58PM +0530, Anshuman Khandual wrote:
> @@ -770,6 +1022,28 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
> void vmemmap_free(unsigned long start, unsigned long end,
> struct vmem_altmap *altmap)
> {
> +#ifdef CONFIG_MEMORY_HOTPLUG
> + /*
> + * FIXME: We should have called remove_pagetable(start, end, true).
> + * vmemmap and vmalloc virtual range might share intermediate kernel
> + * page table entries. Removing vmemmap range page table pages here
> + * can potentially conflict with a concurrent vmalloc() allocation.
> + *
> + * This is primarily because vmalloc() does not take init_mm ptl for
> + * the entire page table walk and it's modification. Instead it just
> + * takes the lock while allocating and installing page table pages
> + * via [p4d|pud|pmd|pte]_alloc(). A concurrently vanishing page table
> + * entry via memory hot remove can cause vmalloc() kernel page table
> + * walk pointers to be invalid on the fly which can cause corruption
> + * or worst, a crash.
> + *
> + * So free_empty_tables() gets called where vmalloc and vmemmap range
> + * do not overlap at any intermediate level kernel page table entry.
> + */
> + unmap_hotplug_range(start, end, true);
> + if (!vmalloc_vmemmap_overlap)
> + free_empty_tables(start, end);
> +#endif
> }
> #endif /* CONFIG_SPARSEMEM_VMEMMAP */
I wonder whether we could simply ignore the vmemmap freeing altogether,
just leave it around and not unmap it. This way, we could call
unmap_kernel_range() for removing the linear map and we save some code.
For the linear map, I think we use just above 2MB of tables for 1GB of
memory mapped (worst case with 4KB pages we need 512 pte pages). For
vmemmap we'd use slightly above 2MB for a 64GB hotplugged memory. Do we
expect such memory to be re-plugged again in the same range? If we do,
then I shouldn't even bother with removing the vmmemmap.
I don't fully understand the use-case for memory hotremove, so any
additional info would be useful to make a decision here.
--
Catalin
