Skip to contentSkip to conversation

jump_label: fix jump_label update

PATCH In review Xiao Guangrong 5 participants 8 messages Jun 21, 2011 – Jul 1, 2011 Static branch/call

lore
Share & tools
lore

Xiao Guangrong

author
#0 lore raw

PATCH v2 1/1 jump_label: fix jump_label update

The key of module is out of __stop___jump_table, it causes the events of modules does not work

Signed-off-by Xiao Guangrong <[email protected]>
---
 kernel/jump_label.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
kernel/jump_label.c +9 -5
index fa27e75..a8ce450 100644
@@ -375,15 +375,19 @@ int jump_label_text_reserved(void *start, void *end)
 
 static void jump_label_update(struct jump_label_key *key, int enable)
 {
-	struct jump_entry *entry = key->entries;
-
-	/* if there are no users, entry can be NULL */
-	if (entry)
-		__jump_label_update(key, entry, __stop___jump_table, enable);
+	struct jump_entry *entry = key->entries, *stop = __stop___jump_table;
 
 #ifdef CONFIG_MODULES
+	struct module *mod = __module_address((jump_label_t)key);
+
 	__jump_label_mod_update(key, enable);
+
+	if (mod)
+		stop = mod->jump_entries + mod->num_jump_entries;
 #endif
+	/* if there are no users, entry can be NULL */
+	if (entry)
+		__jump_label_update(key, entry, stop, enable);
 }
 
 #endif
Signature · 1 line
1.7.5.4
#1 lore raw
Quoted 39 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

Signed-off-by Xiao Guangrong <[email protected]>
---
 kernel/jump_label.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
kernel/jump_label.c +9 -5
index fa27e75..a8ce450 100644
@@ -375,15 +375,19 @@ int jump_label_text_reserved(void *start, void *end)
 
 static void jump_label_update(struct jump_label_key *key, int enable)
 {
-	struct jump_entry *entry = key->entries;
-
-	/* if there are no users, entry can be NULL */
-	if (entry)
-		__jump_label_update(key, entry, __stop___jump_table, enable);
+	struct jump_entry *entry = key->entries, *stop = __stop___jump_table;
 
 #ifdef CONFIG_MODULES
+	struct module *mod = __module_address((jump_label_t)key);
+
 	__jump_label_mod_update(key, enable);
+
+	if (mod)
+		stop = mod->jump_entries + mod->num_jump_entries;
 #endif
+	/* if there are no users, entry can be NULL */
+	if (entry)
+		__jump_label_update(key, entry, stop, enable);
 }
 
 #endif
Signature · 1 line
1.7.5.4

That works for me, but in order to test it I needed to export jump_label_inc/jump_label_dec functions.

Not sure I'm missing something, but to manage a key that is local to the module, we need to call jump_label_inc/jump_label_dec from within the module code, so we need some of the jump_label functions exported.. it'd be probably:

jump_label_inc/jump_label_dec/jump_label_enabled

wbr,
jirka

I used following module code to test:
---
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/jump_label.h>

static struct jump_label_key key;

static bool enabled;
module_param(enabled, bool, 0644);

static int __init mod_init(void)

{
        if (enabled)
                jump_label_inc(&key);

        if (static_branch(&key))
                printk("key enabled\n");
        else
                printk("key disabled\n");

        if (jump_label_enabled(&key))
                printk("key enabled1\n");
        else
                printk("key disabled1\n");

        printk("mod loaded\n");
        return 0;
}

static void __exit mod_exit(void)

{
        printk("mod unloaded\n");
        return;
}

module_init(mod_init);
module_exit(mod_exit);
MODULE_LICENSE("GPL");

#2 lore raw
Quoted 44 lines · Jiri Olsa
Quoted 39 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

Signed-off-by Xiao Guangrong <[email protected]>
---
 kernel/jump_label.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
kernel/jump_label.c +9 -5
index fa27e75..a8ce450 100644
@@ -375,15 +375,19 @@ int jump_label_text_reserved(void *start, void *end)
 
 static void jump_label_update(struct jump_label_key *key, int enable)
 {
-	struct jump_entry *entry = key->entries;
-
-	/* if there are no users, entry can be NULL */
-	if (entry)
-		__jump_label_update(key, entry, __stop___jump_table, enable);
+	struct jump_entry *entry = key->entries, *stop = __stop___jump_table;
 
 #ifdef CONFIG_MODULES
+	struct module *mod = __module_address((jump_label_t)key);
+
 	__jump_label_mod_update(key, enable);
+
+	if (mod)
+		stop = mod->jump_entries + mod->num_jump_entries;
 #endif
+	/* if there are no users, entry can be NULL */
+	if (entry)
+		__jump_label_update(key, entry, stop, enable);
 }
 
 #endif
Signature · 1 line
1.7.5.4

That works for me, but in order to test it I needed to export jump_label_inc/jump_label_dec functions.

Quoted 7 lines

Not sure I'm missing something, but to manage a key that is local to the module, we need to call jump_label_inc/jump_label_dec from within the module code, so we need some of the jump_label functions exported.. it'd be probably:

