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.
> 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.
>
> Two small things below...
Agreed to both, thanks.
>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > ---
> > include/linux/livepatch.h | 15 ++++-----
> > kernel/livepatch/core.c | 79 +++++++++++++++++++++++------------------------
> > 2 files changed, 45 insertions(+), 49 deletions(-)
> >
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index 95023fd..22a67d1 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -28,11 +28,6 @@
> >
> > #include <asm/livepatch.h>
> >
> > -enum klp_state {
> > - KLP_DISABLED,
> > - KLP_ENABLED
> > -};
> > -
> > /**
> > * struct klp_func - function structure for live patching
> > * @old_name: name of the function to be patched
> > @@ -42,6 +37,7 @@ enum klp_state {
> > * @kobj: kobject for sysfs resources
> > * @state: tracks function-level patch application state
> > * @stack_node: list node for klp_ops func_stack list
> > + * @patched: the func has been added to the klp_ops list
> > */
> > struct klp_func {
> > /* external */
> > @@ -59,8 +55,8 @@ struct klp_func {
> >
> > /* internal */
> > struct kobject kobj;
> > - enum klp_state state;
> > struct list_head stack_node;
> > + int patched;
> > };
>
> @state remains in the comment above
>
> > /**
> > @@ -90,7 +86,7 @@ struct klp_reloc {
> > * @kobj: kobject for sysfs resources
> > * @mod: kernel module associated with the patched object
> > * (NULL for vmlinux)
> > - * @state: tracks object-level patch application state
> > + * @patched: the object's funcs have been add to the klp_ops list
> > */
> > struct klp_object {
> > /* external */
> > @@ -101,7 +97,7 @@ struct klp_object {
> > /* internal */
> > struct kobject *kobj;
> > struct module *mod;
> > - enum klp_state state;
> > + int patched;
> > };
> >
> > /**
> > @@ -111,6 +107,7 @@ struct klp_object {
> > * @list: list node for global list of registered patches
> > * @kobj: kobject for sysfs resources
> > * @state: tracks patch-level application state
> > + * @enabled: the patch is enabled (but operation may be incomplete)
> > */
> > struct klp_patch {
> > /* external */
> > @@ -120,7 +117,7 @@ struct klp_patch {
> > /* internal */
> > struct list_head list;
> > struct kobject kobj;
> > - enum klp_state state;
> > + int enabled;
> > };
>
> Dtto
>
> Miroslav
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
