[PATCH 0/2] tracing - wrong scale in ftrace_release

[PATCH 0/2] tracing - wrong scale in ftrace_release

[jolsa]

When the module is about the unload we release its call records.
The ftrace_release function was given wrong values representing
the module core boundaries, thus not releasing its call records.

I prepared 2 patches:

1/2 - simple fix putting correct core boundaries 
2/2 - making ftrace_release function module specific

Please choose whichever version you like more.

wbr,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 kernel/trace/ftrace.c       |  150 +++++++++++-------------------------------
 kernel/trace/trace.c        |  106 ++++++++++++++++++++++++++++++
 kernel/trace/trace.h        |   35 ++++++++++
 kernel/trace/trace_events.c |   60 ++++-------------
 4 files changed, 194 insertions(+), 157 deletions(-)

[PATCH 1/2] tracing - correct module boundaries for ftrace_release

[jolsa]

When the module is about the unload we release its call records.
The ftrace_release function was given wrong values representing
the module core boundaries, thus not releasing its call records.

wbr,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 kernel/trace/ftrace.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9a72853..09971c5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2652,9 +2652,9 @@ static int ftrace_module_notify(struct notifier_block *self,
 				   mod->num_ftrace_callsites);
 		break;
 	case MODULE_STATE_GOING:
-		ftrace_release(mod->ftrace_callsites,
-			       mod->ftrace_callsites +
-			       mod->num_ftrace_callsites);
+		ftrace_release(mod->module_core,
+			       mod->module_core +
+			       mod->core_size);
 		break;
 	}
 

[PATCH 2/2] tracing - correct module boundaries for ftrace_release

[jolsa]

When the module is about the unload we release its call records.
The ftrace_release function was given wrong values representing
the module core boundaries, thus not releasing its call records.

Plus making ftrace_release function module specific.

wbr,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 include/linux/ftrace.h |    2 +-
 kernel/trace/ftrace.c  |   12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index cd3d2ab..0b4f97d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -241,7 +241,7 @@ extern void ftrace_enable_daemon(void);
 # define ftrace_set_filter(buf, len, reset)	do { } while (0)
 # define ftrace_disable_daemon()		do { } while (0)
 # define ftrace_enable_daemon()			do { } while (0)
