linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@linux.ibm.com>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Hildenbrand <david@redhat.com>,
	Vlastimil Babka <vbabka@suse.cz>, Mel Gorman <mgorman@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Qian Cai <cai@lca.pw>,
	Michal Hocko <mhocko@kernel.org>,
	linux-kernel@vger.kernel.org, Baoquan He <bhe@redhat.com>
Subject: Re: [PATCH 1/1] mm: compaction: avoid fast_isolate_around() to set pageblock_skip on reserved pages
Date: Wed, 2 Dec 2020 19:39:23 +0200	[thread overview]
Message-ID: <20201202173923.GM123287@linux.ibm.com> (raw)
In-Reply-To: <X8bjgw5LPAZrSrwp@redhat.com>

Hello Andrea,

On Tue, Dec 01, 2020 at 07:44:51PM -0500, Andrea Arcangeli wrote:
> Hello Mike,
> 
> On Sun, Nov 29, 2020 at 02:32:57PM +0200, Mike Rapoport wrote:
> > Hello Andrea,
> > 
> > On Thu, Nov 26, 2020 at 07:46:05PM +0200, Mike Rapoport wrote:
> > > On Thu, Nov 26, 2020 at 11:05:14AM +0100, David Hildenbrand wrote:
> > > 
> > > Let's try to merge init_unavailable_memory() into memmap_init().
> > > Than it'll be able to set zone/nid for those nasty pfns that BIOS
> > > decided to keep to itself, like in Andrea's case and will also take care
> > > of struct pages that do not really have a frame in DRAM, but are there
> > > because of arbitrary section size.
> > 
> > Did you have a chance to check if this solves your issue?
> > If yes, I'll resend this as a formal patch.
> 
> I tested the patch you sent, but it doesn't seem to boot. Reverting it
> booted.

Hmm, do you have any logs? It worked on my box and with various memory
configurations in qemu.

> Also I noticed leaving pages uninitialized already happened before and
> was fixed in 124049decbb121ec32742c94fb5d9d6bed8f24d8 where literally
> all holes were registered in memblock_reserve() by hand to workaround
> this very same defect in memblock callee we're fixing again.
>
> Then it was lifted in 9fd61bc95130d4971568b89c9548b5e0a4e18e0e since
> the memblock was supposed to be able to initialize all pages.

