Message-ID: <mafs0wm9fn2l3.fsf@kernel.org>
Date: Fri, 13 Jun 2025 16:22:32 +0200
From: Pratyush Yadav <pratyush@...nel.org>
To: Mike Rapoport <rppt@...nel.org>
Cc: Pratyush Yadav <pratyush@...nel.org>,  Pasha Tatashin
 <pasha.tatashin@...een.com>,  Alexander Graf <graf@...zon.com>,  Changyuan
 Lyu <changyuanl@...gle.com>,  Andrew Morton <akpm@...ux-foundation.org>,
  Baoquan He <bhe@...hat.com>,  kexec@...ts.infradead.org,
  linux-kernel@...r.kernel.org,  linux-mm@...ck.org,  Michal Clapinski
 <mclapinski@...gle.com>
Subject: Re: [PATCH] kho: initialize tail pages for higher order folios
 properly

On Wed, Jun 11 2025, Mike Rapoport wrote:

> On Wed, Jun 11, 2025 at 04:01:52PM +0200, Pratyush Yadav wrote:
>> On Wed, Jun 11 2025, Mike Rapoport wrote:
>> 
>> > On Wed, Jun 11, 2025 at 09:14:55AM -0400, Pasha Tatashin wrote:
>> >> On Wed, Jun 11, 2025 at 9:06 AM Pratyush Yadav <pratyush@...nel.org> wrote:
>> >> >
>> >> > On Tue, Jun 10 2025, Pasha Tatashin wrote:
>> >> >
>> >> > >> > > I think it should be the other way around, KHO should depend on
>> >> > >> > > !DEFERRED_STRUCT_PAGE_INIT.
>> >> > >> >
>> >> > >> > Agreed, and this is what I first tried, but that does not work, there
>> >> > >> > is some circular dependency breaking the build. If you feel
>> >> > >> > adventurous you can try that :-)
>> >> > >>
>> >> > >> Hmm, weird, worked for me :/
>> >> >
>> >> > Worked for me as well.
>> >> >
>> >> > >
>> >> > > I am super confused, it did not work for me over weekend, and now it
>> >> > > is working. Even `make menuconfig` would not work. Anyways, I will put
>> >> > > it in the appropriate place.
>> >> > >
>> >> > >>
>> >> > >> > > > We will need to teah KHO to work with deferred struct page init. I
>> >> > >> > > > suspect, we could init preserved struct pages and then skip over them
>> >> > >> > > > during deferred init.
>> >> > >> > >
>> >> > >> > > We could, but with that would mean we'll run this before SMP and it's not
>> >> > >> > > desirable. Also, init_deferred_page() for a random page requires
>> >> > >> >
>> >> > >> > We already run KHO init before smp_init:
>> >> > >> > start_kernel() -> mm_core_init() -> kho_memory_init() ->
>> >> > >> > kho_restore_folio() -> struct pages must be already initialized here!
>> >> > >> >
>> >> > >> > While deferred struct pages are initialized:
>> >> > >> > start_kernel() -> rest_init() -> kernel_init() ->
>> >> > >> > kernel_init_freeable() -> page_alloc_init_late() ->
>> >> > >> > deferred_init_memmap()
>> >> > >> >
>> >> > >> > If the number of preserved pages that is needed during early boot is
>> >> > >> > relatively small, that it should not be an issue to pre-initialize
>> >> > >> > struct pages for them before deferred struct pages are initialized. We
>> >> > >> > already pre-initialize some  "struct pages" that are needed during
>> >> > >> > early boot before the reset are initialized, see deferred_grow_zone()
>> >> > >>
>> >> > >> deferred_grow_zone() takes a chunk in the beginning of uninitialized range,
>> >> > >> with kho we are talking about some random pages. If we preinit them early,
>> >> > >> deferred_init_memmap() will overwrite them.
>> >> > >
>> >> > > Yes, this is why I am saying that we would need to skip the KHO
>> >> > > initialized "struct pages" somehow during deferred initialization. If
>> >> > > we create an ordered by PFN list of early-initialized KHO struct
>> >> > > pages, skipping during deferred initialization could be done
>> >> > > efficiently.
>> >> >
>> >> > Or keep things simple and don't use any KHO struct pages during early
>> >> > init. You can access the page itself, just don't use its struct page.
>> >> >
>> >> > Currently the only user of kho_restore_folio() during init is
>> >> > kho_memory_init(). The FDT is accessed by doing
>> >> > phys_to_virt(kho_in.fdt_phys) anyway, so there is really no need for
>> >> > restoring the folio so early. It can be done later, for example when LUO
>> >> > does the finish event, to clean up and free the folio.
>> >> 
>> >> Good suggestion, however, KHO does not have any sophisticated users
>> >> that we are going to be adding as part of the live update work in the
>> >> future: IR, KVM, early VCPU threads, and so on. So, while today, this
>> >> might work, in the future, I am not sure if we should expect struct
>> >> pages are not accessed until after deferred initialization or simply
>> >> fix it once and for all.
>> >
>> > KHO already accesses stuct page early and uses page->private for order.
>> > Since preserved memory is reserved in memblock, deferred init of struct
>> > pages won't touch those pages, we just need to make sure they are properly 
>> 
>> Not strictly true. Some of them might have been initialized from
>> free_area_init() -> memmap_init() (the ones not eligible for deferred
>> init), which happens before KHO makes its memblock reservations.
>> 
>> > initialized at some point. If we don't expect many kho_restore_folio()
>> > before page_alloc_init_late() we can use init_deferred_page() for early
>> > accesses.
>> 
>> I tried doing this when looking into this initially, but it doesn't work
>> for some reason.
>> 
>>     static void kho_restore_page(struct page *page, unsigned int order)
>>     {
>>     	unsigned int i, nr_pages = (1 << order);
>>     
>>     	/* Head page gets refcount of 1. */
>>     	init_deferred_page(page_to_pfn(page), NUMA_NO_NODE);
>
>
> This would do
>
> 	if (early_page_initialised(pfn, nid))
> 		return;
>
> 	__init_page_from_nid(pfn, nid);
>
> and I'm really surprised it didn't crash in early_page_initialised()
> because of NUMA_NO_NODE :)

Oh, right. Using the wrong node completely throws
early_page_initialised() off.

>
> What might work here is 
>
> 	pfn = page_to_pfn(page);
> 	__init_page_from_nid(pfn, early_pfn_to_nid(pfn));

Yep, that works. Although this would do early_pfn_to_nid() for each page
so it isn't very efficient. And we also need to make sure memblock does
not go away.

>
>>     	set_page_count(page, 1);
>>     
>>     	/* For higher order folios, tail pages get a page count of zero. */
>>     	for (i = 1; i < nr_pages; i++) {
>>     		init_deferred_page(page_to_pfn(page + i), NUMA_NO_NODE);
>>     		set_page_count(page + i, 0);
>>     	}
>>     
>>     [...]
>> 
[...]

-- 
Regards,
Pratyush Yadav

Powered by blists - more mailing lists