On Fri 04-05-18 09:53:11, Jonathan Cameron wrote:
> The case of a new numa node got missed in avoiding using
> the node info from page_struct during hotplug. In this
> path we have a call to register_mem_sect_under_node (which allows
> us to specify it is hotplug so don't change the node),
> via link_mem_sections which unfortunately does not.
>
> Fix is to pass check_nid through link_mem_sections as well and
> disable it in the new numa node path.
>
> Note the bug only 'sometimes' manifests depending on what happens to
> be in the struct page structures - there are lots of them and it only
> needs to match one of them.
>
> Fixes: fc44f7f9231a ("mm/memory_hotplug: don't read nid from struct page during hotplug")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
I believe the proper fix is to refactor the sysfs code because it is
just ridiculous. But this should work good enough as the stop gap
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
> drivers/base/node.c | 5 +++--
> include/linux/node.h | 8 +++++---
> mm/memory_hotplug.c | 2 +-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 7a3a580821e0..a5e821d09656 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -490,7 +490,8 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
> return 0;
> }
>
> -int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
> +int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages,
> + bool check_nid)
> {
> unsigned long end_pfn = start_pfn + nr_pages;
> unsigned long pfn;
> @@ -514,7 +515,7 @@ int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
>
> mem_blk = find_memory_block_hinted(mem_sect, mem_blk);
>
> - ret = register_mem_sect_under_node(mem_blk, nid, true);
> + ret = register_mem_sect_under_node(mem_blk, nid, check_nid);
> if (!err)
> err = ret;
>
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 41f171861dcc..6d336e38d155 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -32,9 +32,11 @@ extern struct node *node_devices[];
> typedef void (*node_registration_func_t)(struct node *);
>
> #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
> -extern int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages);
> +extern int link_mem_sections(int nid, unsigned long start_pfn,
> + unsigned long nr_pages, bool check_nid);
> #else
> -static inline int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
> +static inline int link_mem_sections(int nid, unsigned long start_pfn,
> + unsigned long nr_pages, bool check_nid)
> {
> return 0;
> }
> @@ -57,7 +59,7 @@ static inline int register_one_node(int nid)
> if (error)
> return error;
> /* link memory sections under this node */
> - error = link_mem_sections(nid, pgdat->node_start_pfn, pgdat->node_spanned_pages);
> + error = link_mem_sections(nid, pgdat->node_start_pfn, pgdat->node_spanned_pages, true);
> }
>
> return error;
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index f74826cdceea..25982467800b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1158,7 +1158,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
> * nodes have to go through register_node.
> * TODO clean up this mess.
> */
> - ret = link_mem_sections(nid, start_pfn, nr_pages);
> + ret = link_mem_sections(nid, start_pfn, nr_pages, false);
> register_fail:
> /*
> * If sysfs file of new node can't create, cpu on the node
> --
> 2.16.2
>
--
Michal Hocko
SUSE Labs
