[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aMrGT9IyHBFe5-nH@kernel.org>
Date: Wed, 17 Sep 2025 17:31:43 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Pratyush Yadav <pratyush@...nel.org>
Cc: Alexander Graf <graf@...zon.com>, Changyuan Lyu <changyuanl@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Baoquan He <bhe@...hat.com>,
Pasha Tatashin <pasha.tatashin@...een.com>,
Jason Gunthorpe <jgg@...dia.com>, Chris Li <chrisl@...nel.org>,
Jason Miu <jasonmiu@...gle.com>, linux-kernel@...r.kernel.org,
kexec@...ts.infradead.org, linux-mm@...ck.org
Subject: Re: [PATCH v2 1/2] kho: move sanity checks to kho_restore_page()
On Wed, Sep 17, 2025 at 02:56:53PM +0200, Pratyush Yadav wrote:
> While KHO exposes folio as the primitive externally, internally its
> restoration machinery operates on pages. This can be seen with
> kho_restore_folio() for example. It performs some sanity checks and
> hands it over to kho_restore_page() to do the heavy lifting of page
> restoration. After the work done by kho_restore_page(),
> kho_restore_folio() only converts the head page to folio and returns it.
> Similarly, deserialize_bitmap() operates on the head page directly to
> store the order.
>
> Move the sanity checks for valid phys and order from the public-facing
> kho_restore_folio() to the private-facing kho_restore_page(). This makes
> the boundary between page and folio clearer from KHO's perspective.
>
> While at it, drop the comment above kho_restore_page(). The comment is
> misleading now. The function stopped looking like free_reserved_page()
> since 12b9a2c05d1b4 ("kho: initialize tail pages for higher order folios
> properly"), and now looks even more different.
>
> Signed-off-by: Pratyush Yadav <pratyush@...nel.org>
Reviewed-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> ---
>
> Notes:
> Changes in v2:
>
> - New in v2.
>
> kernel/kexec_handover.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> index ecd1ac210dbd7..69cab82abaaef 100644
> --- a/kernel/kexec_handover.c
> +++ b/kernel/kexec_handover.c
> @@ -183,10 +183,18 @@ static int __kho_preserve_order(struct kho_mem_track *track, unsigned long pfn,
> return 0;
> }
>
> -/* almost as free_reserved_page(), just don't free the page */
> -static void kho_restore_page(struct page *page, unsigned int order)
> +static struct page *kho_restore_page(phys_addr_t phys)
> {
> - unsigned int nr_pages = (1 << order);
> + struct page *page = pfn_to_online_page(PHYS_PFN(phys));
> + unsigned int nr_pages, order;
> +
> + if (!page)
> + return NULL;
> +
> + order = page->private;
> + if (order > MAX_PAGE_ORDER)
> + return NULL;
> + nr_pages = (1 << order);
>
> /* Head page gets refcount of 1. */
> set_page_count(page, 1);
> @@ -199,6 +207,7 @@ static void kho_restore_page(struct page *page, unsigned int order)
> prep_compound_page(page, order);
>
> adjust_managed_page_count(page, nr_pages);
> + return page;
> }
>
> /**
> @@ -209,18 +218,9 @@ static void kho_restore_page(struct page *page, unsigned int order)
> */
> struct folio *kho_restore_folio(phys_addr_t phys)
> {
> - struct page *page = pfn_to_online_page(PHYS_PFN(phys));
> - unsigned long order;
> -
> - if (!page)
> - return NULL;
> -
> - order = page->private;
> - if (order > MAX_PAGE_ORDER)
> - return NULL;
> + struct page *page = kho_restore_page(phys);
>
> - kho_restore_page(page, order);
> - return page_folio(page);
> + return page ? page_folio(page) : NULL;
> }
> EXPORT_SYMBOL_GPL(kho_restore_folio);
>
> --
> 2.47.3
>
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists