[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bc843a93-42c7-4317-bd3d-dc48c63e095d@quicinc.com>
Date: Fri, 10 Jan 2025 11:13:27 +0800
From: Zhenhua Huang <quic_zhenhuah@...cinc.com>
To: Catalin Marinas <catalin.marinas@....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 2025/1/9 22:32, Catalin Marinas wrote:
> 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.
Hi Catalin,
Sorry, but I don't fully understand your comment here, IIUC we shouldn't
add WARN_ON_ONCE in vmemmap_populate(). As you mentioned above, early
code relies on section present. while the hotplug code does not
guarantee, it will set PRESENT after calling vmemmap_populate().
By the way, seems you're not opposed to using SYSTEM_BOOTING ? If so,
please take a look at latest post:
https://lore.kernel.org/linux-mm/20250109093824.452925-1-quic_zhenhuah@quicinc.com/
Thanks very much!
>
Powered by blists - more mailing lists