On 1/15/25 03:24, Petr Mladek wrote: > This commit updates the livepatch documentation to reflect recent changes > in the behavior of states, callbacks, and shadow variables. > > Key changes include: > > - Per-state callbacks replace per-object callbacks, invoked only when a > livepatch introduces or removes a state. > - Shadow variable lifetime is now tied to the corresponding livepatch > state lifetime. > - The "version" field in `struct klp_state` has been replaced with the > "block_disable" flag for improved compatibility handling. > - The "data" field has been removed from `struct klp_state`; shadow > variables are now the recommended way to store state-related data. > > This update ensures the documentation accurately describes the current > livepatch functionality. > > Signed-off-by: Petr Mladek <pmla...@suse.com>
Hi Petr, I'm finally getting around to looking through these patches. I've made it as far as the selftest cleanups, then skipped ahead to the Documentation here. Before diving into code review, I just wanted to clarify some things for my own understanding. Please correct anything below that is incorrect. IMHO it is easy to step off the intended path here :D The original livepatch system states operated with a numeric .version field. New livepatches could only be loaded if providing a new set of states, or an equal-or-better version of states already loaded by existing livepatches. With that in mind, a livepatch state could be thought of as an indication of "a context needing special handling in a (versioned) way". Livepatches claiming to deal with a particular state, needed to do so with its latest or current versioning. A livepatch without a particular state was not bound to any restriction on that state, so nothing prevented subsequent atomic replace patches from blowing away existing states (those patches cleaned up their states on their disabling), or subsequent non-atomic replace patches from adding to the collective livepatch state. This patchset does away with .version and adds .block_disable. This is very different from versioning as prevents the associated state from ever going away. In an atomic-replace series of livepatches, this means once a state is introduced, all following patches must contain that state. In non-atomic-replace series, there is no restriction on subsequent patches, but the original patch introducing the state cannot ever be disabled/unloaded. (I'm not going to consider future hybrid mixed atomic/not use cases as my brain is already full.) Finally, the patchset adds .is_shadow and .callbacks. A short sequence of livepatches may look like: klp_patch A | klp_patch B .states[x] | .states[y] .id = 42 | .id = 42 .callbacks | .callbacks .block_disable | .block_disable .is_shadow | .is_shadow is there any harm or confusion if the two patches' state 42 contained disparate .callbacks, .block_disable, or .is_shadow contents? I /think/ this is allowed by the patchset (though I haven't gotten too deep in my understanding), but I feel that I'm starting to stretch my intuitive understanding of these livepatching states. Applying them to a series of atomic-replace livepatches is fairly straightforward. But then for a non-atomic-replace series, it starts getting weird as multiple callback sets will exist in multiple patches. In a perfect world, we could describe livepatching states absent callbacks and shadow variables. The documentation is a bit circular as one needs to understand them before fully grasping the purpose of the states. But to use them, you will first need to understand how to set them up in the states. :) Maybe there is no better way, but first I need to correct my understanding. Thanks, -- Joe