Re: [PATCH] mm: refactor initialization of stuct page for holes in memory layout

[Andrea Arcangeli <aarcange@redhat.com>]

Hello,

On Thu, Dec 03, 2020 at 08:25:49AM +0200, Mike Rapoport wrote:
> On Wed, Dec 02, 2020 at 03:47:36PM -0800, Andrew Morton wrote:
> > On Tue,  1 Dec 2020 20:15:02 +0200 Mike Rapoport <rppt@kernel.org> wrote:
> > 
> > > From: Mike Rapoport <rppt@linux.ibm.com>
> > > 
> > > There could be struct pages that are not backed by actual physical memory.
> > > This can happen when the actual memory bank is not a multiple of
> > > SECTION_SIZE or when an architecture does not register memory holes
> > > reserved by the firmware as memblock.memory.
> > > 
> > > Such pages are currently initialized using init_unavailable_mem() function
> > > that iterated through PFNs in holes in memblock.memory and if there is a
> > > struct page corresponding to a PFN, the fields if this page are set to
> > > default values and it is marked as Reserved.
> > > 
> > > init_unavailable_mem() does not take into account zone and node the page
> > > belongs to and sets both zone and node links in struct page to zero.
> > > 
> > > On a system that has firmware reserved holes in a zone above ZONE_DMA, for
> > > instance in a configuration below:
> > > 
> > > 	# grep -A1 E820 /proc/iomem
> > > 	7a17b000-7a216fff : Unknown E820 type
> > > 	7a217000-7bffffff : System RAM
> > > 
> > > unset zone link in struct page will trigger
> > > 
> > > 	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
> > 
> > That sounds pretty serious.

My understanding is that with DEBUG_VM=n the invariant that broke
won't cause trouble, but Fedora is helping the upstream testing by
keeping DEBUG_VM=y and it's shipping with v5.8 and v5.9 for a while,
so it could very well crash those kernels if they've that type 20
thing in the e820 map.

> > 
> > > because there are pages in both ZONE_DMA32 and ZONE_DMA (unset zone link in
> > > struct page) in the same pageblock.
> > > 
> > > Interleave initialization of pages that correspond to holes with the
> > > initialization of memory map, so that zone and node information will be
> > > properly set on such pages.
> > > 
> > 
> > Should this be backported to -stable?  If so, do we have a suitable Fixes:?
> 
> Sorry, I forgot to add
> 
> Fixes: 73a6e474cb37 ("mm: memmap_init: iterate over memblock regions rather that check each PFN")

I've been wondering already why I'm the only one getting a crash every
two weeks. Ince it crashed in MADV_HUGEPAGE of qemu that would
definitely happened even with Fedora despite the THP enabled =
madvise, and it hung qemu for good so it was noticeable since it was
in direction compaction.

Other times it was in kcompactd so it just killed the kernel thread
and it was only noticeable in the kernel logs and probably it doesn't
happen that frequently unless THP enabled = always, although it could
still happen, compaction isn't used just for THP.

Thanks,
Andrea