I still that think both 124049decbb1 ("x86/e820: put !E820_TYPE_RAM
regions into memblock.reserved") and 9fd61bc95130 ("Revert "x86/e820:
put !E820_TYPE_RAM regions into memblock.reserved") are band aids and
they do not address the root cause but rather try to workaround it.

The problem is that we have no proper way to initialize struct pages
corresponding to holes. The holes can appear in case when the memory is
not a multiple of SECTION_SIZE and, at least on x86, in case BIOS
reserves pages and they are not E820_TYPE_RAM.

The first case is not much of a problem, as I doubt there are still
systems that have not MAX_ORDER aligned memory banks, so the "trailing"
pages in semi-empty section will never be used by the page allocator.

The BIOS case, however is more problematic as it creates holes inside
MAX_ORDER aligned pageblocks.

Pushing the pages in such holes in and out of memblock.reserved won't
really help because they are not in memblock.memory while most
calculations during initialization of page allocator and memory map use
memblock.memory and iterate over it.

As these holes do not seem to appear on zone/node boundaries, we do not
see reports about wrong zone/node sizing because of the holes.

I believe that the proper solution would be to have all the memory
reserved by the firmware added to memblock.memory, like all other
architectures do, but I'm not sure if we can easily and reliably
determine what of E820 reserved types are actually backed by RAM.
So this is not feasible as a short term solution.

My patch [1], though, is an immediate improvement and I think it's worth
trying to fix off-by-one's that prevent your system from booting.

[1] https://lore.kernel.org/lkml/20201201181502.2340-1-rppt@kernel.org

> Since this seems the second time this happens, I'd suggest to change
> the graceful pfn, 0,0 initialization to memset(page, 0xff,
> sizeof(struct page)) too like we mentioned earlier and then have at
> least a DEBUG_SOMETHING=y to search for struct pages with all 1 in
> page->flags to ensure the boot stage worked right so perhaps there's a
> graceful notification at boot before a VM_BUG_ON triggers later. The
> page struct validation could be done based on DEBUG_VM=y too since it
> won't cause any runtime cost post boot.
>
> Thanks,
> Andrea
> 

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2020-12-02 17:39 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23 21:25 compaction: VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn)) Qian Cai
2020-04-24  3:43 ` Baoquan He
2020-04-24 13:45   ` Qian Cai
2020-05-05 12:43     ` Baoquan He
2020-05-05 13:20       ` Qian Cai
2020-05-11  1:21         ` Baoquan He
2020-04-26 14:41 ` Mike Rapoport
2020-04-27 13:45   ` Qian Cai
2020-11-21 19:45 ` [PATCH 0/1] VM_BUG_ON_PAGE(!zone_spans_pfn) in set_pfnblock_flags_mask Andrea Arcangeli
2020-11-21 19:45   ` [PATCH 1/1] mm: compaction: avoid fast_isolate_around() to set pageblock_skip on reserved pages Andrea Arcangeli
2020-11-21 19:53     ` Andrea Arcangeli
2020-11-23 11:26       ` David Hildenbrand
2020-11-23 13:01     ` Vlastimil Babka
2020-11-24 13:32       ` Mel Gorman
2020-11-24 20:56         ` Andrea Arcangeli
2020-11-25 10:30           ` Mel Gorman
2020-11-25 17:59             ` Andrea Arcangeli
2020-11-26 10:47               ` Mel Gorman
2020-12-06  2:26                 ` Andrea Arcangeli
2020-12-06 23:47                   ` Mel Gorman
2020-11-25  5:34       ` Andrea Arcangeli
2020-11-25  6:45         ` David Hildenbrand
2020-11-25  8:51           ` Mike Rapoport
2020-11-25 10:39           ` Mel Gorman
2020-11-25 11:04             ` David Hildenbrand
2020-11-25 11:41               ` David Hildenbrand
2020-11-25 18:47                 ` Andrea Arcangeli
2020-11-25 13:33               ` Mel Gorman
2020-11-25 13:41                 ` David Hildenbrand
2020-11-25 18:28           ` Andrea Arcangeli
2020-11-25 19:27             ` David Hildenbrand
2020-11-25 20:41               ` Andrea Arcangeli
2020-11-25 21:13                 ` David Hildenbrand
2020-11-25 21:04               ` Mike Rapoport
2020-11-25 21:38                 ` Andrea Arcangeli
2020-11-26  9:36                   ` Mike Rapoport
2020-11-26 10:05                     ` David Hildenbrand
2020-11-26 17:46                       ` Mike Rapoport
2020-11-29 12:32                         ` Mike Rapoport
2020-12-02  0:44                           ` Andrea Arcangeli
2020-12-02 17:39                             ` Mike Rapoport [this message]
2020-12-03  6:23                               ` Andrea Arcangeli
2020-12-03 10:51                                 ` Mike Rapoport
2020-12-03 17:31                                   ` Andrea Arcangeli
2020-12-06  8:09                                     ` Mike Rapoport
2020-11-26 18:15                       ` Andrea Arcangeli
2020-11-26 18:29                     ` Andrea Arcangeli
2020-11-26 19:44                       ` Mike Rapoport
2020-11-26 20:30                         ` Andrea Arcangeli
2020-11-26 21:03                           ` Mike Rapoport
2020-11-26 19:21                     ` Andrea Arcangeli
2020-11-25 12:08         ` Vlastimil Babka
2020-11-25 13:32           ` David Hildenbrand
2020-11-25 14:13             ` Mike Rapoport
2020-11-25 14:42               ` David Hildenbrand
2020-11-26 10:51                 ` Mel Gorman
2020-11-25 19:14               ` Andrea Arcangeli
2020-11-25 19:01           ` Andrea Arcangeli
2020-11-25 19:33             ` David Hildenbrand
2020-11-26  3:40         ` Andrea Arcangeli
2020-11-26 10:43           ` Mike Rapoport

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201202173923.GM123287@linux.ibm.com \
    --to=rppt@linux.ibm.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bhe@redhat.com \
    --cc=cai@lca.pw \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).