On Tue, Jun 23, 2020 at 09:55:43AM +0200, David Hildenbrand wrote: >On 23.06.20 09:39, David Hildenbrand wrote: >>> Hmm.. I thought this is the behavior for early section, while it looks current >>> code doesn't work like this: >>> >>> if (section_is_early && memmap) >>> free_map_bootmem(memmap); >>> else >>> depopulate_section_memmap(pfn, nr_pages, altmap); >>> >>> section_is_early is always "true" for early section, while memmap is not-NULL >>> only when sub-section map is empty. >>> >>> If my understanding is correct, when we remove a sub-section in early section, >>> the code would call depopulate_section_memmap(), which in turn free related >>> memmap. By removing the memmap, the return value from pfn_to_online_page() is >>> not a valid one. >> >> I think you're right, and pfn_valid() would also return true, as it is >> an early section. This looks broken. >> >>> >>> Maybe we want to write the code like this: >>> >>> if (section_is_early) >>> if (memmap) >>> free_map_bootmem(memmap); >>> else >>> depopulate_section_memmap(pfn, nr_pages, altmap); >>> >> >> I guess that should be the way to go >> >> @Dan, I think what Wei proposes here is correct, right? Or how does it >> work in the VMEMMAP case with early sections? >> > >Especially, if you would re-hot-add, section_activate() would assume >there is a memmap, it must not be removed. > You are right here. I didn't notice it. >@Wei, can you send a patch? > Sure, let me prepare for it. >-- >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me