linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] livepatch: make kobject in klp_object statically allocated
@ 2015-05-19 10:01 Jiri Slaby
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jiri Slaby @ 2015-05-19 10:01 UTC (permalink / raw)
  To: live-patching
  Cc: jpoimboe, sjenning, jkosina, vojtech, linux-kernel,
	Miroslav Benes, Jiri Slaby

From: Miroslav Benes <mbenes@suse.cz>

Make kobj variable (of type struct kobject) statically allocated in
klp_object structure. It will allow us to move in the func-object-patch
hierarchy through kobject links.

The only reason to have it dynamic was to not have empty release
callback in the code. However we have empty callbacks for function and
patch in the code now, so it is no longer valid and the advantage of
static allocation is clear.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/livepatch.h |  2 +-
 kernel/livepatch/core.c   | 22 ++++++++++++++++------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ee6dbb39a809..fe45f2f02c8d 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -99,7 +99,7 @@ struct klp_object {
 	struct klp_func *funcs;
 
 	/* internal */
-	struct kobject *kobj;
+	struct kobject kobj;
 	struct module *mod;
 	enum klp_state state;
 };
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index c0ae3d80e6c0..e42654d72eba 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -652,6 +652,15 @@ static struct kobj_type klp_ktype_patch = {
 	.default_attrs = klp_patch_attrs,
 };
 
+static void klp_kobj_release_object(struct kobject *kobj)
+{
+}
+
+static struct kobj_type klp_ktype_object = {
+	.release = klp_kobj_release_object,
+	.sysfs_ops = &kobj_sysfs_ops,
+};
+
 static void klp_kobj_release_func(struct kobject *kobj)
 {
 }
@@ -696,7 +705,7 @@ static void klp_free_objects_limited(struct klp_patch *patch,
 
 	for (obj = patch->objs; obj->funcs && obj != limit; obj++) {
 		klp_free_funcs_limited(obj, NULL);
-		kobject_put(obj->kobj);
+		kobject_put(&obj->kobj);
 	}
 }
 
@@ -714,7 +723,7 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 	func->state = KLP_DISABLED;
 
 	return kobject_init_and_add(&func->kobj, &klp_ktype_func,
-				    obj->kobj, "%s", func->old_name);
+				    &obj->kobj, "%s", func->old_name);
 }
 
 /* parts of the initialization that is done only when the object is loaded */
@@ -754,9 +763,10 @@ 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";
-	obj->kobj = kobject_create_and_add(name, &patch->kobj);
-	if (!obj->kobj)
-		return -ENOMEM;
+	ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object,
+				   &patch->kobj, "%s", name);
+	if (ret)
+		return ret;
 
 	for (func = obj->funcs; func->old_name; func++) {
 		ret = klp_init_func(obj, func);
@@ -774,7 +784,7 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
 
 free:
 	klp_free_funcs_limited(obj, func);
-	kobject_put(obj->kobj);
+	kobject_put(&obj->kobj);
 	return ret;
 }
 
-- 
2.4.1


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

