Skip to contentSkip to conversation

FAILED: patch "[PATCH] mm/memory_hotplug: don't access uninitialized memmaps in" failed to apply to 4.14-stable tree

PATCH New [email protected] 2 participants 3 messages Oct 27, 2019 – Nov 19, 2019 1 message not in the archive

Subsystems (3)
Stable branch Memory management Memory hot(un)plug
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 00d6c019b5bc175cee3770e0e659f2b5f4804ea5 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <[email protected]>
Date: Fri, 18 Oct 2019 20:19:33 -0700
Subject: [PATCH] mm/memory_hotplug: don't access uninitialized memmaps in
shrink_pgdat_span()

We might use the nid of memmaps that were never initialized. For example, if the memmap was poisoned, we will crash the kernel in pfn_to_nid() right now. Let's use the calculated boundaries of the separate zones instead. This now also avoids having to iterate over a whole bunch of subsections again, after shrinking one zone.

Before commit d0dc12e86b31 ("mm/memory_hotplug: optimize memory hotplug"), the memmap was initialized to 0 and the node was set to the right value. After that commit, the node might be garbage.

We'll have to fix shrink_zone_span() next.

Link http://lkml.kernel.org/r/[email protected] open here
Fixes f1dd2cd13c4b ("mm, memory_hotplug: do not associate hotadded memory to zones until online") [d0dc12e86b319]
Signed-off-by David Hildenbrand <[email protected]>
Reported-by Aneesh Kumar K.V <[email protected]>
Cc Oscar Salvador <[email protected]>
Cc David Hildenbrand <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Pavel Tatashin <[email protected]>
Cc Dan Williams <[email protected]>
Cc Wei Yang <[email protected]>
Cc Alexander Duyck <[email protected]>
Cc Alexander Potapenko <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Anshuman Khandual <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Damian Tometzki <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Gerald Schaefer <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Halil Pasic <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Ira Weiny <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Jun Yao <[email protected]>
Cc Logan Gunthorpe <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc "Matthew Wilcox (Oracle)" <[email protected]>
Cc Mel Gorman <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Pankaj Gupta <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Pavel Tatashin <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Qian Cai <[email protected]>
Cc Rich Felker <[email protected]>
Cc Robin Murphy <[email protected]>
Cc Steve Capper <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Tom Lendacky <[email protected]>
Cc Tony Luck <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Vlastimil Babka <[email protected]>
Cc Wei Yang <[email protected]>
Cc Will Deacon <[email protected]>
Cc Yoshinori Sato <[email protected]>
Cc Yu Zhao <[email protected]>
Cc <[email protected]> [4.13+]
Signed-off-by Andrew Morton <[email protected]>
Signed-off-by Linus Torvalds <[email protected]>
mm/memory_hotplug.c +15 -57
index b1be791f772d..df570e5c71cc 100644
@@ -436,67 +436,25 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
 	zone_span_writeunlock(zone);
 }
 
