From: Mike Rapoport <rppt@kernel.org>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andrew Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86/NUMA: don't pass MAX_NUMNODES to memblock_set_node()
Date: Fri, 31 May 2024 12:42:34 +0300 [thread overview]
Message-ID: <ZlmbitudD6_JNb1s@kernel.org> (raw)
In-Reply-To: <bec94a1e-8c87-461a-a8db-1ea57385e745@intel.com>
Hi Dave,
On Wed, May 29, 2024 at 09:08:12AM -0700, Dave Hansen wrote:
> On 5/29/24 09:00, Jan Beulich wrote:
> >> In other words, it's not completely clear why ff6c3d81f2e8 introduced
> >> this problem.
> > It is my understanding that said change, by preventing the NUMA
> > configuration from being rejected, resulted in different code paths to
> > be taken. The observed crash was somewhat later than the "No NUMA
> > configuration found" etc messages. Thus I don't really see a connection
> > between said change not having had any MAX_NUMNODES check and it having
> > introduced the (only perceived?) regression.
>
> So your system has a bad NUMA config. If it's rejected, then all is
> merry. Something goes and writes over the nids in all of the memblocks
> to point to 0 (probably).
>
> If it _isn't_ rejected, then it leaves a memblock in place that points
> to MAX_NUMNODES. That MAX_NUMNODES is a ticking time bomb for later.
>
> So this patch doesn't actually revert the rejection behavior change in
> the Fixes: commit. It just makes the rest of the code more tolerant to
> _not_ rejecting the NUMA config?
It actually does. Before ff6c3d81f2e8 the NUMA coverage was verified
against numa_meminfo rather than memblock, so it could detect that only
small portion of the memory has node ID assigned.
With transition to memblock, the verification relies on node IDs set by the
arch code, but since memblock_validate_numa_coverage() only checked for
NUMA_NO_NODE is missed the ranges with nid == MAX_NUMNODES.
I took Jan's fix for memblock:
https://lore.kernel.org/all/1c8a058c-5365-4f27-a9f1-3aeb7fb3e7b2@suse.com
but I think that we should replace MAX_NUMNODES with NUMA_NO_NODE in calls to
memblock_set_node() in arch/x86.
--
Sincerely yours,
Mike.
prev parent reply other threads:[~2024-05-31 9:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-29 7:42 [PATCH] x86/NUMA: don't pass MAX_NUMNODES to memblock_set_node() Jan Beulich
2024-05-29 15:36 ` Dave Hansen
2024-05-29 16:00 ` Jan Beulich
2024-05-29 16:08 ` Dave Hansen
2024-05-31 6:21 ` Jan Beulich
2024-05-31 9:42 ` Mike Rapoport [this message]
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=ZlmbitudD6_JNb1s@kernel.org \
--to=rppt@kernel.org \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=jbeulich@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=peterz@infradead.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