Re: [PATCH v2 10/32] liveupdate: luo_core: Live Update Orchestrator

[Jason Gunthorpe <jgg@nvidia.com>]

On Sun, Aug 03, 2025 at 09:11:20PM -0400, Pasha Tatashin wrote:

> Having a global state is necessary for performance optimizations. This
> is similar to why we export the state to userspace via sysfs: it
> allows other subsystems to behave differently during a
> performance-optimized live update versus a normal boot.
 
> For example, in our code base we have a driver that doesn't
> participate in the live update itself (it has no state to preserve).
> However, during boot, it checks this global state. If it's a live
> update boot, the driver skips certain steps, like loading firmware, to
> accelerate the overall boot time.

TBH, I'm against this. Give the driver a 0 byte state if it wants to
behave differently during live update. We should not be making
implicit things like this.

Plus the usual complaining about building core kernel infrastructure
around weird out of tree drivers.

If userspace wants a device to participate in live update, even just
"optimizations", then it has to opt in.

Frankly, this driver has no idea what the prior kernel did, and by
"optimizing" I think you are actually assuming that the prior kernel
had it bound to a normal kernel driver that left it in some
predictable configuration.

Vs say bound to VFIO and completely messed up.

So this should be represented by a LUO serialization that says "the
prior kernel left this device in well defined state X" even if it
takes 0 bytes to describe that state.

So no globals, there should be a way for a driver to tell if it is
participating in LUO, but not some global 'is luo' boot fla.g

> > +       ret = liveupdate_register_subsystem(&luo_file_subsys);
> > +       if (ret) {
> > +               pr_warn("Failed to register luo_file subsystem [%d]\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       if (liveupdate_state_updated()) {
> >
> > Thats going to be a standard pattern - I would expect that
> > liveupdate_register_subsystem() would do the check for updated and
> > then arrange to call back something like
> > liveupdate_subsystem.ops.post_update()
> >
> > And then post_update() would get the info that is currently under
> > liveupdate_get_subsystem_data() as arguments instead of having to make
> > more functions calls.
> >
> > Maybe even the fdt_node_check_compatible() can be hoisted.
> >
> > That would remove a bunch more liveupdate_state_updated() calls.
> 
> That's a good suggestion for a potential refactor. For now, the
> state-check call is inexpensive and is not in a performance-critical
> path. We can certainly implement this optimization later if it becomes
> necessary.

It is not an optimization, it is having a proper logical code
structure that doesn't rely on globals. I'm strongly against
sprinkling globals everywhere in the code when there are simple
logical APIs that entirely avoid it.

When I looked at where there were globals I didn't find any good
justifications, just thinks like this that are poor API design.

Jason