On 05-17 20:20, Mike Rapoport wrote:
> 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 <[email protected]>
> > ---
> > 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...
The freeing happen during finish, that was by design.
>
> There were more comments by sashiko, didn't check them yet.
The other comment is stupid: yes, changing ABI prevents updating from
older kernel.
>
> > - 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.