* [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 10:01 [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Jiri Slaby
@ 2015-05-19 10:01 ` Jiri Slaby
  2015-05-19 12:27   ` Minfei Huang
                     ` (2 more replies)
  2015-05-19 14:21 ` [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Josh Poimboeuf
  2015-05-19 21:59 ` Jiri Kosina
  2 siblings, 3 replies; 11+ messages in thread
From: Jiri Slaby @ 2015-05-19 10:01 UTC (permalink / raw)
  To: live-patching
  Cc: jpoimboe, sjenning, jkosina, vojtech, linux-kernel, Jiri Slaby

klp_for_each_object and klp_for_each_func are now used all over the
code. One need not think what is the proper condition to check in the
for loop now.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/livepatch.h |  6 ++++++
 kernel/livepatch/core.c   | 18 +++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index fe45f2f02c8d..31db7a05dd36 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -123,6 +123,12 @@ struct klp_patch {
 	enum klp_state state;
 };
 
+#define klp_for_each_object(patch, obj) \
+	for (obj = patch->objs; obj->funcs; obj++)
+
+#define klp_for_each_func(obj, func) \
+	for (func = obj->funcs; func->old_name; func++)
+
 int klp_register_patch(struct klp_patch *);
 int klp_unregister_patch(struct klp_patch *);
 int klp_enable_patch(struct klp_patch *);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index e42654d72eba..a545541dcbab 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -423,7 +423,7 @@ static void klp_disable_object(struct klp_object *obj)
 {
 	struct klp_func *func;
 
-	for (func = obj->funcs; func->old_name; func++)
+	klp_for_each_func(obj, func)
 		if (func->state == KLP_ENABLED)
 			klp_disable_func(func);
 
@@ -441,7 +441,7 @@ static int klp_enable_object(struct klp_object *obj)
 	if (WARN_ON(!klp_is_object_loaded(obj)))
 		return -EINVAL;
 
-	for (func = obj->funcs; func->old_name; func++) {
+	klp_for_each_func(obj, func) {
 		ret = klp_enable_func(func);
 		if (ret) {
 			klp_disable_object(obj);
@@ -464,7 +464,7 @@ static int __klp_disable_patch(struct klp_patch *patch)
 
 	pr_notice("disabling patch '%s'\n", patch->mod->name);
 
-	for (obj = patch->objs; obj->funcs; obj++) {
+	klp_for_each_object(patch, obj) {
 		if (obj->state == KLP_ENABLED)
 			klp_disable_object(obj);
 	}
@@ -524,7 +524,7 @@ static int __klp_enable_patch(struct klp_patch *patch)
 
 	pr_notice("enabling patch '%s'\n", patch->mod->name);
 
-	for (obj = patch->objs; obj->funcs; obj++) {
+	klp_for_each_object(patch, obj) {
 		if (!klp_is_object_loaded(obj))
 			continue;
 
@@ -690,7 +690,7 @@ static void klp_free_object_loaded(struct klp_object *obj)
 
 	obj->mod = NULL;
 
-	for (func = obj->funcs; func->old_name; func++)
+	klp_for_each_func(obj, func)
 		func->old_addr = 0;
 }
 
@@ -739,7 +739,7 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 			return ret;
 	}
 
-	for (func = obj->funcs; func->old_name; func++) {
+	klp_for_each_func(obj, func) {
 		ret = klp_find_verify_func_addr(obj, func);
 		if (ret)
 			return ret;
@@ -768,7 +768,7 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
 	if (ret)
 		return ret;
 
-	for (func = obj->funcs; func->old_name; func++) {
+	klp_for_each_func(obj, func) {
 		ret = klp_init_func(obj, func);
 		if (ret)
 			goto free;
@@ -805,7 +805,7 @@ static int klp_init_patch(struct klp_patch *patch)
 	if (ret)
 		goto unlock;
 
-	for (obj = patch->objs; obj->funcs; obj++) {
+	klp_for_each_object(patch, obj) {
 		ret = klp_init_object(patch, obj);
 		if (ret)
 			goto free;
@@ -960,7 +960,7 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action,
 		mod->klp_alive = false;
 
 	list_for_each_entry(patch, &klp_patches, list) {
-		for (obj = patch->objs; obj->funcs; obj++) {
+		klp_for_each_object(patch, obj) {
 			if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
 				continue;
 
-- 
2.4.1


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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
@ 2015-05-19 12:27   ` Minfei Huang
  2015-05-19 21:58     ` Jiri Kosina
  2015-05-19 14:22   ` Josh Poimboeuf
  2015-05-19 21:59   ` Jiri Kosina
  2 siblings, 1 reply; 11+ messages in thread
From: Minfei Huang @ 2015-05-19 12:27 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: live-patching, jpoimboe, sjenning, jkosina, vojtech, linux-kernel

On 05/19/15 at 12:01P, Jiri Slaby wrote:
> klp_for_each_object and klp_for_each_func are now used all over the
> code. One need not think what is the proper condition to check in the
> for loop now.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  include/linux/livepatch.h |  6 ++++++
>  kernel/livepatch/core.c   | 18 +++++++++---------
>  2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index fe45f2f02c8d..31db7a05dd36 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -805,7 +805,7 @@ static int klp_init_patch(struct klp_patch *patch)
>  	if (ret)
>  		goto unlock;
>  
> -	for (obj = patch->objs; obj->funcs; obj++) {
> +	klp_for_each_object(patch, obj) {
>  		ret = klp_init_object(patch, obj);
>  		if (ret)
>  			goto free;
> @@ -960,7 +960,7 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action,
>  		mod->klp_alive = false;
>  
>  	list_for_each_entry(patch, &klp_patches, list) {
> -		for (obj = patch->objs; obj->funcs; obj++) {
> +		klp_for_each_object(patch, obj) {

The code is more clearly to use "if", instead of the loop, although we will take
more than one line than previous, since we will always get the first function
from the object.

Thanks
Minfei

>  			if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
>  				continue;
>  

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

* Re: [PATCH 1/2] livepatch: make kobject in klp_object statically allocated
  2015-05-19 10:01 [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Jiri Slaby
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
@ 2015-05-19 14:21 ` Josh Poimboeuf
  2015-05-19 21:59 ` Jiri Kosina
  2 siblings, 0 replies; 11+ messages in thread
From: Josh Poimboeuf @ 2015-05-19 14:21 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: live-patching, sjenning, jkosina, vojtech, linux-kernel, Miroslav Benes

On Tue, May 19, 2015 at 12:01:18PM +0200, Jiri Slaby wrote:
> From: Miroslav Benes <mbenes@suse.cz>
> 
> Make kobj variable (of type struct kobject) statically allocated in
> klp_object structure. It will allow us to move in the func-object-patch
> hierarchy through kobject links.
> 
> The only reason to have it dynamic was to not have empty release
> callback in the code. However we have empty callbacks for function and
> patch in the code now, so it is no longer valid and the advantage of
> static allocation is clear.
> 
> Signed-off-by: Miroslav Benes <mbenes@suse.cz>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
  2015-05-19 12:27   ` Minfei Huang
@ 2015-05-19 14:22   ` Josh Poimboeuf
  2015-05-19 21:59   ` Jiri Kosina
  2 siblings, 0 replies; 11+ messages in thread
From: Josh Poimboeuf @ 2015-05-19 14:22 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: live-patching, sjenning, jkosina, vojtech, linux-kernel

On Tue, May 19, 2015 at 12:01:19PM +0200, Jiri Slaby wrote:
> klp_for_each_object and klp_for_each_func are now used all over the
> code. One need not think what is the proper condition to check in the
> for loop now.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 12:27   ` Minfei Huang
@ 2015-05-19 21:58     ` Jiri Kosina
  2015-05-20  1:51       ` Minfei Huang
  0 siblings, 1 reply; 11+ messages in thread
From: Jiri Kosina @ 2015-05-19 21:58 UTC (permalink / raw)
  To: Minfei Huang
  Cc: Jiri Slaby, live-patching, jpoimboe, sjenning, vojtech, linux-kernel

On Tue, 19 May 2015, Minfei Huang wrote:

> > klp_for_each_object and klp_for_each_func are now used all over the
> > code. One need not think what is the proper condition to check in the
> > for loop now.
> > 
> > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > ---
> >  include/linux/livepatch.h |  6 ++++++
> >  kernel/livepatch/core.c   | 18 +++++++++---------
> >  2 files changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index fe45f2f02c8d..31db7a05dd36 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -805,7 +805,7 @@ static int klp_init_patch(struct klp_patch *patch)
> >  	if (ret)
> >  		goto unlock;
> >  
> > -	for (obj = patch->objs; obj->funcs; obj++) {
> > +	klp_for_each_object(patch, obj) {
> >  		ret = klp_init_object(patch, obj);
> >  		if (ret)
> >  			goto free;
> > @@ -960,7 +960,7 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action,
> >  		mod->klp_alive = false;
> >  
> >  	list_for_each_entry(patch, &klp_patches, list) {
> > -		for (obj = patch->objs; obj->funcs; obj++) {
> > +		klp_for_each_object(patch, obj) {
> 
> The code is more clearly to use "if", instead of the loop, although we will take
> more than one line than previous, since we will always get the first function
> from the object.

I have absolutely no idea what you are trying to say here, sorry. Could 
you please try to rephrase your review comment?

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH 1/2] livepatch: make kobject in klp_object statically allocated
  2015-05-19 10:01 [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Jiri Slaby
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
  2015-05-19 14:21 ` [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Josh Poimboeuf
@ 2015-05-19 21:59 ` Jiri Kosina
  2 siblings, 0 replies; 11+ messages in thread
From: Jiri Kosina @ 2015-05-19 21:59 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: live-patching, jpoimboe, sjenning, vojtech, linux-kernel, Miroslav Benes

On Tue, 19 May 2015, Jiri Slaby wrote:

> From: Miroslav Benes <mbenes@suse.cz>
> 
> Make kobj variable (of type struct kobject) statically allocated in
> klp_object structure. It will allow us to move in the func-object-patch
> hierarchy through kobject links.
> 
> The only reason to have it dynamic was to not have empty release
> callback in the code. However we have empty callbacks for function and
> patch in the code now, so it is no longer valid and the advantage of
> static allocation is clear.

Applied to for-4.2/upstream.

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
  2015-05-19 12:27   ` Minfei Huang
  2015-05-19 14:22   ` Josh Poimboeuf
@ 2015-05-19 21:59   ` Jiri Kosina
  2 siblings, 0 replies; 11+ messages in thread
From: Jiri Kosina @ 2015-05-19 21:59 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: live-patching, jpoimboe, sjenning, vojtech, linux-kernel

On Tue, 19 May 2015, Jiri Slaby wrote:

> klp_for_each_object and klp_for_each_func are now used all over the
> code. One need not think what is the proper condition to check in the
> for loop now.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Applied to for-4.2/upstream.

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-19 21:58     ` Jiri Kosina
@ 2015-05-20  1:51       ` Minfei Huang
  2015-05-20  7:11         ` Jiri Slaby
  0 siblings, 1 reply; 11+ messages in thread
From: Minfei Huang @ 2015-05-20  1:51 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jiri Slaby, live-patching, jpoimboe, sjenning, vojtech, linux-kernel

On 05/19/15 at 11:58P, Jiri Kosina wrote:
> On Tue, 19 May 2015, Minfei Huang wrote:
> 
> > > klp_for_each_object and klp_for_each_func are now used all over the
> > > code. One need not think what is the proper condition to check in the
> > > for loop now.
> > > 
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > ---
> > >  include/linux/livepatch.h |  6 ++++++
> > >  kernel/livepatch/core.c   | 18 +++++++++---------
> > >  2 files changed, 15 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > > index fe45f2f02c8d..31db7a05dd36 100644
> > > --- a/include/linux/livepatch.h
> > > +++ b/include/linux/livepatch.h
> > > @@ -805,7 +805,7 @@ static int klp_init_patch(struct klp_patch *patch)
> > >  	if (ret)
> > >  		goto unlock;
> > >  
> > > -	for (obj = patch->objs; obj->funcs; obj++) {
> > > +	klp_for_each_object(patch, obj) {
> > >  		ret = klp_init_object(patch, obj);
> > >  		if (ret)
> > >  			goto free;
> > > @@ -960,7 +960,7 @@ static int klp_module_notify(struct notifier_block *nb, unsigned long action,
> > >  		mod->klp_alive = false;
> > >  
> > >  	list_for_each_entry(patch, &klp_patches, list) {
> > > -		for (obj = patch->objs; obj->funcs; obj++) {
> > > +		klp_for_each_object(patch, obj) {
> > 
> > The code is more clearly to use "if", instead of the loop, although we will take
> > more than one line than previous, since we will always get the first function
> > from the object.
> 
> I have absolutely no idea what you are trying to say here, sorry. Could 
> you please try to rephrase your review comment?
> 

Sure. Sorry for confuse you with my comment.

Following is the livepatch code.

list_for_each_entry(patch, &klp_patches, list) {
	for (obj = patch->objs; obj->funcs; obj++) {
----------------------------------
We get the fisrt object from the patch, then we break the loop. The code is more clearly to
use "if", instead of the loop.
----------------------------------
		if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
			continue;

		if (action == MODULE_STATE_COMING) {
			obj->mod = mod;
			klp_module_notify_coming(patch, obj);
		} else /* MODULE_STATE_GOING */
			klp_module_notify_going(patch, obj);

		break;
----------------------------------
Here we break the loop.
----------------------------------
	}
}

Thanks
Minfei

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-20  1:51       ` Minfei Huang
@ 2015-05-20  7:11         ` Jiri Slaby
  2015-05-23 13:23           ` Minfei Huang
  0 siblings, 1 reply; 11+ messages in thread
From: Jiri Slaby @ 2015-05-20  7:11 UTC (permalink / raw)
  To: Minfei Huang, Jiri Kosina
  Cc: live-patching, jpoimboe, sjenning, vojtech, linux-kernel

On 05/20/2015, 03:51 AM, Minfei Huang wrote:
> Sure. Sorry for confuse you with my comment.

Oh, I see now, but:

> list_for_each_entry(patch, &klp_patches, list) {
> 	for (obj = patch->objs; obj->funcs; obj++) {
> ----------------------------------
> We get the fisrt object from the patch, then we break the loop. The code is more clearly to
> use "if", instead of the loop.
> ----------------------------------
> 		if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
> 			continue;

See 'continue' here. This *is* a loop and we do not fetch the first
object. We look for the one with the same name.

> 		if (action == MODULE_STATE_COMING) {
> 			obj->mod = mod;
> 			klp_module_notify_coming(patch, obj);
> 		} else /* MODULE_STATE_GOING */
> 			klp_module_notify_going(patch, obj);
> 
> 		break;
> ----------------------------------
> Here we break the loop.

Only if we found the one.

> ----------------------------------
> 	}
> }

thanks,
-- 
js
suse labs

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

* Re: [PATCH 2/2] livepatch: introduce patch/func-walking helpers
  2015-05-20  7:11         ` Jiri Slaby
@ 2015-05-23 13:23           ` Minfei Huang
  0 siblings, 0 replies; 11+ messages in thread
From: Minfei Huang @ 2015-05-23 13:23 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Minfei Huang, Jiri Kosina, live-patching, jpoimboe, sjenning,
	vojtech, linux-kernel

On 05/20/15 at 09:11am, Jiri Slaby wrote:
> On 05/20/2015, 03:51 AM, Minfei Huang wrote:
> > Sure. Sorry for confuse you with my comment.
> 
> Oh, I see now, but:
> 
> > list_for_each_entry(patch, &klp_patches, list) {
> > 	for (obj = patch->objs; obj->funcs; obj++) {
> > ----------------------------------
> > We get the fisrt object from the patch, then we break the loop. The code is more clearly to
> > use "if", instead of the loop.
> > ----------------------------------
> > 		if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
> > 			continue;
> 
> See 'continue' here. This *is* a loop and we do not fetch the first
> object. We look for the one with the same name.
> 
> > 		if (action == MODULE_STATE_COMING) {
> > 			obj->mod = mod;
> > 			klp_module_notify_coming(patch, obj);
> > 		} else /* MODULE_STATE_GOING */
> > 			klp_module_notify_going(patch, obj);
> > 
> > 		break;
> > ----------------------------------
> > Here we break the loop.
> 
> Only if we found the one.
> 

Hi, Jiri.

Ooops!

Since it is impossible for the differect objects which have the some
name in one patch, it is right to break the loop once we find a matched
object. It confuses me with the logic in __klp_enable_patch.

Thanks for your explanation.

Thanks
Minfei

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

end of thread, other threads:[~2015-05-23 13:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 10:01 [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Jiri Slaby
2015-05-19 10:01 ` [PATCH 2/2] livepatch: introduce patch/func-walking helpers Jiri Slaby
2015-05-19 12:27   ` Minfei Huang
2015-05-19 21:58     ` Jiri Kosina
2015-05-20  1:51       ` Minfei Huang
2015-05-20  7:11         ` Jiri Slaby
2015-05-23 13:23           ` Minfei Huang
2015-05-19 14:22   ` Josh Poimboeuf
2015-05-19 21:59   ` Jiri Kosina
2015-05-19 14:21 ` [PATCH 1/2] livepatch: make kobject in klp_object statically allocated Josh Poimboeuf
2015-05-19 21:59 ` 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).