LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code
@ 2019-05-03 13:26 Petr Mladek
  2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Petr Mladek @ 2019-05-03 13:26 UTC (permalink / raw)
  To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
  Cc: Joe Lawrence, Kamalesh Babulal, Tobin C . Harding,
	Greg Kroah-Hartman, live-patching, linux-kernel, Petr Mladek

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/20190430001534.26246-1-tobin@kernel.org


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] livepatch: Remove custom kobject state handling
  2019-05-03 13:26 [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Petr Mladek
@ 2019-05-03 13:26 ` Petr Mladek
  2019-05-03 16:35   ` Kamalesh Babulal
  2019-05-07 12:32   ` Miroslav Benes
  2019-05-03 13:26 ` [PATCH 2/2] livepatch: Remove duplicated code for early initialization Petr Mladek
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 9+ messages in thread
From: Petr Mladek @ 2019-05-03 13:26 UTC (permalink / raw)
  To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
  Cc: Joe Lawrence, Kamalesh Babulal, Tobin C . Harding,
	Greg Kroah-Hartman, live-patching, linux-kernel, Petr Mladek

kobject_init() always succeeds and sets the reference count to 1.
It allows to always free the structures via kobject_put() and
the related release callback.

Note that the custom kobject state handling was used only
because we did not know that kobject_put() can and actually
should get called even when kobject_init_and_add() fails.

The patch should not change the existing behavior.

Suggested-by: "Tobin C. Harding" <tobin@kernel.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/livepatch.h |  3 ---
 kernel/livepatch/core.c   | 56 ++++++++++++++---------------------------------
 2 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 53551f470722..a14bab1a0a3e 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -86,7 +86,6 @@ struct klp_func {
 	struct list_head node;
 	struct list_head stack_node;
 	unsigned long old_size, new_size;
-	bool kobj_added;
 	bool nop;
 	bool patched;
 	bool transition;
@@ -141,7 +140,6 @@ struct klp_object {
 	struct list_head func_list;
 	struct list_head node;
 	struct module *mod;
-	bool kobj_added;
 	bool dynamic;
 	bool patched;
 };
@@ -170,7 +168,6 @@ struct klp_patch {
 	struct list_head list;
 	struct kobject kobj;
 	struct list_head obj_list;
-	bool kobj_added;
 	bool enabled;
 	bool forced;
 	struct work_struct free_work;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index eb0ee10a1981..1ff91f7cbafb 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -426,6 +426,9 @@ 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 struct klp_object *klp_alloc_object_dynamic(const char *name)
 {
 	struct klp_object *obj;
@@ -443,6 +446,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);
 	obj->dynamic = true;
 
 	return obj;
@@ -471,6 +475,7 @@ static struct klp_func *klp_alloc_func_nop(struct klp_func *old_func,
 		}
 	}
 
+	kobject_init(&func->kobj, &klp_ktype_func);
 	/*
 	 * func->new_func is same as func->old_func. These addresses are
 	 * set when the object is loaded, see klp_init_object_loaded().
@@ -588,13 +593,7 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only)
 			continue;
 
 		list_del(&func->node);
-
-		/* Might be called from klp_init_patch() error path. */
-		if (func->kobj_added) {
-			kobject_put(&func->kobj);
-		} else if (func->nop) {
-			klp_free_func_nop(func);
-		}
+		kobject_put(&func->kobj);
 	}
 }
 
@@ -624,13 +623,7 @@ static void __klp_free_objects(struct klp_patch *patch, bool nops_only)
 			continue;
 
 		list_del(&obj->node);
-
-		/* Might be called from klp_init_patch() error path. */
-		if (obj->kobj_added) {
-			kobject_put(&obj->kobj);
-		} else if (obj->dynamic) {
-			klp_free_object_dynamic(obj);
-		}
+		kobject_put(&obj->kobj);
 	}
 }
 
