diff mbox series

[05/67] mm/page_alloc: fix and rework pfn handling in memmap_init_zone()

Message ID 20200204013359.YjZ6CpL86%akpm@linux-foundation.org (mailing list archive)
State New, archived
Headers show
Series [01/67] ocfs2: fix oops when writing cloned file | expand

Commit Message

Andrew Morton Feb. 4, 2020, 1:33 a.m. UTC
From: David Hildenbrand <david@redhat.com>
Subject: mm/page_alloc: fix and rework pfn handling in memmap_init_zone()

Let's update the pfn manually whenever we continue the loop.  This makes
the code easier to read but also less error prone (and we can directly fix
one issue).

When overlap_memmap_init() returns true, pfn is updated to
"memblock_region_memory_end_pfn(r)".  So it already points at the *next*
pfn to process.  Incrementing the pfn another time is wrong, we might
leave one uninitialized.  I spotted this by inspecting the code, so I have
no idea if this is relevant in practise (with kernelcore=3Dmirror).

Link: http://lkml.kernel.org/r/20200113144035.10848-2-david@redhat.com
Fixes: a9a9e77fbf27 ("mm: move mirrored memory specific code outside of m=
emmap_init_zone")
Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Mel Gorman <mgorman@suse.de>
Cc: "Jin, Zhi" <zhi.jin@intel.com>
Cc: Michal Hocko <mhocko@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/page_alloc.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Linus Torvalds Feb. 4, 2020, 2:45 a.m. UTC | #1
Anxdrew,

 this whole patch-series seems to be corrupt. Not only was the
"incoming" cover emal missing the base to apply it to, but the patches
themselves are broken too.

See for example this one:

On Tue, Feb 4, 2020 at 1:34 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> From: David Hildenbrand <david@redhat.com>
> Subject: mm/page_alloc: fix and rework pfn handling in memmap_init_zone()
>
> Let's update the pfn manually whenever we continue the loop.  This makes
> the code easier to read but also less error prone (and we can directly fix
> one issue).
>
> When overlap_memmap_init() returns true, pfn is updated to
> "memblock_region_memory_end_pfn(r)".  So it already points at the *next*
> pfn to process.  Incrementing the pfn another time is wrong, we might
> leave one uninitialized.  I spotted this by inspecting the code, so I have
> no idea if this is relevant in practise (with kernelcore=3Dmirror).

Note that "=3D". That's some MIME stuff that you haven't decoded, and
then sent out as an email as-is.

> Link: http://lkml.kernel.org/r/20200113144035.10848-2-david@redhat.com
> Fixes: a9a9e77fbf27 ("mm: move mirrored memory specific code outside of m=
> emmap_init_zone")

Same here. That "=\n" shouldn't be in the email, it comes from some
original MIME data that wasn't ever properly decoded.

The *patch* itself seems fine, and the "=" signs are not mime:

> -       for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> +       for (pfn = start_pfn; pfn < end_pfn; ) {

so it seems that only the actual explanations have gotten corrupted somehow.

Cut-and-paste from some MIME source in a MUA that doesn't know mime?
Or directly from some mbox file without any MIME decoding?

                     Linus
diff mbox series

Patch

--- a/mm/page_alloc.c~mm-page_alloc-fix-and-rework-pfn-handling-in-memmap_init_zone
+++ a/mm/page_alloc.c
@@ -5905,18 +5905,20 @@  void __meminit memmap_init_zone(unsigned
 	}
 #endif
 
-	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+	for (pfn = start_pfn; pfn < end_pfn; ) {
 		/*
 		 * There can be holes in boot-time mem_map[]s handed to this
 		 * function.  They do not exist on hotplugged memory.
 		 */
 		if (context == MEMMAP_EARLY) {
 			if (!early_pfn_valid(pfn)) {
-				pfn = next_pfn(pfn) - 1;
+				pfn = next_pfn(pfn);
 				continue;
 			}
-			if (!early_pfn_in_nid(pfn, nid))
+			if (!early_pfn_in_nid(pfn, nid)) {
+				pfn++;
 				continue;
+			}
 			if (overlap_memmap_init(zone, &pfn))
 				continue;
 			if (defer_init(nid, pfn, end_pfn))
@@ -5944,6 +5946,7 @@  void __meminit memmap_init_zone(unsigned
 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
 			cond_resched();
 		}
+		pfn++;
 	}
 }