[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z3_d59kp4CuHQp97@arm.com>
Date: Thu, 9 Jan 2025 14:32:07 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Zhenhua Huang <quic_zhenhuah@...cinc.com>
Cc: Anshuman Khandual <anshuman.khandual@....com>, will@...nel.org,
ardb@...nel.org, ryan.roberts@....com, mark.rutland@....com,
joey.gouly@....com, dave.hansen@...ux.intel.com,
akpm@...ux-foundation.org, chenfeiyang@...ngson.cn,
chenhuacai@...nel.org, linux-mm@...ck.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
quic_tingweiz@...cinc.com, stable@...r.kernel.org
Subject: Re: [PATCH v4] arm64: mm: Populate vmemmap/linear at the page level
for hotplugged sections
On Thu, Jan 09, 2025 at 03:04:22PM +0800, Zhenhua Huang wrote:
> On 2025/1/8 18:52, Anshuman Khandual wrote:
> > > I found another bug, that even for early section, when
> > > vmemmap_populate is called, SECTION_IS_EARLY is not set.
> > > Therefore, early_section() always return false.
[...]
> > > Since vmemmap_populate() occurs during section initialization, it
> > > may be hard to say it is a bug.. However, should we instead using
> > > SECTION_MARKED_PRESENT to check? I tested well in my setup.
> > >
> > > Hot plug flow:
> > > 1. section_activate -> vmemmap_populate
> > > 2. mark PRESENT
> > >
> > > In contrast, the early flow:
> > > 1. memblocks_present -> mark PRESENT
> > > 2. __populate_section_memmap -> vmemmap_populate
> >
> > But from a semantics perspective, should SECTION_MARKED_PRESENT be marked on a
> > section before SECTION_IS_EARLY ? Is it really the expected behaviour here or
> > that needs to be fixed first ?
>
> The tricky part is vmemmap_populate initializes mem_map, that happens during
> mem_section initialization process. PRESENT or EARLY tag is in the same
> process as well. There doesn't appear to be a compelling reason to enforce a
> specific sequence..
The order in which a section is marked as present and vmemmap created
does seem a bit arbitrary. At least the early code seems to rely on the
for_each_present_section_nr() loop, so we'll always have this first but
it's not some internal kernel API that guarantees this.
> > Although SYSTEM_BOOTING state check might help but section flag seems to be the
> > right thing to do here.
>
> Good idea, I prefer to vote for this alternative rather than PRESENT tag. As
> I see we already took this stage to determine whether memmap pages are boot
> pages or not in common mm code:
> https://elixir.bootlin.com/linux/v6.13-rc3/source/mm/sparse-vmemmap.c#L465
The advantage of SYSTEM_BOOTING is that we don't need to rely on the
section information at all, though we could add a WARN_ON_ONCE if the
section is not present.
--
Catalin
Powered by blists - more mailing lists