-static void shrink_pgdat_span(struct pglist_data *pgdat,
-			      unsigned long start_pfn, unsigned long end_pfn)
+static void update_pgdat_span(struct pglist_data *pgdat)
 {
-	unsigned long pgdat_start_pfn = pgdat->node_start_pfn;
-	unsigned long p = pgdat_end_pfn(pgdat); /* pgdat_end_pfn namespace clash */
-	unsigned long pgdat_end_pfn = p;
-	unsigned long pfn;
-	int nid = pgdat->node_id;
-
-	if (pgdat_start_pfn == start_pfn) {
-		/*
-		 * If the section is smallest section in the pgdat, it need
-		 * shrink pgdat->node_start_pfn and pgdat->node_spanned_pages.
-		 * In this case, we find second smallest valid mem_section
-		 * for shrinking zone.
-		 */
-		pfn = find_smallest_section_pfn(nid, NULL, end_pfn,
-						pgdat_end_pfn);
-		if (pfn) {
-			pgdat->node_start_pfn = pfn;
-			pgdat->node_spanned_pages = pgdat_end_pfn - pfn;
-		}
-	} else if (pgdat_end_pfn == end_pfn) {
-		/*
-		 * If the section is biggest section in the pgdat, it need
-		 * shrink pgdat->node_spanned_pages.
-		 * In this case, we find second biggest valid mem_section for
-		 * shrinking zone.
-		 */
-		pfn = find_biggest_section_pfn(nid, NULL, pgdat_start_pfn,
-					       start_pfn);
-		if (pfn)
-			pgdat->node_spanned_pages = pfn - pgdat_start_pfn + 1;
-	}
-
-	/*
-	 * If the section is not biggest or smallest mem_section in the pgdat,
-	 * it only creates a hole in the pgdat. So in this case, we need not
-	 * change the pgdat.
-	 * But perhaps, the pgdat has only hole data. Thus it check the pgdat
-	 * has only hole or not.
-	 */
-	pfn = pgdat_start_pfn;
-	for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SUBSECTION) {
-		if (unlikely(!pfn_valid(pfn)))
-			continue;
-
-		if (pfn_to_nid(pfn) != nid)
-			continue;
+	unsigned long node_start_pfn = 0, node_end_pfn = 0;
+	struct zone *zone;
 
-		/* Skip range to be removed */
-		if (pfn >= start_pfn && pfn < end_pfn)
-			continue;
+	for (zone = pgdat->node_zones;
+	     zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
+		unsigned long zone_end_pfn = zone->zone_start_pfn +
+					     zone->spanned_pages;
 
-		/* If we find valid section, we have nothing to do */
-		return;
+		/* No need to lock the zones, they can't change. */
+		if (zone_end_pfn > node_end_pfn)
+			node_end_pfn = zone_end_pfn;
+		if (zone->zone_start_pfn < node_start_pfn)
+			node_start_pfn = zone->zone_start_pfn;
 	}
 
-	/* The pgdat has no valid section */
-	pgdat->node_start_pfn = 0;
-	pgdat->node_spanned_pages = 0;
+	pgdat->node_start_pfn = node_start_pfn;
+	pgdat->node_spanned_pages = node_end_pfn - node_start_pfn;
 }
 
 static void __remove_zone(struct zone *zone, unsigned long start_pfn,

@@ -507,7 +465,7 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn,
 
 	pgdat_resize_lock(zone->zone_pgdat, &flags);
 	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
-	shrink_pgdat_span(pgdat, start_pfn, start_pfn + nr_pages);
+	update_pgdat_span(pgdat);
 	pgdat_resize_unlock(zone->zone_pgdat, &flags);
 }
#1 lore raw
Quoted 177 lines · On 27.10.19 14:43, [email protected] wrote:

The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 00d6c019b5bc175cee3770e0e659f2b5f4804ea5 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <[email protected]>
Date: Fri, 18 Oct 2019 20:19:33 -0700
Subject: [PATCH] mm/memory_hotplug: don't access uninitialized memmaps in
shrink_pgdat_span()

We might use the nid of memmaps that were never initialized. For example, if the memmap was poisoned, we will crash the kernel in pfn_to_nid() right now. Let's use the calculated boundaries of the separate zones instead. This now also avoids having to iterate over a whole bunch of subsections again, after shrinking one zone.

Before commit d0dc12e86b31 ("mm/memory_hotplug: optimize memory hotplug"), the memmap was initialized to 0 and the node was set to the right value. After that commit, the node might be garbage.

We'll have to fix shrink_zone_span() next.

