On Wed, Dec 10, 2014 at 11:30:16AM +0100, Petr Mladek wrote:
> On Tue 2014-12-09 12:51:41, Josh Poimboeuf wrote:
> > On Tue, Dec 09, 2014 at 07:05:06PM +0100, Petr Mladek wrote:
> > > This patch makes it clear what initialization and freeing steps need to be done
> > > when an object (module) is being loaded or removed. It will help to maintain
> > > the module coming and going handlers. Also it will remove duplicated
> > > code from these handlers.
> > >
> > > Signed-off-by: Petr Mladek <pmladek@xxxxxxx>
> > > ---
> > > kernel/livepatch/core.c | 92 ++++++++++++++++++++++++++++++++-----------------
> > > 1 file changed, 61 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > > index 97a8d4a3d6d8..fe312b9ada78 100644
> > > --- a/kernel/livepatch/core.c
> > > +++ b/kernel/livepatch/core.c
> > > @@ -590,6 +590,12 @@ static struct kobj_type klp_ktype_func = {
> > > .sysfs_ops = &kobj_sysfs_ops,
> > > };
> > >
> > > +/* Clean up when a patched object is unloaded */
> > > +static void klp_free_func_loaded(struct klp_func *func)
> > > +{
> > > + func->old_addr = 0;
> > > +}
> > > +
> > > /*
> > > * Free all functions' kobjects in the array up to some limit. When limit is
> > > * NULL, all kobjects are freed.
> > > @@ -603,6 +609,17 @@ static void klp_free_funcs_limited(struct klp_object *obj,
> > > kobject_put(&func->kobj);
> > > }
> > >
> > > +/* Clean up when a patched object is unloaded */
> > > +static void klp_free_object_loaded(struct klp_object *obj)
> > > +{
> > > + struct klp_func *func;
> > > +
> > > + obj->mod = NULL;
> > > +
> > > + for (func = obj->funcs; func->old_name; func++)
> > > + klp_free_func_loaded(func);
> > > +}
> > > +
> > > /*
> > > * Free all objects' kobjects in the array up to some limit. When limit is
> > > * NULL, all kobjects are freed.
> > > @@ -626,6 +643,12 @@ static void klp_free_patch(struct klp_patch *patch)
> > > kobject_put(&patch->kobj);
> > > }
> > >
> > > +/* parts of the initialization that is done only when the object is loaded */
> > > +static int klp_init_func_loaded(struct klp_object *obj, struct klp_func *func)
> > > +{
> > > + return klp_find_verify_func_addr(obj, func);
> > > +}
> > > +
> >
> > Creating a new function here for one line of code, which is only called
> > once, seems excessive, and makes the code harder to understand IMO.
>
> I see your point. Well, note that the split code is code is called
> twice from klp_init_object() and klp_module_coming(), so it helps
> to remove the code duplicity.
>
> Also it clearly separates the operations that are always done
> and that are done only when the object is loaded.
>
> If you suggest to call klp_find_verify_func_addr() directly
> from klp_init_object_loaded(), I am fine with it. We could always
> create it later if there are more operations needed for
> struct func.
Thanks, I'll merge the rest of this patch (except for the separate
klp_init_func_loaded and klp_free_func_loaded).
I should have v6 soon.
--
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
