[PATCH v1 0/2] Optimize the code in kexec_handover.c

[PATCH v1 0/2] Optimize the code in kexec_handover.c

[Chenghao Duan]

Patch 1: No functional changes, remove redundant code.
Patch 2: Rework error unwind logic for kho_restore_vmalloc().

Chenghao Duan (2):
  kho: remove useless phys increment in inner loop
  kho: unwind restored pages on kho_restore_vmalloc error

 kernel/liveupdate/kexec_handover.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH v1 1/2] kho: remove useless phys increment in inner loop

[Chenghao Duan]

The statement phys += contig_pages * PAGE_SIZE at the end of the inner
for-loop in kho_restore_vmalloc() is redundant code. The variable 'phys'
is local to each loop iteration (re-assigned from chunk->phys[i] on every
iteration) and is never referenced after this increment, so the statement
has no functional impact. Remove it to avoid confusion during code review.

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
 kernel/liveupdate/kexec_handover.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
index 175c08a6e41e..ba03ff5baa9e 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -1191,8 +1191,6 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 
 			for (int j = 0; j < contig_pages; j++)
 				pages[idx++] = page + j;
-
-			phys += contig_pages * PAGE_SIZE;
 		}
 
 		page = kho_restore_pages(virt_to_phys(chunk), 1);
-- 
2.25.1

[PATCH v1 2/2] kho: unwind restored pages on kho_restore_vmalloc error

[Chenghao Duan]

In kho_restore_vmalloc(), when kho_restore_pages() succeeds, the
recovered pages are handed to the buddy allocator (via
adjust_managed_page_count()). If any later step (e.g.
__get_vm_area_node() or vmap_pages_range()) fails, the original error
path only called kvfree(pages), leaking those folio pages.

Fix by tracking how many folio groups have been restored with
restored_idx. On any failure, use err_unwind_restored to walk
restored_idx backwards and return each folio group to the buddy via
__free_pages() before freeing the pages array.

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
 kernel/liveupdate/kexec_handover.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
index ba03ff5baa9e..ed35405f59ab 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -1161,6 +1161,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 	struct vm_struct *area;
 	struct page **pages;
 	unsigned int idx = 0;
+	unsigned int restored_idx = 0;
 	int err;
 
 	vm_flags = kho_flags_to_vmalloc(preservation->flags);
@@ -1183,11 +1184,11 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 			phys_addr_t phys = chunk->phys[i];
 
 			if (idx + contig_pages > total_pages)
-				goto err_free_pages_array;
+				goto err_unwind_restored;
 
 			page = kho_restore_pages(phys, contig_pages);
 			if (!page)
-				goto err_free_pages_array;
+				goto err_unwind_restored;
 
 			for (int j = 0; j < contig_pages; j++)
 				pages[idx++] = page + j;
@@ -1195,13 +1196,14 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 
 		page = kho_restore_pages(virt_to_phys(chunk), 1);
 		if (!page)
-			goto err_free_pages_array;
+			goto err_unwind_restored;
+		restored_idx = idx;
 		chunk = KHOSER_LOAD_PTR(chunk->hdr.next);
 		__free_page(page);
 	}
 
 	if (idx != total_pages)
-		goto err_free_pages_array;
+		goto err_unwind_restored;
 
 	area = __get_vm_area_node(total_pages * PAGE_SIZE, align, shift,
 				  vm_flags | VM_UNINITIALIZED,
@@ -1209,7 +1211,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 				  NUMA_NO_NODE, GFP_KERNEL,
 				  __builtin_return_address(0));
 	if (!area)
-		goto err_free_pages_array;
+		goto err_unwind_restored;
 
 	addr = (unsigned long)area->addr;
 	size = get_vm_area_size(area);
@@ -1231,7 +1233,16 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
 
 err_free_vm_area:
 	free_vm_area(area);
-err_free_pages_array:
+err_unwind_restored:
+	/*
+	 * Pages already restored via kho_restore_pages() have been given to
+	 * the buddy allocator (via adjust_managed_page_count()). Return them
+	 * to the buddy so that failure leaves the system in a clean state.
+	 */
+	while (restored_idx > 0) {
+		restored_idx -= contig_pages;
+		__free_pages(pages[restored_idx], order);
+	}
 	kvfree(pages);
 	return NULL;
 }
-- 
2.25.1

Re: [PATCH v1 1/2] kho: remove useless phys increment in inner loop

[Pratyush Yadav]

On Fri, Aug 07 2026, Chenghao Duan wrote:

