Re: [PATCH] mm/memory_hotplug: Fix leftover use of struct page during hotplug

[Oscar Salvador <osalvador@techadventures.net>]

On Wed, May 23, 2018 at 04:16:08PM +0200, Michal Hocko wrote:
> On Wed 23-05-18 15:54:03, Oscar Salvador wrote:
> > On Thu, May 10, 2018 at 02:02:00PM +0200, Michal Hocko wrote:
> > > On Fri 04-05-18 17:50:51, Jonathan Cameron wrote:
> > > [...]
> > > > Exact path to the problem is as follows:
> > > > 
> > > > mm/memory_hotplug.c : add_memory_resource
> > > > The node is not online so we enter the
> > > > if (new_node) twice, on the second such block there is a call to
> > > > link_mem_sections which calls into
> > > > drivers/node.c: link_mem_sections which calls
> > > > drivers/node.c: register_mem_sect_under_node which calls
> > > > get_nid_for_pfn and keeps trying until the output of that matches
> > > > the expected node (passed all the way down from add_memory_resource)
> > > 
> > > I am sorry but I am still confused. Why don't we create sysfs files from
> > > __add_pages
> > >   __add_section
> > >     hotplug_memory_register
> > >       register_mem_sect_under_node
> > 
> > IIUC the problem is that at the point we are calling register_mem_sect_under_node(),
> > pages are not initialized yet.
> 
> Ahh, of course. I keep forgetting the latest hotplug optimizations that
> we do not initialize even nid for struct pages. Which is the whole point
> of this patch... Sigh.
> 
> I think the whole sysfs initialization needs to be refactored to be more
> sane. The way how we depend on things silently is just not maintainable.

I will try to work that out.
I also want to see if we can get rid of link_mem_sections() since it shares
almost all the code with walk_memory_range().
Maybe we can pass register_mem_sect_under_node() as a callback of walk_memory_range().

Oscar Salvador