[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zr4J-U-HmmtxU-BB@pathway.suse.cz>
Date: Thu, 15 Aug 2024 16:00:25 +0200
From: Petr Mladek <pmladek@...e.com>
To: Miroslav Benes <mbenes@...e.cz>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>,
Joe Lawrence <joe.lawrence@...hat.com>,
Nicolai Stange <nstange@...e.de>, live-patching@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [POC 7/7] livepatching: Remove per-state version
On Thu 2024-07-25 16:16:44, Miroslav Benes wrote:
> On Fri, 10 Nov 2023, Petr Mladek wrote:
>
> > The livepatch state API was added to help with maintaining:
> >
> > + changes done by livepatch callbasks
> > + lifetime of shadow variables
> >
> > The original API was hard to use. Both objectives are better handled
> > by the new per-state callbacks. They are called when the state is
> > introduced or removed. There is also support for automatically freeing
> > obsolete shadow variables.
> >
> > The new callbacks changed the view of compatibility. The livepatch
> > can be replaced to any older one as long the current livepatch is
> > able to disable the obsolete state.
> >
> > As a result, the new patch does not need to support the currently
> > used states. The current patch will be able to disable them.
> >
> > The remaining question is what to do with the per-state version.
> > It was supposed to allow doing more modifications on an existing
> > state. The experience shows that it is not needed in practice.
> >
> > Well, it still might make sense to prevent downgrade when the state
> > could not be disabled easily or when the author does not want to
> > deal with it.
> >
> > Replace the per-state version with per-state block_disable flag.
> > It allows to handle several scenarios:
>
> I have no opinion to be honest. block_disable flag might be sufficient in
> the end.
>
> [...]
>
> > @@ -159,7 +159,9 @@ struct klp_state {
> > * @mod: reference to the live patch module
> > * @objs: object entries for kernel objects to be patched
> > * @states: system states that can get modified
> > + * version: livepatch version (optional)
> > * @replace: replace all actively used patches
> > + *
> > * @list: list node for global list of actively used patches
> > * @kobj: kobject for sysfs resources
> > * @obj_list: dynamic list of the object entries
> > @@ -173,6 +175,7 @@ struct klp_patch {
> > struct module *mod;
> > struct klp_object *objs;
> > struct klp_state *states;
> > + unsigned int version;
> > bool replace;
>
> Is it still needed then? What would be the use case?
Heh, I think that I actually wanted to remove the version completely.
This change is not mentioned in the changelog. And the version is
no longer used in the selftests.
I am going to remove it in the next version of the patchset.
> > /*
> > * Check that the new livepatch will not break the existing system states.
> > - * Cumulative patches must handle all already modified states.
> > - * Non-cumulative patches can touch already modified states.
> > + * The patch could replace existing patches only when the obsolete
> > + * states can be disabled.
> > */
> > bool klp_is_patch_compatible(struct klp_patch *patch)
> > {
> > struct klp_patch *old_patch;
> > struct klp_state *old_state;
> >
> > + /* Non-cumulative patches are always compatible. */
> > + if (!patch->replace)
> > + return true;
> > +
>
> Cumulative != atomic replace. Those are two different things.
I see. :-)
Best Regards,
Petr
Powered by blists - more mailing lists