> The statement phys += contig_pages * PAGE_SIZE at the end of the inner
> for-loop in kho_restore_vmalloc() is redundant code. The variable 'phys'
> is local to each loop iteration (re-assigned from chunk->phys[i] on every
> iteration) and is never referenced after this increment, so the statement
> has no functional impact. Remove it to avoid confusion during code review.
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

[...]

-- 
Regards,
Pratyush Yadav

Re: [PATCH v1 2/2] kho: unwind restored pages on kho_restore_vmalloc error

[Pratyush Yadav]

On Fri, Aug 07 2026, Chenghao Duan wrote:

> In kho_restore_vmalloc(), when kho_restore_pages() succeeds, the
> recovered pages are handed to the buddy allocator (via
> adjust_managed_page_count()). If any later step (e.g.
> __get_vm_area_node() or vmap_pages_range()) fails, the original error
> path only called kvfree(pages), leaking those folio pages.
>
> Fix by tracking how many folio groups have been restored with
> restored_idx. On any failure, use err_unwind_restored to walk
> restored_idx backwards and return each folio group to the buddy via
> __free_pages() before freeing the pages array.
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> ---
>  kernel/liveupdate/kexec_handover.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index ba03ff5baa9e..ed35405f59ab 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1161,6 +1161,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  	struct vm_struct *area;
>  	struct page **pages;
>  	unsigned int idx = 0;
> +	unsigned int restored_idx = 0;
>  	int err;
>  
>  	vm_flags = kho_flags_to_vmalloc(preservation->flags);
> @@ -1183,11 +1184,11 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  			phys_addr_t phys = chunk->phys[i];
>  
>  			if (idx + contig_pages > total_pages)
> -				goto err_free_pages_array;
> +				goto err_unwind_restored;
>  
>  			page = kho_restore_pages(phys, contig_pages);
>  			if (!page)
> -				goto err_free_pages_array;
> +				goto err_unwind_restored;
>  
>  			for (int j = 0; j < contig_pages; j++)
>  				pages[idx++] = page + j;
> @@ -1195,13 +1196,14 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  
>  		page = kho_restore_pages(virt_to_phys(chunk), 1);
>  		if (!page)
> -			goto err_free_pages_array;
> +			goto err_unwind_restored;
> +		restored_idx = idx;
>  		chunk = KHOSER_LOAD_PTR(chunk->hdr.next);
>  		__free_page(page);
>  	}
>  
>  	if (idx != total_pages)
> -		goto err_free_pages_array;
> +		goto err_unwind_restored;
>  
>  	area = __get_vm_area_node(total_pages * PAGE_SIZE, align, shift,
>  				  vm_flags | VM_UNINITIALIZED,
> @@ -1209,7 +1211,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  				  NUMA_NO_NODE, GFP_KERNEL,
>  				  __builtin_return_address(0));
>  	if (!area)
> -		goto err_free_pages_array;
> +		goto err_unwind_restored;
>  
>  	addr = (unsigned long)area->addr;
>  	size = get_vm_area_size(area);
> @@ -1231,7 +1233,16 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  
>  err_free_vm_area:
>  	free_vm_area(area);
> -err_free_pages_array:
> +err_unwind_restored:
> +	/*
> +	 * Pages already restored via kho_restore_pages() have been given to
> +	 * the buddy allocator (via adjust_managed_page_count()). Return them
> +	 * to the buddy so that failure leaves the system in a clean state.
> +	 */
> +	while (restored_idx > 0) {
> +		restored_idx -= contig_pages;
> +		__free_pages(pages[restored_idx], order);
> +	}

This looks wrong. These are 0-order pages. You can't free them at order.

I sent a patch to do exactly this a while ago [0]. At the time it was
rejected with the below argument.

    Hm, I am not sure if KHO should be responsible for freeing the
    restored pages. We don't know the content of those pages, and what
    they are used for. They could be used by a hypervisor or a device.
    Therefore, it may be better to keep them leaked, and let the caller
    decide what to do next: i.e., boot into a maintenance mode, crash the
    kernel, or allow the leak until the next reboot.

Although thinking about this again, why would any device or hypervisor
use vmalloc buffers? They should only be used for metadata. So perhaps
we take another look at my patch? Pasha, what do you think?

[0] https://lore.kernel.org/all/20251118181811.47336-1-pratyush@kernel.org/

>  	kvfree(pages);
>  	return NULL;
>  }

-- 
Regards,
Pratyush Yadav