[PATCH] kho: cleanup error handling in kho_populate()

Mike Rapoport posted 1 patch 2 weeks, 2 days ago
kernel/liveupdate/kexec_handover.c | 39 +++++++++++++-----------------
1 file changed, 17 insertions(+), 22 deletions(-)
[PATCH] kho: cleanup error handling in kho_populate()
Posted by Mike Rapoport 2 weeks, 2 days ago
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

* use dedicated labels for error handling instead of checking if a pointer
  is not null to decide if it should be unmapped
* drop assignment of values to err that are only used to print a numeric
  error code, there are pr_warn()s for each failure already so printing a
  numeric error code in the next line does not add anything useful

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 kernel/liveupdate/kexec_handover.c | 39 +++++++++++++-----------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
index d4482b6e3cae..e248ce2e04b7 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -1441,46 +1441,40 @@ void __init kho_memory_init(void)
 void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
 			 phys_addr_t scratch_phys, u64 scratch_len)
 {
+	unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
 	struct kho_scratch *scratch = NULL;
 	phys_addr_t mem_map_phys;
 	void *fdt = NULL;
-	int err = 0;
-	unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
+	int err;
 
 	/* Validate the input FDT */
 	fdt = early_memremap(fdt_phys, fdt_len);
 	if (!fdt) {
 		pr_warn("setup: failed to memremap FDT (0x%llx)\n", fdt_phys);
-		err = -EFAULT;
-		goto out;
+		goto err_report;
 	}
 	err = fdt_check_header(fdt);
 	if (err) {
 		pr_warn("setup: handover FDT (0x%llx) is invalid: %d\n",
 			fdt_phys, err);
-		err = -EINVAL;
-		goto out;
+		goto err_unmap_fdt;
 	}
 	err = fdt_node_check_compatible(fdt, 0, KHO_FDT_COMPATIBLE);
 	if (err) {
 		pr_warn("setup: handover FDT (0x%llx) is incompatible with '%s': %d\n",
 			fdt_phys, KHO_FDT_COMPATIBLE, err);
-		err = -EINVAL;
-		goto out;
+		goto err_unmap_fdt;
 	}
 
 	mem_map_phys = kho_get_mem_map_phys(fdt);
-	if (!mem_map_phys) {
-		err = -ENOENT;
-		goto out;
-	}
+	if (!mem_map_phys)
+		goto err_unmap_fdt;
 
 	scratch = early_memremap(scratch_phys, scratch_len);
 	if (!scratch) {
 		pr_warn("setup: failed to memremap scratch (phys=0x%llx, len=%lld)\n",
 			scratch_phys, scratch_len);
-		err = -EFAULT;
-		goto out;
+		goto err_unmap_fdt;
 	}
 
 	/*
@@ -1497,7 +1491,7 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
 		if (WARN_ON(err)) {
 			pr_warn("failed to mark the scratch region 0x%pa+0x%pa: %pe",
 				&area->addr, &size, ERR_PTR(err));
-			goto out;
+			goto err_unmap_scratch;
 		}
 		pr_debug("Marked 0x%pa+0x%pa as scratch", &area->addr, &size);
 	}
@@ -1519,13 +1513,14 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
 	kho_scratch_cnt = scratch_cnt;
 	pr_info("found kexec handover data.\n");
 
-out:
-	if (fdt)
-		early_memunmap(fdt, fdt_len);
-	if (scratch)
-		early_memunmap(scratch, scratch_len);
-	if (err)
-		pr_warn("disabling KHO revival: %d\n", err);
+	return;
+
+err_unmap_scratch:
+	early_memunmap(scratch, scratch_len);
+err_unmap_fdt:
+	early_memunmap(fdt, fdt_len);
+err_report:
+	pr_warn("disabling KHO revival\n");
 }
 
 /* Helper functions for kexec_file_load */

base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
-- 
2.51.0
Re: [PATCH] kho: cleanup error handling in kho_populate()
Posted by Pratyush Yadav 1 week, 4 days ago
On Thu, Jan 22 2026, Mike Rapoport wrote:

> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> * use dedicated labels for error handling instead of checking if a pointer
>   is not null to decide if it should be unmapped
> * drop assignment of values to err that are only used to print a numeric
>   error code, there are pr_warn()s for each failure already so printing a
>   numeric error code in the next line does not add anything useful
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

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

[...]

-- 
Regards,
Pratyush Yadav
Re: [PATCH] kho: cleanup error handling in kho_populate()
Posted by Pasha Tatashin 2 weeks, 2 days ago
On Thu, Jan 22, 2026 at 7:18 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> * use dedicated labels for error handling instead of checking if a pointer
>   is not null to decide if it should be unmapped
> * drop assignment of values to err that are only used to print a numeric
>   error code, there are pr_warn()s for each failure already so printing a
>   numeric error code in the next line does not add anything useful
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  kernel/liveupdate/kexec_handover.c | 39 +++++++++++++-----------------
>  1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index d4482b6e3cae..e248ce2e04b7 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1441,46 +1441,40 @@ void __init kho_memory_init(void)
>  void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>                          phys_addr_t scratch_phys, u64 scratch_len)
>  {
> +       unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
>         struct kho_scratch *scratch = NULL;
>         phys_addr_t mem_map_phys;
>         void *fdt = NULL;
> -       int err = 0;
> -       unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
> +       int err;
>
>         /* Validate the input FDT */
>         fdt = early_memremap(fdt_phys, fdt_len);
>         if (!fdt) {
>                 pr_warn("setup: failed to memremap FDT (0x%llx)\n", fdt_phys);
> -               err = -EFAULT;
> -               goto out;
> +               goto err_report;
>         }
>         err = fdt_check_header(fdt);
>         if (err) {
>                 pr_warn("setup: handover FDT (0x%llx) is invalid: %d\n",
>                         fdt_phys, err);
> -               err = -EINVAL;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>         err = fdt_node_check_compatible(fdt, 0, KHO_FDT_COMPATIBLE);
>         if (err) {
>                 pr_warn("setup: handover FDT (0x%llx) is incompatible with '%s': %d\n",
>                         fdt_phys, KHO_FDT_COMPATIBLE, err);
> -               err = -EINVAL;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>
>         mem_map_phys = kho_get_mem_map_phys(fdt);
> -       if (!mem_map_phys) {
> -               err = -ENOENT;
> -               goto out;
> -       }
> +       if (!mem_map_phys)
> +               goto err_unmap_fdt;
>
>         scratch = early_memremap(scratch_phys, scratch_len);
>         if (!scratch) {
>                 pr_warn("setup: failed to memremap scratch (phys=0x%llx, len=%lld)\n",
>                         scratch_phys, scratch_len);
> -               err = -EFAULT;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>
>         /*
> @@ -1497,7 +1491,7 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>                 if (WARN_ON(err)) {
>                         pr_warn("failed to mark the scratch region 0x%pa+0x%pa: %pe",
>                                 &area->addr, &size, ERR_PTR(err));
> -                       goto out;
> +                       goto err_unmap_scratch;
>                 }
>                 pr_debug("Marked 0x%pa+0x%pa as scratch", &area->addr, &size);
>         }
> @@ -1519,13 +1513,14 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>         kho_scratch_cnt = scratch_cnt;
>         pr_info("found kexec handover data.\n");
>
> -out:
> -       if (fdt)
> -               early_memunmap(fdt, fdt_len);
> -       if (scratch)
> -               early_memunmap(scratch, scratch_len);
> -       if (err)
> -               pr_warn("disabling KHO revival: %d\n", err);
> +       return;
> +
> +err_unmap_scratch:
> +       early_memunmap(scratch, scratch_len);
> +err_unmap_fdt:
> +       early_memunmap(fdt, fdt_len);
> +err_report:
> +       pr_warn("disabling KHO revival\n");

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Nice clean-up, thanks.

Pasha
Re: [PATCH] kho: cleanup error handling in kho_populate()
Posted by Mike Rapoport 2 weeks, 2 days ago
(cc the correct Pratyush's address)

On Thu, Jan 22, 2026 at 02:17:57PM +0200, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> 
> * use dedicated labels for error handling instead of checking if a pointer
>   is not null to decide if it should be unmapped
> * drop assignment of values to err that are only used to print a numeric
>   error code, there are pr_warn()s for each failure already so printing a
>   numeric error code in the next line does not add anything useful
> 
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  kernel/liveupdate/kexec_handover.c | 39 +++++++++++++-----------------
>  1 file changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index d4482b6e3cae..e248ce2e04b7 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1441,46 +1441,40 @@ void __init kho_memory_init(void)
>  void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>  			 phys_addr_t scratch_phys, u64 scratch_len)
>  {
> +	unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
>  	struct kho_scratch *scratch = NULL;
>  	phys_addr_t mem_map_phys;
>  	void *fdt = NULL;
> -	int err = 0;
> -	unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
> +	int err;
>  
>  	/* Validate the input FDT */
>  	fdt = early_memremap(fdt_phys, fdt_len);
>  	if (!fdt) {
>  		pr_warn("setup: failed to memremap FDT (0x%llx)\n", fdt_phys);
> -		err = -EFAULT;
> -		goto out;
> +		goto err_report;
>  	}
>  	err = fdt_check_header(fdt);
>  	if (err) {
>  		pr_warn("setup: handover FDT (0x%llx) is invalid: %d\n",
>  			fdt_phys, err);
> -		err = -EINVAL;
> -		goto out;
> +		goto err_unmap_fdt;
>  	}
>  	err = fdt_node_check_compatible(fdt, 0, KHO_FDT_COMPATIBLE);
>  	if (err) {
>  		pr_warn("setup: handover FDT (0x%llx) is incompatible with '%s': %d\n",
>  			fdt_phys, KHO_FDT_COMPATIBLE, err);
> -		err = -EINVAL;
> -		goto out;
> +		goto err_unmap_fdt;
>  	}
>  
>  	mem_map_phys = kho_get_mem_map_phys(fdt);
> -	if (!mem_map_phys) {
> -		err = -ENOENT;
> -		goto out;
> -	}
> +	if (!mem_map_phys)
> +		goto err_unmap_fdt;
>  
>  	scratch = early_memremap(scratch_phys, scratch_len);
>  	if (!scratch) {
>  		pr_warn("setup: failed to memremap scratch (phys=0x%llx, len=%lld)\n",
>  			scratch_phys, scratch_len);
> -		err = -EFAULT;
> -		goto out;
> +		goto err_unmap_fdt;
>  	}
>  
>  	/*
> @@ -1497,7 +1491,7 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>  		if (WARN_ON(err)) {
>  			pr_warn("failed to mark the scratch region 0x%pa+0x%pa: %pe",
>  				&area->addr, &size, ERR_PTR(err));
> -			goto out;
> +			goto err_unmap_scratch;
>  		}
>  		pr_debug("Marked 0x%pa+0x%pa as scratch", &area->addr, &size);
>  	}
> @@ -1519,13 +1513,14 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>  	kho_scratch_cnt = scratch_cnt;
>  	pr_info("found kexec handover data.\n");
>  
> -out:
> -	if (fdt)
> -		early_memunmap(fdt, fdt_len);
> -	if (scratch)
> -		early_memunmap(scratch, scratch_len);
> -	if (err)
> -		pr_warn("disabling KHO revival: %d\n", err);
> +	return;
> +
> +err_unmap_scratch:
> +	early_memunmap(scratch, scratch_len);
> +err_unmap_fdt:
> +	early_memunmap(fdt, fdt_len);
> +err_report:
> +	pr_warn("disabling KHO revival\n");
>  }
>  
>  /* Helper functions for kexec_file_load */
> 
> base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
> -- 
> 2.51.0
> 

-- 
Sincerely yours,
Mike.