-static inline void ftrace_release(void *start, unsigned long size) { }
+static inline void ftrace_release_mod(struct module *mod) {}
 static inline int register_ftrace_command(struct ftrace_func_command *cmd)
 {
 	return -EINVAL;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9a72853..559e552 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2608,19 +2608,17 @@ static int ftrace_convert_nops(struct module *mod,
 }
 
 #ifdef CONFIG_MODULES
-void ftrace_release(void *start, void *end)
+void ftrace_release_mod(struct module *mod)
 {
 	struct dyn_ftrace *rec;
 	struct ftrace_page *pg;
-	unsigned long s = (unsigned long)start;
-	unsigned long e = (unsigned long)end;
 
-	if (ftrace_disabled || !start || start == end)
+	if (ftrace_disabled)
 		return;
 
 	mutex_lock(&ftrace_lock);
 	do_for_each_ftrace_rec(pg, rec) {
-		if ((rec->ip >= s) && (rec->ip < e)) {
+		if (within_module_core(rec->ip, mod)) {
 			/*
 			 * rec->ip is changed in ftrace_free_rec()
 			 * It should not between s and e if record was freed.
@@ -2652,9 +2650,7 @@ static int ftrace_module_notify(struct notifier_block *self,
 				   mod->num_ftrace_callsites);
 		break;
 	case MODULE_STATE_GOING:
-		ftrace_release(mod->ftrace_callsites,
-			       mod->ftrace_callsites +
-			       mod->num_ftrace_callsites);
+		ftrace_release_mod(mod);
 		break;
 	}
 

Re: [PATCH 2/2] tracing - correct module boundaries for ftrace_release

[Steven Rostedt]

On Wed, 2009-10-07 at 19:00 +0200, jolsa@redhat.com wrote:
> When the module is about the unload we release its call records.
> The ftrace_release function was given wrong values representing
> the module core boundaries, thus not releasing its call records.
> 
> Plus making ftrace_release function module specific.
> 

I'll take this version. Thanks! I'll get this ready for mainline urgent,
as well as push it off to stable.

-- Steve

> wbr,
> jirka
> 
> 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>  include/linux/ftrace.h |    2 +-
>  kernel/trace/ftrace.c  |   12 ++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index cd3d2ab..0b4f97d 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -241,7 +241,7 @@ extern void ftrace_enable_daemon(void);
>  # define ftrace_set_filter(buf, len, reset)	do { } while (0)
>  # define ftrace_disable_daemon()		do { } while (0)
>  # define ftrace_enable_daemon()			do { } while (0)
> -static inline void ftrace_release(void *start, unsigned long size) { }
> +static inline void ftrace_release_mod(struct module *mod) {}
>  static inline int register_ftrace_command(struct ftrace_func_command *cmd)
>  {
>  	return -EINVAL;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 9a72853..559e552 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2608,19 +2608,17 @@ static int ftrace_convert_nops(struct module *mod,
>  }
>  
>  #ifdef CONFIG_MODULES
> -void ftrace_release(void *start, void *end)
> +void ftrace_release_mod(struct module *mod)
>  {
>  	struct dyn_ftrace *rec;
>  	struct ftrace_page *pg;
> -	unsigned long s = (unsigned long)start;
> -	unsigned long e = (unsigned long)end;
>  
> -	if (ftrace_disabled || !start || start == end)
> +	if (ftrace_disabled)
>  		return;
>  
>  	mutex_lock(&ftrace_lock);
>  	do_for_each_ftrace_rec(pg, rec) {
> -		if ((rec->ip >= s) && (rec->ip < e)) {
> +		if (within_module_core(rec->ip, mod)) {
>  			/*
>  			 * rec->ip is changed in ftrace_free_rec()
>  			 * It should not between s and e if record was freed.
> @@ -2652,9 +2650,7 @@ static int ftrace_module_notify(struct notifier_block *self,
>  				   mod->num_ftrace_callsites);
>  		break;
>  	case MODULE_STATE_GOING:
> -		ftrace_release(mod->ftrace_callsites,
> -			       mod->ftrace_callsites +
> -			       mod->num_ftrace_callsites);
> +		ftrace_release_mod(mod);
>  		break;
>  	}
>  

[tip:tracing/urgent] tracing: correct module boundaries for ftrace_release

[tip-bot for jolsa@redhat.com]

Commit-ID:  e7247a15ff3bbdab0a8b402dffa1171e5c05a8e0
Gitweb:     http://git.kernel.org/tip/e7247a15ff3bbdab0a8b402dffa1171e5c05a8e0
Author:     jolsa@redhat.com <jolsa@redhat.com>
AuthorDate: Wed, 7 Oct 2009 19:00:35 +0200
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Wed, 7 Oct 2009 15:52:09 -0400

tracing: correct module boundaries for ftrace_release

When the module is about the unload we release its call records.
The ftrace_release function was given wrong values representing
the module core boundaries, thus not releasing its call records.

Plus making ftrace_release function module specific.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
LKML-Reference: <1254934835-363-3-git-send-email-jolsa@redhat.com>
Cc: stable@kernel.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h |    2 +-
 kernel/trace/ftrace.c  |   12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index cd3d2ab..0b4f97d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -241,7 +241,7 @@ extern void ftrace_enable_daemon(void);
 # define ftrace_set_filter(buf, len, reset)	do { } while (0)
 # define ftrace_disable_daemon()		do { } while (0)
 # define ftrace_enable_daemon()			do { } while (0)
-static inline void ftrace_release(void *start, unsigned long size) { }
+static inline void ftrace_release_mod(struct module *mod) {}
 static inline int register_ftrace_command(struct ftrace_func_command *cmd)
 {
 	return -EINVAL;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 46592fe..c701476 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2658,19 +2658,17 @@ static int ftrace_convert_nops(struct module *mod,
 }
 
 #ifdef CONFIG_MODULES
-void ftrace_release(void *start, void *end)
+void ftrace_release_mod(struct module *mod)
 {
 	struct dyn_ftrace *rec;
 	struct ftrace_page *pg;
-	unsigned long s = (unsigned long)start;
-	unsigned long e = (unsigned long)end;
 
-	if (ftrace_disabled || !start || start == end)
+	if (ftrace_disabled)
 		return;
 
 	mutex_lock(&ftrace_lock);
 	do_for_each_ftrace_rec(pg, rec) {
-		if ((rec->ip >= s) && (rec->ip < e)) {
+		if (within_module_core(rec->ip, mod)) {
 			/*
 			 * rec->ip is changed in ftrace_free_rec()
 			 * It should not between s and e if record was freed.
@@ -2702,9 +2700,7 @@ static int ftrace_module_notify(struct notifier_block *self,
 				   mod->num_ftrace_callsites);
 		break;
 	case MODULE_STATE_GOING:
-		ftrace_release(mod->ftrace_callsites,
-			       mod->ftrace_callsites +
-			       mod->num_ftrace_callsites);
+		ftrace_release_mod(mod);
 		break;
 	}