Re: [PATCH v2 10/13] kho: Update FDT dynamically for subtree addition/removal

Pasha Tatashin
2025-11-15T14:51:46Z

On Sat, Nov 15, 2025 at 4:40 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 01:59:59PM -0500, Pasha Tatashin wrote:
> > - struct kho_sub_fdt *sub_fdt;
> > + phys_addr_t phys = virt_to_phys(fdt);
> > + void *root_fdt = kho_out.fdt;
> > + int err = -ENOMEM;
> > + int off, fdt_err;
> >
> > - sub_fdt = kmalloc(sizeof(*sub_fdt), GFP_KERNEL);
> > - if (!sub_fdt)
> > - return -ENOMEM;
> > + guard(mutex)(&kho_out.lock);
> > +
> > + fdt_err = fdt_open_into(root_fdt, root_fdt, PAGE_SIZE);
> > + if (fdt_err < 0)
> > + return err;
> >
> > - INIT_LIST_HEAD(&sub_fdt->l);
> > - sub_fdt->name = name;
> > - sub_fdt->fdt = fdt;
> > + off = fdt_add_subnode(root_fdt, 0, name);
>
> Why not
> fdt_err = fdt_add_subnode()
>
> as I asked in v1 review?
>

Oh, I missed that, there is a slight difference between the two:
'fdt_err' only contains FDT return value, i.e. error if negative. The
'off' on the other hand in the happy path contains subnode offset, and
contains error only in the unhappy path. This is why I think it is a
little cleaner to keep different name, however, if you still prefer
re-using a single local variable for both, this is fix-up patch:

diff --git a/kernel/liveupdate/kexec_handover.c
b/kernel/liveupdate/kexec_handover.c
index 224bdf5becb6..81f60ccb2dc7 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -713,7 +713,7 @@ int kho_add_subtree(const char *name, void *fdt)
phys_addr_t phys = virt_to_phys(fdt);
void *root_fdt = kho_out.fdt;
int err = -ENOMEM;
- int off, fdt_err;
+ int fdt_err;

guard(mutex)(&kho_out.lock);

@@ -721,14 +721,14 @@ int kho_add_subtree(const char *name, void *fdt)
if (fdt_err < 0)
return err;

- off = fdt_add_subnode(root_fdt, 0, name);
- if (off < 0) {
- if (off == -FDT_ERR_EXISTS)
+ fdt_err = fdt_add_subnode(root_fdt, 0, name);
+ if (fdt_err < 0) {
+ if (fdt_err == -FDT_ERR_EXISTS)
err = -EEXIST;
goto out_pack;
}

- err = fdt_setprop(root_fdt, off, PROP_SUB_FDT, &phys, sizeof(phys));
+ err = fdt_setprop(root_fdt, fdt_err, PROP_SUB_FDT, &phys, sizeof(phys));
if (err < 0)
goto out_pack;