Link http://lkml.kernel.org/r/[email protected] open here
Fixes f1dd2cd13c4b ("mm, memory_hotplug: do not associate hotadded memory to zones until online") [d0dc12e86b319]
Signed-off-by David Hildenbrand <[email protected]>
Reported-by Aneesh Kumar K.V <[email protected]>
Cc Oscar Salvador <[email protected]>
Cc David Hildenbrand <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Pavel Tatashin <[email protected]>
Cc Dan Williams <[email protected]>
Cc Wei Yang <[email protected]>
Cc Alexander Duyck <[email protected]>
Cc Alexander Potapenko <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Anshuman Khandual <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Damian Tometzki <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Gerald Schaefer <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Halil Pasic <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Ira Weiny <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Jun Yao <[email protected]>
Cc Logan Gunthorpe <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc "Matthew Wilcox (Oracle)" <[email protected]>
Cc Mel Gorman <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Mike Rapoport <[email protected]>
Cc Pankaj Gupta <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Pavel Tatashin <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Qian Cai <[email protected]>
Cc Rich Felker <[email protected]>
Cc Robin Murphy <[email protected]>
Cc Steve Capper <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Tom Lendacky <[email protected]>
Cc Tony Luck <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Vlastimil Babka <[email protected]>
Cc Wei Yang <[email protected]>
Cc Will Deacon <[email protected]>
Cc Yoshinori Sato <[email protected]>
Cc Yu Zhao <[email protected]>
Cc <[email protected]> [4.13+]
Signed-off-by Andrew Morton <[email protected]>
Signed-off-by Linus Torvalds <[email protected]>
mm/memory_hotplug.c +14 -56
index b1be791f772d..df570e5c71cc 100644
@@ -436,67 +436,25 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
  	zone_span_writeunlock(zone);
  }
 
