+ mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug.patch added to -mm tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The patch titled
     Subject: mm/memory_hotplug: fix leftover use of struct page during hotplug
has been added to the -mm tree.  Its filename is
     mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug.patch

This patch should soon appear at
    http://ozlabs.org/~akpm/mmots/broken-out/mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug.patch
and later at
    http://ozlabs.org/~akpm/mmotm/broken-out/mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Subject: mm/memory_hotplug: fix leftover use of struct page during hotplug

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.

The result of the bug is that (with a new memory only node) we never
successfully call register_mem_sect_under_node so don't get the memory
associated with the node in sysfs and meminfo for the node doesn't report
it.

It came up whilst testing some arm64 hotplug patches, but appears to be
universal.  Whilst I'm triggering it by removing then reinserting memory
to a node with no other elements (thus making the node disappear then
appear again), it appears it would happen on hotplugging memory where
there was none before and it doesn't seem to be related the arm64 patches.
These patches call __add_pages (where most of the issue was fixed by
Pavel's patch).  If there is a node at the time of the __add_pages call
then all is well as it calls register_mem_sect_under_node from there with
check_nid set to false.  Without a node that function returns having not
done the sysfs related stuff as there is no node to use.  This is expected
but it is the resulting path that fails...

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)

It is effectively the same fix as the one referred to in the fixes tag
just in the code path for a new node where the comments point out we have
to rerun the link creation because it will have failed in
register_new_memory (as there was no node at the time).  (actually that
comment is wrong now as we don't have register_new_memory any more it got
renamed to hotplug_memory_register in Pavel's patch).

Link: http://lkml.kernel.org/r/20180504085311.1240-1-Jonathan.Cameron@xxxxxxxxxx
Fixes: fc44f7f9231a ("mm/memory_hotplug: don't read nid from struct page during hotplug")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Reviewed-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---

 drivers/base/node.c  |    5 +++--
 include/linux/node.h |    8 +++++---
 mm/memory_hotplug.c  |    2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff -puN drivers/base/node.c~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug drivers/base/node.c
--- a/drivers/base/node.c~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug
+++ a/drivers/base/node.c
@@ -490,7 +490,8 @@ int unregister_mem_sect_under_nodes(stru
 	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
 
 		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 -puN include/linux/node.h~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug include/linux/node.h
--- a/include/linux/node.h~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug
+++ a/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
 		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 -puN mm/memory_hotplug.c~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug mm/memory_hotplug.c
--- a/mm/memory_hotplug.c~mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug
+++ a/mm/memory_hotplug.c
@@ -1158,7 +1158,7 @@ int __ref add_memory_resource(int nid, s
 		 * 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
_

Patches currently in -mm which might be from Jonathan.Cameron@xxxxxxxxxx are

mm-memory_hotplug-fix-leftover-use-of-struct-page-during-hotplug.patch

--
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux