FAILED: patch "[PATCH] mm/memory_hotplug: don't access uninitialized memmaps in" failed to apply to 4.19-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 4.19-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.
mm/memory_hotplug.c +15 -57
@@ -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); }
Quoted 177 lines · On 27.10.19 14:43, [email protected] wrote:
The patch below does not apply to the 4.19-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
00d6c019b5bc175cee3770e0e659f2b5f4804ea5Mon 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.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@@ -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]
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.
--- mm/memory_hotplug.c | 8 ++++++++ 1 file changed, 8 insertions(+)
mm/memory_hotplug.c +8 -0
@@ -448,6 +448,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