From: David Hildenbrand <david@redhat.com>
To: Michal Hocko <mhocko@suse.com>, Mike Rapoport <rppt@kernel.org>,
Mel Gorman <mgorman@suse.de>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Andrea Arcangeli" <aarcange@redhat.com>,
"Baoquan He" <bhe@redhat.com>, "Borislav Petkov" <bp@alien8.de>,
"Chris Wilson" <chris@chris-wilson.co.uk>,
"H. Peter Anvin" <hpa@zytor.com>,
"Ingo Molnar" <mingo@redhat.com>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
"Łukasz Majczak" <lma@semihalf.com>,
"Mike Rapoport" <rppt@linux.ibm.com>, "Qian Cai" <cai@lca.pw>,
"Sarvela, Tomi P" <tomi.p.sarvela@intel.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Vlastimil Babka" <vbabka@suse.cz>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
stable@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH v5 1/1] mm: refactor initialization of struct page for holes in memory layout
Date: Mon, 15 Feb 2021 10:05:10 +0100 [thread overview]
Message-ID: <c2da1e76-d2ea-04df-d258-cf8a87a397d6@redhat.com> (raw)
In-Reply-To: <YCo4Lyio1h2Heixh@dhcp22.suse.cz>
On 15.02.21 10:00, Michal Hocko wrote:
> On Sun 14-02-21 20:00:16, Mike Rapoport wrote:
>> 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.
>
> Touching a reserved page which doesn't belong to you is a bug. Sure we
> cannot enforce that rule by runtime checks. But incorrect/misleading zone/node
> association is the least of the problem when somebody already does 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 really fail to see what you mean by correct zone/node for a memory
> range which is not associated with any real node.
>
>> I believe that any improvement in memory map consistency is a step forward.
>
> I do agree but we are talking about a subtle bug (VM_BUG_ON) which would
> be better of with a simplistic fix first. You can work on consistency
> improvements on top of that.
>
>>>>> 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.
>
> Can we actually explore that path before adding yet additional
> complexity and potentially a very involved fix for a subtle problem?
>
> Mel who is author of this code might help us out. I have to say I do not
> see the point for the VM_BUG_ON other than a better debuggability. If
> there is a real incosistency problem as a result then we should be
> handling that situation for non debugging kernels as well.
>
I have no time to summarize, you can find the complete discussion (also
involving Mel) at
https://lkml.kernel.org/r/20201121194506.13464-1-aarcange@redhat.com
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2021-02-15 9:07 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-08 11:08 [PATCH v5 1/1] mm: refactor initialization of struct page for holes in memory layout Mike Rapoport
2021-02-08 21:11 ` Andrew Morton
2021-02-08 21:25 ` Mike Rapoport
2021-02-12 9:55 ` David Hildenbrand
2021-02-12 9:56 ` David Hildenbrand
2021-02-12 10:11 ` Michal Hocko
2021-02-12 10:16 ` David Hildenbrand
2021-02-12 10:37 ` Michal Hocko
2021-02-14 17:29 ` Mike Rapoport
2021-02-15 8:45 ` David Hildenbrand
2021-02-16 11:13 ` Mike Rapoport
2021-02-12 10:33 ` Michal Hocko
2021-02-12 10:42 ` David Hildenbrand
2021-02-12 13:18 ` Michal Hocko
2021-02-14 18:00 ` Mike Rapoport
2021-02-15 9:00 ` Michal Hocko
2021-02-15 9:05 ` David Hildenbrand [this message]
2021-02-15 21:24 ` Mike Rapoport
2021-02-16 8:33 ` Michal Hocko
2021-02-16 11:01 ` Mike Rapoport
2021-02-16 11:39 ` Michal Hocko
2021-02-16 12:34 ` Vlastimil Babka
2021-02-16 12:59 ` Vlastimil Babka
2021-02-16 13:11 ` Michal Hocko
2021-02-16 16:39 ` Vlastimil Babka
2021-02-16 17:49 ` Mike Rapoport
2021-02-17 12:27 ` Vlastimil Babka
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=c2da1e76-d2ea-04df-d258-cf8a87a397d6@redhat.com \
--to=david@redhat.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bhe@redhat.com \
--cc=bp@alien8.de \
--cc=cai@lca.pw \
--cc=chris@chris-wilson.co.uk \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lma@semihalf.com \
--cc=mgorman@suse.de \
--cc=mhocko@suse.com \
--cc=mingo@redhat.com \
--cc=rppt@kernel.org \
--cc=rppt@linux.ibm.com \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=tomi.p.sarvela@intel.com \
--cc=torvalds@linux-foundation.org \
--cc=vbabka@suse.cz \
--cc=x86@kernel.org \
/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).