-static void shrink_pgdat_span(struct pglist_data *pgdat,
-			      unsigned long start_pfn, unsigned long end_pfn)
+static void update_pgdat_span(struct pglist_data *pgdat)
  {
-	unsigned long pgdat_start_pfn = pgdat->node_start_pfn;
-	unsigned long p = pgdat_end_pfn(pgdat); /* pgdat_end_pfn namespace clash */
-	unsigned long pgdat_end_pfn = p;
-	unsigned long pfn;
-	int nid = pgdat->node_id;
-
-	if (pgdat_start_pfn == start_pfn) {
-		/*
-		 * If the section is smallest section in the pgdat, it need
-		 * shrink pgdat->node_start_pfn and pgdat->node_spanned_pages.
-		 * In this case, we find second smallest valid mem_section
-		 * for shrinking zone.
-		 */
-		pfn = find_smallest_section_pfn(nid, NULL, end_pfn,
-						pgdat_end_pfn);
-		if (pfn) {
-			pgdat->node_start_pfn = pfn;
-			pgdat->node_spanned_pages = pgdat_end_pfn - pfn;
-		}
-	} else if (pgdat_end_pfn == end_pfn) {
-		/*
-		 * If the section is biggest section in the pgdat, it need
-		 * shrink pgdat->node_spanned_pages.
-		 * In this case, we find second biggest valid mem_section for
-		 * shrinking zone.
-		 */
-		pfn = find_biggest_section_pfn(nid, NULL, pgdat_start_pfn,
-					       start_pfn);
-		if (pfn)
-			pgdat->node_spanned_pages = pfn - pgdat_start_pfn + 1;
-	}
-
-	/*
-	 * If the section is not biggest or smallest mem_section in the pgdat,
-	 * it only creates a hole in the pgdat. So in this case, we need not
-	 * change the pgdat.
-	 * But perhaps, the pgdat has only hole data. Thus it check the pgdat
-	 * has only hole or not.
-	 */
-	pfn = pgdat_start_pfn;
-	for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SUBSECTION) {
-		if (unlikely(!pfn_valid(pfn)))
-			continue;
-
-		if (pfn_to_nid(pfn) != nid)
-			continue;
+	unsigned long node_start_pfn = 0, node_end_pfn = 0;
+	struct zone *zone;
 
-		/* Skip range to be removed */
-		if (pfn >= start_pfn && pfn < end_pfn)
-			continue;
+	for (zone = pgdat->node_zones;
+	     zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
+		unsigned long zone_end_pfn = zone->zone_start_pfn +
+					     zone->spanned_pages;
 
-		/* If we find valid section, we have nothing to do */
-		return;
+		/* No need to lock the zones, they can't change. */
+		if (zone_end_pfn > node_end_pfn)
+			node_end_pfn = zone_end_pfn;
+		if (zone->zone_start_pfn < node_start_pfn)
+			node_start_pfn = zone->zone_start_pfn;
  	}
 
-	/* The pgdat has no valid section */
-	pgdat->node_start_pfn = 0;
-	pgdat->node_spanned_pages = 0;
+	pgdat->node_start_pfn = node_start_pfn;
+	pgdat->node_spanned_pages = node_end_pfn - node_start_pfn;
  }
 
  static void __remove_zone(struct zone *zone, unsigned long start_pfn,

mm/memory_hotplug.c static void __remove_zone(struct zone *zone, unsigned long start_pfn, · line 471

Quoted 8 lines
mm/memory_hotplug.c +1 -1
@@ -507,7 +465,7 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn,
 
  	pgdat_resize_lock(zone->zone_pgdat, &flags);
  	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
-	shrink_pgdat_span(pgdat, start_pfn, start_pfn + nr_pages);
+	update_pgdat_span(pgdat);
  	pgdat_resize_unlock(zone->zone_pgdat, &flags);
  }

We'll come back to this once we sorted out

https://lkml.org/lkml/2019/10/27/738

Signature · 3 lines
Thanks,

David / dhildenb

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

PATCH 2/2 mm/memory_hotplug: fix updating the node span

commit 656d571193262a11c2daa4012e53e4d645bbce56 upstream.

We recently started updating the node span based on the zone span to avoid touching uninitialized memmaps.

Currently, we will always detect the node span to start at 0, meaning a node can easily span too many pages. pgdat_is_empty() will still work correctly if all zones span no pages. We should skip over all zones without spanned pages and properly handle the first detected zone that spans pages.

Unfortunately, in contrast to the zone span (/proc/zoneinfo), the node span cannot easily be inspected and tested. The node span gives no real guarantees when an architecture supports memory hotplug, meaning it can easily contain holes or span pages of different nodes.

The node span is not really used after init on architectures that support memory hotplug.

E.g., we use it in mm/memory_hotplug.c:try_offline_node() and in mm/kmemleak.c:kmemleak_scan(). These users seem to be fine.

Link http://lkml.kernel.org/r/[email protected] open here
Fixes 00d6c019b5bc ("mm/memory_hotplug: don't access uninitialized memmaps in shrink_pgdat_span()")
Signed-off-by David Hildenbrand <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Oscar Salvador <[email protected]>
Cc Stephen Rothwell <[email protected]>
Cc Dan Williams <[email protected]>
Cc Pavel Tatashin <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
Signed-off-by Linus Torvalds <[email protected]>
---
 mm/memory_hotplug.c | 8 ++++++++
 1 file changed, 8 insertions(+)
mm/memory_hotplug.c +8 -0
index 9cd25b19e111..d4affa9982ca 100644
@@ -470,6 +470,14 @@ static void update_pgdat_span(struct pglist_data *pgdat)
 					     zone->spanned_pages;
 
 		/* No need to lock the zones, they can't change. */
+		if (!zone->spanned_pages)
+			continue;
+		if (!node_end_pfn) {
+			node_start_pfn = zone->zone_start_pfn;
+			node_end_pfn = zone_end_pfn;
+			continue;
+		}
+
 		if (zone_end_pfn > node_end_pfn)
 			node_end_pfn = zone_end_pfn;
 		if (zone->zone_start_pfn < node_start_pfn)
Signature · 1 line
2.21.0