@@ -675,10 +668,8 @@ static void klp_free_patch_finish(struct klp_patch *patch)
 	 * this is called when the patch gets disabled and it
 	 * cannot get enabled again.
 	 */
-	if (patch->kobj_added) {
-		kobject_put(&patch->kobj);
-		wait_for_completion(&patch->finish);
-	}
+	kobject_put(&patch->kobj);
+	wait_for_completion(&patch->finish);
 
 	/* Put the module after the last access to struct klp_patch. */
 	if (!patch->forced)
@@ -700,8 +691,6 @@ static void klp_free_patch_work_fn(struct work_struct *work)
 
 static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 {
-	int ret;
-
 	if (!func->old_name)
 		return -EINVAL;
 
@@ -724,13 +713,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 	 * object. If the user selects 0 for old_sympos, then 1 will be used
 	 * since a unique symbol will be the first occurrence.
 	 */
-	ret = kobject_init_and_add(&func->kobj, &klp_ktype_func,
-				   &obj->kobj, "%s,%lu", func->old_name,
-				   func->old_sympos ? func->old_sympos : 1);
-	if (!ret)
-		func->kobj_added = true;
-
-	return ret;
+	return kobject_add(&func->kobj, &obj->kobj, "%s,%lu",
+			   func->old_name,
+			   func->old_sympos ? func->old_sympos : 1);
 }
 
 /* Arches may override this to finish any remaining arch-specific tasks */
@@ -801,11 +786,9 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
 	klp_find_object_module(obj);
 
 	name = klp_is_module(obj) ? obj->name : "vmlinux";
-	ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object,
-				   &patch->kobj, "%s", name);
+	ret = kobject_add(&obj->kobj, &patch->kobj, "%s", name);
 	if (ret)
 		return ret;
-	obj->kobj_added = true;
 
 	klp_for_each_func(obj, func) {
 		ret = klp_init_func(obj, func);
@@ -829,7 +812,7 @@ static int klp_init_patch_early(struct klp_patch *patch)
 
 	INIT_LIST_HEAD(&patch->list);
 	INIT_LIST_HEAD(&patch->obj_list);
-	patch->kobj_added = false;
+	kobject_init(&patch->kobj, &klp_ktype_patch);
 	patch->enabled = false;
 	patch->forced = false;
 	INIT_WORK(&patch->free_work, klp_free_patch_work_fn);
@@ -840,11 +823,11 @@ static int klp_init_patch_early(struct klp_patch *patch)
 			return -EINVAL;
 
 		INIT_LIST_HEAD(&obj->func_list);
-		obj->kobj_added = false;
+		kobject_init(&obj->kobj, &klp_ktype_object);
 		list_add_tail(&obj->node, &patch->obj_list);
 
 		klp_for_each_func_static(obj, func) {
-			func->kobj_added = false;
+			kobject_init(&func->kobj, &klp_ktype_func);
 			list_add_tail(&func->node, &obj->func_list);
 		}
 	}
@@ -860,11 +843,9 @@ static int klp_init_patch(struct klp_patch *patch)
 	struct klp_object *obj;
 	int ret;
 
-	ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
-				   klp_root_kobj, "%s", patch->mod->name);
+	ret = kobject_add(&patch->kobj, klp_root_kobj, "%s", patch->mod->name);
 	if (ret)
 		return ret;
