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

[Miroslav Benes]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/