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
