On 02/06/20 at 02:55pm, David Hildenbrand wrote: > On 06.02.20 14:50, Baoquan He wrote: > > On 02/06/20 at 02:28pm, David Hildenbrand wrote: > >> On 06.02.20 13:53, Wei Yang wrote: > >>> When we use SPARSEMEM instead of SPARSEMEM_VMEMMAP, pfn_to_page() > >>> doesn't work before sparse_init_one_section() is called. This leads to a > >>> crash when hotplug memory. > >>> > >>> We should use memmap as it did. > >>> > >>> Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug") > >>> Signed-off-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx> > >>> CC: Dan Williams <dan.j.williams@xxxxxxxxx> > >>> --- > >>> mm/sparse.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/mm/sparse.c b/mm/sparse.c > >>> index 5a8599041a2a..2efb24ff8f96 100644 > >>> --- a/mm/sparse.c > >>> +++ b/mm/sparse.c > >>> @@ -882,7 +882,7 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn, > >>> * Poison uninitialized struct pages in order to catch invalid flags > >>> * combinations. > >>> */ > >>> - page_init_poison(pfn_to_page(start_pfn), sizeof(struct page) * nr_pages); > >>> + page_init_poison(memmap, sizeof(struct page) * nr_pages); > >> > >> If you add sub-sections that don't fall onto the start of the section, > >> > >> pfn_to_page(start_pfn) != memmap > >> > >> and your patch would break that under SPARSEMEM_VMEMMAP if I am not wrong. > > > > It returns the pfn_to_page(pfn) from __populate_section_memmap() and > > assign to memmap in vmemmap case, how come it breaks anything. Correct > > me if I was wrong. > > I'm sorry, I can't follow :) Can you elaborate? > > Was your comment targeted at why the old code cannot be broken or why > this patch cannot be broken? Sorry for the confusion :-) the latter. I mean the returned memmap has been at the pfn_to_page(start_pfn) in SPARSEMEM_VMEMMAP case.