Thread (57 messages) flat view 57 messages, 6 authors, 2025-08-06

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

From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2025-08-05 12:31:07
Also in: linux-doc, linux-fsdevel, linux-mm, lkml

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
quoted
+       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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help