Tobin's patch[1] uncovered that the livepatching code handles kobjects
a too complicated way.
The first patch removes the unnecessary custom kobject state handling.
The second patch is an optional code deduplication. I did something
similar already when introducing the atomic replace. But it was
not considered worth it. There are more duplicated things now...
[1] https://lkml.kernel.org/r/[email protected]
Petr Mladek (2):
livepatch: Remove custom kobject state handling
livepatch: Remove duplicated code for early initialization
include/linux/livepatch.h | 3 --
kernel/livepatch/core.c | 86 ++++++++++++++++++++---------------------------
2 files changed, 37 insertions(+), 52 deletions(-)
--
2.16.4
kobject_init() call added one more operation that has to be
done when doing the early initialization of both static and
dynamic livepatch structures.
It would have been easier when the early initialization code
was not duplicated. Let's deduplicate it for future generations
of livepatching hackers.
The patch does not change the existing behavior.
Signed-off-by: Petr Mladek <[email protected]>
---
kernel/livepatch/core.c | 42 ++++++++++++++++++++++++++----------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 1ff91f7cbafb..0ec6ce8691b8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -426,10 +426,13 @@ static void klp_free_object_dynamic(struct klp_object *obj)
kfree(obj);
}
-static struct kobj_type klp_ktype_object;
-static struct kobj_type klp_ktype_func;
+static void klp_init_func_early(struct klp_object *obj,
+ struct klp_func *func);
+static void klp_init_object_early(struct klp_patch *patch,
+ struct klp_object *obj);
-static struct klp_object *klp_alloc_object_dynamic(const char *name)
+static struct klp_object *klp_alloc_object_dynamic(const char *name,
+ struct klp_patch *patch)
{
struct klp_object *obj;
@@ -445,8 +448,7 @@ static struct klp_object *klp_alloc_object_dynamic(const char *name)
}
}
- INIT_LIST_HEAD(&obj->func_list);
- kobject_init(&obj->kobj, &klp_ktype_object);
+ klp_init_object_early(patch, obj);
obj->dynamic = true;
return obj;
@@ -475,7 +477,7 @@ static struct klp_func *klp_alloc_func_nop(struct klp_func *old_func,
}
}
- kobject_init(&func->kobj, &klp_ktype_func);
+ klp_init_func_early(obj, func);
/*
* func->new_func is same as func->old_func. These addresses are
* set when the object is loaded, see klp_init_object_loaded().
@@ -495,11 +497,9 @@ static int klp_add_object_nops(struct klp_patch *patch,
obj = klp_find_object(patch, old_obj);
if (!obj) {
- obj = klp_alloc_object_dynamic(old_obj->name);
+ obj = klp_alloc_object_dynamic(old_obj->name, patch);
if (!obj)
return -ENOMEM;
-
- list_add_tail(&obj->node, &patch->obj_list);
}
klp_for_each_func(old_obj, old_func) {
@@ -510,8 +510,6 @@ static int klp_add_object_nops(struct klp_patch *patch,
func = klp_alloc_func_nop(old_func, obj);
if (!func)
return -ENOMEM;
-
- list_add_tail(&func->node, &obj->func_list);
}
return 0;
@@ -802,6 +800,21 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
return ret;
}
+static void klp_init_func_early(struct klp_object *obj,
+ struct klp_func *func)
+{
+ kobject_init(&func->kobj, &klp_ktype_func);
+ list_add_tail(&func->node, &obj->func_list);
+}
+
+static void klp_init_object_early(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ INIT_LIST_HEAD(&obj->func_list);
+ kobject_init(&obj->kobj, &klp_ktype_object);
+ list_add_tail(&obj->node, &patch->obj_list);
+}
+
static int klp_init_patch_early(struct klp_patch *patch)
{
struct klp_object *obj;
@@ -822,13 +835,10 @@ static int klp_init_patch_early(struct klp_patch *patch)
if (!obj->funcs)
return -EINVAL;
- INIT_LIST_HEAD(&obj->func_list);
- kobject_init(&obj->kobj, &klp_ktype_object);
- list_add_tail(&obj->node, &patch->obj_list);
+ klp_init_object_early(patch, obj);
klp_for_each_func_static(obj, func) {
- kobject_init(&func->kobj, &klp_ktype_func);
- list_add_tail(&func->node, &obj->func_list);
+ klp_init_func_early(obj, func);
}
}
--
2.16.4
On Fri, May 03, 2019 at 03:26:23PM +0200, Petr Mladek wrote:
> Tobin's patch[1] uncovered that the livepatching code handles kobjects
> a too complicated way.
>
> The first patch removes the unnecessary custom kobject state handling.
>
> The second patch is an optional code deduplication. I did something
> similar already when introducing the atomic replace. But it was
> not considered worth it. There are more duplicated things now...
>
> [1] https://lkml.kernel.org/r/[email protected]
>
>
> Petr Mladek (2):
> livepatch: Remove custom kobject state handling
> livepatch: Remove duplicated code for early initialization
>
> include/linux/livepatch.h | 3 --
> kernel/livepatch/core.c | 86 ++++++++++++++++++++---------------------------
> 2 files changed, 37 insertions(+), 52 deletions(-)
>
> --
> 2.16.4
>
For the series,
Acked-by: Joe Lawrence <[email protected]>
-- Joe
On Fri, May 03, 2019 at 03:26:25PM +0200, Petr Mladek wrote:
> kobject_init() call added one more operation that has to be
> done when doing the early initialization of both static and
> dynamic livepatch structures.
>
> It would have been easier when the early initialization code
> was not duplicated. Let's deduplicate it for future generations
> of livepatching hackers.
>
> The patch does not change the existing behavior.
>
> Signed-off-by: Petr Mladek <[email protected]>
Reviewed-by: Kamalesh Babulal <[email protected]>
--
Kamalesh
On Fri, 3 May 2019, Petr Mladek wrote:
> Tobin's patch[1] uncovered that the livepatching code handles kobjects
> a too complicated way.
>
> The first patch removes the unnecessary custom kobject state handling.
>
> The second patch is an optional code deduplication. I did something
> similar already when introducing the atomic replace. But it was
> not considered worth it. There are more duplicated things now...
>
> [1] https://lkml.kernel.org/r/[email protected]
>
>
> Petr Mladek (2):
> livepatch: Remove custom kobject state handling
> livepatch: Remove duplicated code for early initialization
I've applied this to for-5.2/core. Thanks,
--
Jiri Kosina
SUSE Labs