Re: [RFC v2 05/16] luo: luo_core: integrate with KHO

[Pratyush Yadav <pratyush@kernel.org>]

On Sat, Jun 07 2025, Pasha Tatashin wrote:
[...]
>>
>> This weirdness happens because luo_prepare() and luo_cancel() control
>> the KHO state machine, but then also get controlled by it via the
>> notifier callbacks. So the relationship between then is not clear.
>> __luo_prepare() at least needs access to struct kho_serialization, so it
>> needs to come from the callback. So I don't have a clear way to clean
>> this all up off the top of my head.
>
> On production machine, without KHO_DEBUGFS, only LUO can control KHO
> state, but if debugfs is enabled, KHO can be finalized manually, and
> in this case LUO transitions to prepared state. In both cases, the
> path is identical. The KHO debugfs path is only for
> developers/debugging purposes.

What I meant is that even without KHO_DEBUGFS, LUO drives KHO, but then
KHO calls into LUO from the notifier, which makes the control flow
somewhat convoluted. If LUO is supposed to be the only thing that
interacts directly with KHO, maybe we should get rid of the notifier and
only let LUO drive things.

This can be done later though; it doesn't have to be in the initial
revision.

>
>> >  static int __init luo_startup(void)
>> >  {
>> > -     __luo_set_state(LIVEUPDATE_STATE_NORMAL);
>> > +     phys_addr_t fdt_phys;
>> > +     int ret;
>> > +
>> > +     if (!kho_is_enabled()) {
>> > +             if (luo_enabled)
>> > +                     pr_warn("Disabling liveupdate because KHO is disabled\n");
>> > +             luo_enabled = false;
>> > +             return 0;
>> > +     }
>> > +
>> > +     ret = register_kho_notifier(&luo_kho_notifier_nb);
>> > +     if (ret) {
>> > +             luo_enabled = false;
>>
>> You set luo_enabled to false here, but none of the LUO entry points like
>> luo_prepare() or luo_freeze() actually check it. So LUO will appear work
>> just fine even when it hasn't initialized properly.
>
> luo_enabled check was missing from luo_ioctl.c, as we should not
> create a device if LUO is not enabled. This is fixed.
>
>>
>> > +             pr_warn("Failed to register with KHO [%d]\n", ret);
>>
>> I guess you don't return here so a previous liveupdate can still be
>> recovered, even though we won't be able to make the next one. If so, a
>> comment would be nice to point this out.
>
> This is correct, but this is not going to work. Because, with the
> current change I am disabling "/dev/liveupdate" iff luo_enable ==
> false. Let's just return here, failing to register with KHO should not
> really happen, it usually means that there is another notifier with
> the same name has already registered.

Okay, fair enough.

-- 
Regards,
Pratyush Yadav