On Tue, Jun 23, 2020 at 05:18:28PM +0200, Michal Hocko wrote: >On Tue 23-06-20 17:42:58, Wei Yang wrote: >> For early sections, we assumes its memmap will never be partially >> removed. But current behavior breaks this. >> >> Let's correct it. >> >> Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug") >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx> > >Can a user trigger this or is this a theoretical bug? Let me rewrite the changelog a little. Look forward any comments. For early sections, its memmap is handled specially even sub-section is enabled. The memmap could only be populated as a whole. Quoted from the comment of section_activate(): * The early init code does not consider partially populated * initial sections, it simply assumes that memory will never be * referenced. If we hot-add memory into such a section then we * do not need to populate the memmap and can simply reuse what * is already there. While current section_deactivate() breaks this rule. When hot-remove a sub-section, section_deactivate() would depopulate its memmap. The consequence is if we hot-add this subsection again, its memmap never get proper populated. > >> --- >> mm/sparse.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/mm/sparse.c b/mm/sparse.c >> index b2b9a3e34696..1a0069f492f5 100644 >> --- a/mm/sparse.c >> +++ b/mm/sparse.c >> @@ -825,10 +825,10 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages, >> ms->section_mem_map &= ~SECTION_HAS_MEM_MAP; >> } >> >> - if (section_is_early && memmap) >> - free_map_bootmem(memmap); >> - else >> + if (!section_is_early) > >This begs a comment. > >> depopulate_section_memmap(pfn, nr_pages, altmap); >> + else if (memmap) >> + free_map_bootmem(memmap); >> >> if (empty) >> ms->section_mem_map = (unsigned long)NULL; >> -- >> 2.20.1 (Apple Git-117) >> > >-- >Michal Hocko >SUSE Labs -- Wei Yang Help you, Help me