On Fri, Jun 13, 2025 at 04:22:32PM +0200, Pratyush Yadav wrote:
> 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 <praty...@kernel.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.
Yeah, that's far from ideal. Let's just gate KHO on
!CONFIG_DEFERRED_PAGE_STRUCT_INIT for now.
> >> 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
--
Sincerely yours,
Mike.