-	patch->kobj_added = true;
 
 	if (patch->replace) {
 		ret = klp_add_nops(patch);
@@ -926,9 +907,6 @@ static int __klp_enable_patch(struct klp_patch *patch)
 	if (WARN_ON(patch->enabled))
 		return -EINVAL;
 
-	if (!patch->kobj_added)
-		return -EINVAL;
-
 	pr_notice("enabling patch '%s'\n", patch->mod->name);
 
 	klp_init_transition(patch, KLP_PATCHED);
-- 
2.16.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] livepatch: Remove duplicated code for early initialization
  2019-05-03 13:26 [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Petr Mladek
  2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
@ 2019-05-03 13:26 ` Petr Mladek
  2019-05-03 16:35   ` Kamalesh Babulal
  2019-05-03 15:39 ` [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Joe Lawrence
  2019-05-03 19:16 ` Jiri Kosina
  3 siblings, 1 reply; 9+ messages in thread
From: Petr Mladek @ 2019-05-03 13:26 UTC (permalink / raw)
  To: Jiri Kosina, Josh Poimboeuf, Miroslav Benes
  Cc: Joe Lawrence, Kamalesh Babulal, Tobin C . Harding,
	Greg Kroah-Hartman, live-patching, linux-kernel, Petr Mladek

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 <pmladek@suse.com>
---
 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code
  2019-05-03 13:26 [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Petr Mladek
  2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
  2019-05-03 13:26 ` [PATCH 2/2] livepatch: Remove duplicated code for early initialization Petr Mladek
@ 2019-05-03 15:39 ` Joe Lawrence
  2019-05-03 19:16 ` Jiri Kosina
  3 siblings, 0 replies; 9+ messages in thread
From: Joe Lawrence @ 2019-05-03 15:39 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Jiri Kosina, Josh Poimboeuf, Miroslav Benes, Kamalesh Babulal,
	Tobin C . Harding, Greg Kroah-Hartman, live-patching,
	linux-kernel

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/20190430001534.26246-1-tobin@kernel.org
> 
> 
> 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 <joe.lawrence@redhat.com>

-- Joe

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] livepatch: Remove custom kobject state handling
  2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
@ 2019-05-03 16:35   ` Kamalesh Babulal
  2019-05-07 12:32   ` Miroslav Benes
  1 sibling, 0 replies; 9+ messages in thread
From: Kamalesh Babulal @ 2019-05-03 16:35 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Jiri Kosina, Josh Poimboeuf, Miroslav Benes, Joe Lawrence,
	Tobin C . Harding, Greg Kroah-Hartman, live-patching,
	linux-kernel

On Fri, May 03, 2019 at 03:26:24PM +0200, Petr Mladek wrote:
> kobject_init() always succeeds and sets the reference count to 1.
> It allows to always free the structures via kobject_put() and
> the related release callback.
> 
> Note that the custom kobject state handling was used only
> because we did not know that kobject_put() can and actually
> should get called even when kobject_init_and_add() fails.
> 
> The patch should not change the existing behavior.
> 
> Suggested-by: "Tobin C. Harding" <tobin@kernel.org>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  include/linux/livepatch.h |  3 ---
>  kernel/livepatch/core.c   | 56 ++++++++++++++---------------------------------
>  2 files changed, 17 insertions(+), 42 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 53551f470722..a14bab1a0a3e 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -86,7 +86,6 @@ struct klp_func {
>  	struct list_head node;
>  	struct list_head stack_node;
>  	unsigned long old_size, new_size;
> -	bool kobj_added;
>  	bool nop;
>  	bool patched;
>  	bool transition;

Minor nitpick, the description of kobj_added needs to be removed from
structure descriptions. 

Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>

-- 
Kamalesh


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] livepatch: Remove duplicated code for early initialization
  2019-05-03 13:26 ` [PATCH 2/2] livepatch: Remove duplicated code for early initialization Petr Mladek
@ 2019-05-03 16:35   ` Kamalesh Babulal
  0 siblings, 0 replies; 9+ messages in thread
From: Kamalesh Babulal @ 2019-05-03 16:35 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Jiri Kosina, Josh Poimboeuf, Miroslav Benes, Joe Lawrence,
	Tobin C . Harding, Greg Kroah-Hartman, live-patching,
	linux-kernel

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 <pmladek@suse.com>

Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>

-- 
Kamalesh


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code
  2019-05-03 13:26 [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Petr Mladek
                   ` (2 preceding siblings ...)
  2019-05-03 15:39 ` [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Joe Lawrence
@ 2019-05-03 19:16 ` Jiri Kosina
  3 siblings, 0 replies; 9+ messages in thread
From: Jiri Kosina @ 2019-05-03 19:16 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Josh Poimboeuf, Miroslav Benes, Joe Lawrence, Kamalesh Babulal,
	Tobin C . Harding, Greg Kroah-Hartman, live-patching,
	linux-kernel

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/20190430001534.26246-1-tobin@kernel.org
> 
> 
> 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] livepatch: Remove custom kobject state handling
  2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
  2019-05-03 16:35   ` Kamalesh Babulal
@ 2019-05-07 12:32   ` Miroslav Benes
  2019-05-07 13:06     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 9+ messages in thread
From: Miroslav Benes @ 2019-05-07 12:32 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Jiri Kosina, Josh Poimboeuf, Joe Lawrence, Kamalesh Babulal,
	Tobin C . Harding, Greg Kroah-Hartman, live-patching,
	linux-kernel

On Fri, 3 May 2019, Petr Mladek wrote:

> kobject_init() always succeeds and sets the reference count to 1.
> It allows to always free the structures via kobject_put() and
> the related release callback.
> 
> Note that the custom kobject state handling was used only
> because we did not know that kobject_put() can and actually
> should get called even when kobject_init_and_add() fails.
> 
> The patch should not change the existing behavior.

Pity that the changelog does not describe the change from 
kobject_init_and_add() to two-stage kobject init (separate kobject_init() 
and kobject_add()).

Petr changed it, because now each member of new dynamic lists (created in 
klp_init_patch_early()) is initialized with kobject_init(), so we do not 
have to worry about calling kobject_put() (this is slightly different from 
kobj_added).

It would also be possible to retain kobject_init_and_add() and move it to 
klp_init_patch_early(), but it would be uglier in my opinion.

Miroslav

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] livepatch: Remove custom kobject state handling
  2019-05-07 12:32   ` Miroslav Benes
@ 2019-05-07 13:06     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-07 13:06 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Petr Mladek, Jiri Kosina, Josh Poimboeuf, Joe Lawrence,
	Kamalesh Babulal, Tobin C . Harding, live-patching, linux-kernel

On Tue, May 07, 2019 at 02:32:57PM +0200, Miroslav Benes wrote:
> On Fri, 3 May 2019, Petr Mladek wrote:
> 
> > kobject_init() always succeeds and sets the reference count to 1.
> > It allows to always free the structures via kobject_put() and
> > the related release callback.
> > 
> > Note that the custom kobject state handling was used only
> > because we did not know that kobject_put() can and actually
> > should get called even when kobject_init_and_add() fails.
> > 
> > The patch should not change the existing behavior.
> 
> Pity that the changelog does not describe the change from 
> kobject_init_and_add() to two-stage kobject init (separate kobject_init() 
> and kobject_add()).
> 
> Petr changed it, because now each member of new dynamic lists (created in 
> klp_init_patch_early()) is initialized with kobject_init(), so we do not 
> have to worry about calling kobject_put() (this is slightly different from 
> kobj_added).
> 
> It would also be possible to retain kobject_init_and_add() and move it to 
> klp_init_patch_early(), but it would be uglier in my opinion.

kobject_init_and_add() is only there for the "simple" use cases.
There's no problem with doing the two-stage process on your own like
this, that's exactly what it is there for :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-07 13:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03 13:26 [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Petr Mladek
2019-05-03 13:26 ` [PATCH 1/2] livepatch: Remove custom kobject state handling Petr Mladek
2019-05-03 16:35   ` Kamalesh Babulal
2019-05-07 12:32   ` Miroslav Benes
2019-05-07 13:06     ` Greg Kroah-Hartman
2019-05-03 13:26 ` [PATCH 2/2] livepatch: Remove duplicated code for early initialization Petr Mladek
2019-05-03 16:35   ` Kamalesh Babulal
2019-05-03 15:39 ` [PATCH 0/2] livepatch: Remove custom kobject state handling and duplicated code Joe Lawrence
2019-05-03 19:16 ` Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).