jump_label_inc/jump_label_dec/jump_label_enabled

You're not missing anything, right now all the enabling/disabling is done in the core kernel, hence no need for the export. I'd prefer to keep them non-exported until we had a real use-case that required them to be exported. But I don't feel that strongly about it.

Thanks,

-Jason

Acked-by on v2 1/1
#3 lore raw
In reply to Xiao Guangrong
Quoted 39 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

Signed-off-by Xiao Guangrong <[email protected]>
---
 kernel/jump_label.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
kernel/jump_label.c +9 -5
index fa27e75..a8ce450 100644
@@ -375,15 +375,19 @@ int jump_label_text_reserved(void *start, void *end)
 
 static void jump_label_update(struct jump_label_key *key, int enable)
 {
-	struct jump_entry *entry = key->entries;
-
-	/* if there are no users, entry can be NULL */
-	if (entry)
-		__jump_label_update(key, entry, __stop___jump_table, enable);
+	struct jump_entry *entry = key->entries, *stop = __stop___jump_table;
 
 #ifdef CONFIG_MODULES
+	struct module *mod = __module_address((jump_label_t)key);
+
 	__jump_label_mod_update(key, enable);
+
+	if (mod)
+		stop = mod->jump_entries + mod->num_jump_entries;
 #endif
+	/* if there are no users, entry can be NULL */
+	if (entry)
+		__jump_label_update(key, entry, stop, enable);
 }
 
 #endif
Signature · 1 line
1.7.5.4

Looks good. Thanks for the fix!

Acked-by Jason Baron <[email protected]>

-Jason

#4 lore raw
In reply to Xiao Guangrong
Quoted 3 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

What's the status of this patch? I'm affected by this as well (I'm guessing the same module is involved). I'm near the end of a bisect, but there's probably no point given this patch.

Signature · 1 line
error compiling committee.c: too many arguments to function
Tested-by
#5 lore raw
Quoted 6 lines · Avi Kivity
Quoted 3 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

What's the status of this patch?

Oh, it's just two days out. Didn't mean to sound impatient.

Quoted 2 lines

I'm affected by this as well (I'm guessing the same module is involved). I'm near the end of a bisect, but there's probably no point given this patch.

I can confirm it fixes my issue.

Tested-by Avi Kivity <[email protected]>
Signature · 1 line
error compiling committee.c: too many arguments to function
reviewer
#6 lore raw
Quoted 9 lines · Avi Kivity
Quoted 6 lines · Avi Kivity
Quoted 3 lines · Xiao Guangrong

The key of module is out of __stop___jump_table, it causes the events of modules does not work

What's the status of this patch?

Oh, it's just two days out. Didn't mean to sound impatient.

I've been working on other things than tracing lately.

Quoted 10 lines
Quoted 5 lines

I'm affected by this as well (I'm guessing the same module is involved). I'm near the end of a bisect, but there's probably no point given this patch.

I can confirm it fixes my issue.

Tested-by Avi Kivity <[email protected]>

Thanks, I'll queue it up for 3.0 and start my tests on it.

-- Steve

author
#7 lore raw

[tip:perf/urgent] jump_label: Fix jump_label update for modules

In reply to Xiao Guangrong
Commit-ID:  140fe3b1ab9c082182ef13359fab4ddba95c24c3
Gitweb:     http://git.kernel.org/tip/140fe3b1ab9c082182ef13359fab4ddba95c24c3
Author:     Xiao Guangrong <[email protected]>
AuthorDate: Tue, 21 Jun 2011 10:35:55 +0800
Committer:  Steven Rostedt <[email protected]>
CommitDate: Wed, 29 Jun 2011 09:59:17 -0400

jump_label: Fix jump_label update for modules

The jump labels entries for modules do not stop at __stop__jump_table, but after mod->jump_entries + mod_num_jump_entries.

By checking the wrong end point, module trace events never get enabled.

Cc Ingo Molnar <[email protected]>
Acked-by Jason Baron <[email protected]>
Tested-by Avi Kivity <[email protected]>
Tested-by Johannes Berg <[email protected]>
Signed-off-by Xiao Guangrong <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/jump_label.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
kernel/jump_label.c +9 -5
index fa27e75..a8ce450 100644
@@ -375,15 +375,19 @@ int jump_label_text_reserved(void *start, void *end)
 
 static void jump_label_update(struct jump_label_key *key, int enable)
 {
-	struct jump_entry *entry = key->entries;
-
-	/* if there are no users, entry can be NULL */
-	if (entry)
-		__jump_label_update(key, entry, __stop___jump_table, enable);
+	struct jump_entry *entry = key->entries, *stop = __stop___jump_table;
 
 #ifdef CONFIG_MODULES
+	struct module *mod = __module_address((jump_label_t)key);
+
 	__jump_label_mod_update(key, enable);
+
+	if (mod)
+		stop = mod->jump_entries + mod->num_jump_entries;
 #endif
+	/* if there are no users, entry can be NULL */
+	if (entry)
+		__jump_label_update(key, entry, stop, enable);
 }
 
 #endif