Re: [PATCH v2 01/10] liveupdate: centralize state management into struct luo_ser

[Mike Rapoport <rppt@kernel.org>]

On Thu, May 14, 2026 at 10:26:19PM +0000, Pasha Tatashin wrote:
> Transition the LUO to ABI v2, which centralizes state management into a
> single struct luo_ser header.
> 
> Previously, LUO state was spread across multiple FDT properties and
> subnodes. ABI v2 simplifies this by placing all core state, including
> the liveupdate number and physical addresses for sessions and FLB
> headers into a centralized struct luo_ser.
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  include/linux/kho/abi/luo.h      | 91 +++++++++++---------------------
>  kernel/liveupdate/luo_core.c     | 59 ++++++++++++++-------
>  kernel/liveupdate/luo_flb.c      | 65 ++++-------------------
>  kernel/liveupdate/luo_internal.h |  8 +--
>  kernel/liveupdate/luo_session.c  | 57 +++-----------------
>  5 files changed, 93 insertions(+), 187 deletions(-)

This looks lovely :)

> @@ -115,27 +117,29 @@ static int __init luo_early_startup(void)
>  		return -EINVAL;
>  	}
>  
> -	ln_size = 0;
> -	ptr = fdt_getprop(luo_global.fdt_in, 0, LUO_FDT_LIVEUPDATE_NUM,
> -			  &ln_size);
> -	if (!ptr || ln_size != sizeof(luo_global.liveupdate_num)) {
> -		pr_err("Unable to get live update number '%s' [%d]\n",
> -		       LUO_FDT_LIVEUPDATE_NUM, ln_size);
> +	header_size = 0;
> +	ptr = fdt_getprop(luo_global.fdt_in, 0, LUO_FDT_ABI_HEADER, &header_size);
> +	if (!ptr || header_size != sizeof(u64)) {
> +		pr_err("Unable to get ABI header '%s' [%d]\n",
> +		       LUO_FDT_ABI_HEADER, header_size);
>  
>  		return -EINVAL;
>  	}
>  
> -	luo_global.liveupdate_num = get_unaligned((u64 *)ptr);
> +	luo_ser_pa = get_unaligned((u64 *)ptr);
> +	luo_ser = phys_to_virt(luo_ser_pa);
> +
> +	luo_global.liveupdate_num = luo_ser->liveupdate_num;
>  	pr_info("Retrieved live update data, liveupdate number: %lld\n",
>  		luo_global.liveupdate_num);
>  
> -	err = luo_session_setup_incoming(luo_global.fdt_in);
> +	err = luo_session_setup_incoming(luo_ser->sessions_pa);
>  	if (err)
>  		return err;
>  
> -	err = luo_flb_setup_incoming(luo_global.fdt_in);
> +	luo_flb_setup_incoming(luo_ser->flbs_pa);

Sashiko asks: 

  Is there a leak of the preserved luo_ser memory here?

  The outgoing kernel allocates the header using kho_alloc_preserve(), but
  luo_early_startup() returns without calling kho_restore_free() on the
  luo_ser pointer once the fields have been processed. This results in an
  unreleased memory reservation on every successful live update.

This seems preexisting, and we probably don't care enough, but still...

There were more comments by sashiko, didn't check them yet.
  
> -	return err;
> +	return 0;
>  }
>  
>  static int __init liveupdate_early_init(void)
> @@ -156,7 +160,8 @@ early_initcall(liveupdate_early_init);
>  /* Called during boot to create outgoing LUO fdt tree */
>  static int __init luo_fdt_setup(void)
>  {
> -	const u64 ln = luo_global.liveupdate_num + 1;
> +	struct luo_ser *luo_ser;
> +	u64 luo_ser_pa;
>  	void *fdt_out;
>  	int err;
>  
> @@ -166,27 +171,45 @@ static int __init luo_fdt_setup(void)
>  		return PTR_ERR(fdt_out);
>  	}
>  
> +	luo_ser = kho_alloc_preserve(sizeof(*luo_ser));
> +	if (IS_ERR(luo_ser)) {
> +		err = PTR_ERR(luo_ser);
> +		goto exit_free_fdt;
> +	}
> +	luo_ser_pa = virt_to_phys(luo_ser);
> +
>  	err = fdt_create(fdt_out, LUO_FDT_SIZE);
>  	err |= fdt_finish_reservemap(fdt_out);
>  	err |= fdt_begin_node(fdt_out, "");
>  	err |= fdt_property_string(fdt_out, "compatible", LUO_FDT_COMPATIBLE);
> -	err |= fdt_property(fdt_out, LUO_FDT_LIVEUPDATE_NUM, &ln, sizeof(ln));
> -	err |= luo_session_setup_outgoing(fdt_out);
> -	err |= luo_flb_setup_outgoing(fdt_out);
> +	err |= fdt_property(fdt_out, LUO_FDT_ABI_HEADER, &luo_ser_pa,
> +			    sizeof(luo_ser_pa));
>  	err |= fdt_end_node(fdt_out);
>  	err |= fdt_finish(fdt_out);
>  	if (err)
> -		goto exit_free;
> +		goto exit_free_luo_ser;
> +
> +	err = luo_session_setup_outgoing(&luo_ser->sessions_pa);
> +	if (err)
> +		goto exit_free_luo_ser;
> +
> +	err = luo_flb_setup_outgoing(&luo_ser->flbs_pa);
> +	if (err)
> +		goto exit_free_luo_ser;
> +
> +	luo_ser->liveupdate_num = luo_global.liveupdate_num + 1;
>  
>  	err = kho_add_subtree(LUO_FDT_KHO_ENTRY_NAME, fdt_out,
>  			      fdt_totalsize(fdt_out));
>  	if (err)
> -		goto exit_free;
> +		goto exit_free_luo_ser;

And here we also seem to leak memory allocated by sesttion/flb setup.

>  	luo_global.fdt_out = fdt_out;
>  
>  	return 0;
>  
> -exit_free:
> +exit_free_luo_ser:
> +	kho_unpreserve_free(luo_ser);
> +exit_free_fdt:
>  	kho_unpreserve_free(fdt_out);
>  	pr_err("failed to prepare LUO FDT: %d\n", err);

-- 
Sincerely yours,
Mike.