[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210214180016.GO242749@kernel.org>
Date: Sun, 14 Feb 2021 20:00:16 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Michal Hocko <mhocko@...e.com>
Cc: David Hildenbrand <david@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Andrea Arcangeli <aarcange@...hat.com>,
Baoquan He <bhe@...hat.com>, Borislav Petkov <bp@...en8.de>,
Chris Wilson <chris@...is-wilson.co.uk>,
"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Ćukasz Majczak <lma@...ihalf.com>,
Mel Gorman <mgorman@...e.de>,
Mike Rapoport <rppt@...ux.ibm.com>, Qian Cai <cai@....pw>,
"Sarvela, Tomi P" <tomi.p.sarvela@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, stable@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v5 1/1] mm: refactor initialization of struct page for
holes in memory layout
On Fri, Feb 12, 2021 at 02:18:20PM +0100, Michal Hocko wrote:
> On Fri 12-02-21 11:42:15, David Hildenbrand wrote:
> > On 12.02.21 11:33, Michal Hocko wrote:
> [...]
> > > I have to digest this but my first impression is that this is more heavy
> > > weight than it needs to. Pfn walkers should normally obey node range at
> > > least. The first pfn is usually excluded but I haven't seen real
> >
> > We've seen examples where this is not sufficient. Simple example:
> >
> > Have your physical memory end within a memory section. Easy via QEMU, just
> > do a "-m 4000M". The remaining part of the last section has fake/wrong
> > node/zone info.
>
> Does this really matter though. If those pages are reserved then nobody
> will touch them regardless of their node/zone ids.
>
> > Hotplug memory. The node/zone gets resized such that PFN walkers might
> > stumble over it.
> >
> > The basic idea is to make sure that any initialized/"online" pfn belongs to
> > exactly one node/zone and that the node/zone spans that PFN.
>
> Yeah, this sounds like a good idea but what is the poper node for hole
> between two ranges associated with a different nodes/zones? This will
> always be a random number. We should have a clear way to tell "do not
> touch those pages" and PageReserved sounds like a good way to tell that.
Nobody should touch reserved pages, but I don't think we can ensure that.
We can correctly set the zone links for the reserved pages for holes in the
middle of a zone based on the architecture constraints and with only the
holes in the beginning/end of the memory will be not spanned by any
node/zone which in practice does not seem to be a problem as the VM_BUG_ON
in set_pfnblock_flags_mask() never triggered on pfn 0.
I believe that any improvement in memory map consistency is a step forward.
> > > problems with that. The VM_BUG_ON blowing up is really bad but as said
> > > above we can simply make it less offensive in presence of reserved pages
> > > as those shouldn't reach that path AFAICS normally.
> >
> > Andrea tried tried working around if via PG_reserved pages and it resulted
> > in quite some ugly code. Andrea also noted that we cannot rely on any random
> > page walker to do the right think when it comes to messed up node/zone info.
>
> I am sorry, I haven't followed previous discussions. Has the removal of
> the VM_BUG_ON been considered as an immediate workaround?
It was never discussed, but I'm not sure it's a good idea.
Judging by the commit message that introduced the VM_BUG_ON (commit
86051ca5eaf5 ("mm: fix usemap initialization")) there was yet another
inconsistency in the memory map that required a special care.
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists