Re: [RFC PATCH 2/9] livepatch: separate enabled and patched states

Miroslav Benes
2015-02-13T14:46:18Z

On Fri, 13 Feb 2015, Josh Poimboeuf wrote:

> On Fri, Feb 13, 2015 at 01:57:38PM +0100, Miroslav Benes wrote:
> > On Mon, 9 Feb 2015, Josh Poimboeuf wrote:
> >
> > > Once we have a consistency model, patches and their objects will be
> > > enabled and disabled at different times. For example, when a patch is
> > > disabled, its loaded objects' funcs can remain registered with ftrace
> > > indefinitely until the unpatching operation is complete and they're no
> > > longer in use.
> > >
> > > It's less confusing if we give them different names: patches can be
> > > enabled or disabled; objects (and their funcs) can be patched or
> > > unpatched:
> > >
> > > - Enabled means that a patch is logically enabled (but not necessarily
> > > fully applied).
> > >
> > > - Patched means that an object's funcs are registered with ftrace and
> > > added to the klp_ops func stack.
> > >
> > > Also, since these states are binary, represent them with boolean-type
> > > variables instead of enums.
> >
> > They are binary now but will it hold also in the future? I cannot come up
> > with any other possible state of the function right now, but that doesn't
> > mean there isn't any. It would be sad to return it back to enums one day
> > :)
>
> I really can't think of any reason why they would become non-binary.
> IMO it's more likely we could add more boolean variables, but if that
> got out of hand we could just switch to using bit flags.
>
> Either way I don't see a problem with changing them later if we need to.

Agreed.

> > Also would it be useful to expose patched variable for functions and
> > objects in sysfs?
>
> Not that I know of. Do you have a use case in mind? I view "patched"
> as an internal variable, corresponding to whether the object or its
> functions are registered with ftrace/klp_ops. It doesn't mean "patched"
> in a way that would really make sense to the user, because of the
> gradual nature of the patching process.

The only reasonable thing which I thought about was in case of an error.
If something bad happens it could be useful to know which state the functions
are in (patched/unpatched). Anyway it is nothing of importance right now
and we can add it anytime later if we decide it useful.

Miroslav