ftrace: fix perf-tracepoint OOPS
Subsystems (3)
Peter Zijlstra
maintainerPATCH 1/1 ftrace: fix perf-tracepoint OOPS
Not all tracepoints are created equal, in specific the ftrace tracepoints are created with TRACE_EVENT_FORMAT() which does not generate the needed bits to tie them into perf counters.
For those events, don't create the 'id' file and fail ->profile_enable when their ID is specified through other means.
--- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
kernel/trace/trace_event_profile.c +1 -1
@@ -14,7 +14,7 @@ int ftrace_profile_enable(int event_id)mutex_lock (&event_mutex);list_for_each_entry (event, &ftrace_events, list) { -if (event->id == event_id) { +if (event->id == event_id && event->profile_enable) { ret = event->profile_enable (event);break ; }
kernel/trace/trace_events.c +1 -1
@@ -971,7 +971,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable); -if (call->id) +if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);
tip-bot for Peter Zijlstra
maintainer[tip:perfcounters/urgent] ftrace: Fix perf-tracepoint OOPS
Commit-ID: 5fe31b14cc8eccc06311c3cc5674420a66431372 Gitweb: http://git.kernel.org/tip/5fe31b14cc8eccc06311c3cc5674420a66431372 Author: Peter Zijlstra <[email protected]> AuthorDate: Wed, 5 Aug 2009 20:41:04 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 5 Aug 2009 20:45:52 +0200
ftrace: Fix perf-tracepoint OOPS
Not all tracepoints are created equal, in specific the ftrace tracepoints are created with TRACE_EVENT_FORMAT() which does not generate the needed bits to tie them into perf counters.
For those events, don't create the 'id' file and fail ->profile_enable when their ID is specified through other means.
LKML-Reference: <1249497664.5890.4.camel@laptop>
--- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
kernel/trace/trace_event_profile.c +1 -1
@@ -14,7 +14,7 @@ int ftrace_profile_enable(int event_id)mutex_lock (&event_mutex);list_for_each_entry (event, &ftrace_events, list) { -if (event->id == event_id) { +if (event->id == event_id && event->profile_enable) { ret = event->profile_enable (event);break ; }
kernel/trace/trace_events.c +1 -1
@@ -940,7 +940,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable); -if (call->id) +if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);
Quoted 9 lines · Peter Zijlstra
Not all tracepoints are created equal, in specific the ftrace tracepoints are created with
TRACE_EVENT_FORMAT()which does not generate the needed bits to tie them into perf counters.For those events, don't create the 'id' file and fail ->profile_enable when their ID is specified through other means.
Reported-by Chris Mason <[email protected]>Signed-off-by Peter Zijlstra <[email protected]>
-- Steve
Quoted 34 lines
--- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)kernel/trace/trace_event_profile.c +1
-1@@ -14,7 +14,7 @@ int ftrace_profile_enable(int event_id)mutex_lock (&event_mutex);list_for_each_entry (event, &ftrace_events, list) { -if (event->id == event_id) { +if (event->id == event_id && event->profile_enable) { ret = event->profile_enable (event);break ; }kernel/trace/trace_events.c +1
-1@@ -971,7 +971,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable); -if (call->id) +if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);
Peter Zijlstra
maintainerPATCH 1/1 perf: Don't list tracepoints without an id
In reply to Peter ZijlstraStop perf list from displaying tracepoints without an id file.
--- tools/perf/util/parse-events.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
tools/perf/util/parse-events.c +10 -0
@@ -568,6 +568,7 @@ static void print_tracepoint_events(void)struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;struct stat st;char evt_path[MAXPATHLEN ]; +int fd;if (valid_debugfs_mount (debugfs_path))return ;
@@ -582,6 +583,15 @@ static void print_tracepoint_events(void) goto cleanup;for_each_event (sys_dirent, evt_dir, evt_dirent, evt_next, evt_path, st) { + +snprintf (evt_path,MAXPATHLEN ,"%s/%s/%s/id" , + debugfs_path, + sys_dirent.d_name, evt_dirent.d_name); + fd =open (evt_path,O_RDONLY ); +if (fd <0 ) +continue ; +close (fd); +snprintf (evt_path,MAXPATHLEN ,"%s:%s" , sys_dirent.d_name, evt_dirent.d_name); fprintf(stderr, " %-40s [%s]\n", evt_path,
Re: [PATCH] perf: Don't list tracepoints without an id
Quoted 27 lines · Peter Zijlstra
Stop perf list from displaying tracepoints without an id file.
Signed-off-by Peter Zijlstra <[email protected]>--- tools/perf/util/parse-events.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)tools/perf/util/parse-events.c +3
-0@@ -568,6 +568,7 @@ static void print_tracepoint_events(void)struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;struct stat st;char evt_path[MAXPATHLEN ]; +int fd;if (valid_debugfs_mount (debugfs_path))return ;@@ -582,6 +583,15 @@ static void print_tracepoint_events(void) goto cleanup;for_each_event (sys_dirent, evt_dir, evt_dirent, evt_next, evt_path, st) { + + snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id",
tools/perf/util/parse-events.c static void print_tracepoint_events(void) · line 597
Quoted 10 lines
tools/perf/util/parse-events.c +7
-0+ debugfs_path, + sys_dirent.d_name, evt_dirent.d_name); + fd =open (evt_path,O_RDONLY ); +if (fd <0 ) +continue ; +close (fd); +snprintf (evt_path,MAXPATHLEN ,"%s:%s" , sys_dirent.d_name, evt_dirent.d_name); fprintf(stderr, " %-40s [%s]\n", evt_path,
hmmm, might be nicer to add this check to the 'for_each_event()' macro instead...
thanks,
-Jason
Peter Zijlstra
maintainerPATCH 1/1 perf: Don't list tracepoints without an id
Stop perf list from displaying tracepoints without an id file.
--- tools/perf/util/parse-events.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/parse-events.c +17 -1
@@ -121,13 +121,29 @@ static unsigned long hw_cache_stat[C(MAX (strcmp(sys_dirent.d_name, ".")) && \ (strcmp(sys_dirent.d_name, ".."))) +static int tp_event_has_id (struct dirent *sys_dir,struct dirent *evt_dir) +{ +char evt_path[MAXPATHLEN ]; +int fd; + +snprintf (evt_path,MAXPATHLEN ,"%s/%s/%s/id" , debugfs_path, + sys_dir->d_name, evt_dir->d_name); + fd =open (evt_path,O_RDONLY ); +if (fd <0 ) +return -EINVAL ; +close (fd); + +return 0 ; +} +#define for_each_event (sys_dirent, evt_dir, evt_dirent, evt_next, file, st) \ while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next) \ if (snprintf(file, MAXPATHLEN, "%s/%s/%s", debugfs_path, \ sys_dirent.d_name, evt_dirent.d_name) && \ (!stat(file, &st)) && (S_ISDIR(st.st_mode)) && \ (strcmp(evt_dirent.d_name, ".")) && \ - (strcmp(evt_dirent.d_name, ".."))) + (strcmp(evt_dirent.d_name, "..")) && \ + (!tp_event_has_id(&sys_dirent, &evt_dirent)))#define MAX_EVENT_LENGTH 30
Re: [PATCH -v2] perf: Don't list tracepoints without an id
Quoted 32 lines · Peter Zijlstra
Stop perf list from displaying tracepoints without an id file.
Signed-off-by Peter Zijlstra <[email protected]>--- tools/perf/util/parse-events.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)Index:
linux-2.6/tools/perf/util/parse-events.c
===================================================================tools/perf/util/parse-events.c +15
-0@@ -121,13 +121,29 @@ static unsigned long hw_cache_stat[C(MAX (strcmp(sys_dirent.d_name, ".")) && \ (strcmp(sys_dirent.d_name, ".."))) +static int tp_event_has_id (struct dirent *sys_dir,struct dirent *evt_dir) +{ +char evt_path[MAXPATHLEN ]; +int fd; + +snprintf (evt_path,MAXPATHLEN ,"%s/%s/%s/id" , debugfs_path, + sys_dir->d_name, evt_dir->d_name); + fd =open (evt_path,O_RDONLY ); +if (fd <0 ) +return -EINVAL ; +close (fd); + +return 0 ; +} +#define for_each_event (sys_dirent, evt_dir, evt_dirent, evt_next, file, st) \ while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next) \
tools/perf/util/parse-events.c static unsigned long hw_cache_stat[C(MAX · line 149
Quoted 10 lines
tools/perf/util/parse-events.c +2
-1if (snprintf (file,MAXPATHLEN ,"%s/%s/%s" , debugfs_path, \ sys_dirent.d_name, evt_dirent.d_name) && \ (!stat (file, &st)) && (S_ISDIR (st.st_mode)) && \ (strcmp (evt_dirent.d_name,"." )) && \ - (strcmp(evt_dirent.d_name, ".."))) + (strcmp (evt_dirent.d_name,".." )) && \ + (!tp_event_has_id(&sys_dirent, &evt_dirent)))#define MAX_EVENT_LENGTH 30
looks good to me.
thanks,
-Jason
tip-bot for Peter Zijlstra
maintainer[tip:perfcounters/urgent] perf: Don't list tracepoints without an id
In reply to Peter ZijlstraCommit-ID: 79e372825f775fabd2219c9c45fbdff93c24cd3f Gitweb: http://git.kernel.org/tip/79e372825f775fabd2219c9c45fbdff93c24cd3f Author: Peter Zijlstra <[email protected]> AuthorDate: Thu, 6 Aug 2009 16:48:54 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Aug 2009 13:09:10 +0200
perf: Don't list tracepoints without an id
Stop perf list from displaying tracepoints without an id file, those are special tracepoints that are not interfaced to perfcounters.
LKML-Reference: <1249570134.32113.483.camel@twins>
--- tools/perf/util/parse-events.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)
tools/perf/util/parse-events.c +17 -1
@@ -121,13 +121,29 @@ static unsigned long hw_cache_stat[C(MAX)] = { (strcmp(sys_dirent.d_name, ".")) && \ (strcmp(sys_dirent.d_name, ".."))) +static int tp_event_has_id (struct dirent *sys_dir,struct dirent *evt_dir) +{ +char evt_path[MAXPATHLEN ]; +int fd; + +snprintf (evt_path,MAXPATHLEN ,"%s/%s/%s/id" , debugfs_path, + sys_dir->d_name, evt_dir->d_name); + fd =open (evt_path,O_RDONLY ); +if (fd <0 ) +return -EINVAL ; +close (fd); + +return 0 ; +} +#define for_each_event (sys_dirent, evt_dir, evt_dirent, evt_next, file, st) \ while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next) \ if (snprintf(file, MAXPATHLEN, "%s/%s/%s", debugfs_path, \ sys_dirent.d_name, evt_dirent.d_name) && \ (!stat(file, &st)) && (S_ISDIR(st.st_mode)) && \ (strcmp(evt_dirent.d_name, ".")) && \ - (strcmp(evt_dirent.d_name, ".."))) + (strcmp(evt_dirent.d_name, "..")) && \ + (!tp_event_has_id(&sys_dirent, &evt_dirent)))#define MAX_EVENT_LENGTH 30
tip-bot for Peter Zijlstra
maintainer[tip:perfcounters/urgent] ftrace: Fix perf-tracepoint OOPS
In reply to Peter ZijlstraCommit-ID: af6af30c0fcd77e621638e53ef8b176bca8bd3b4 Gitweb: http://git.kernel.org/tip/af6af30c0fcd77e621638e53ef8b176bca8bd3b4 Author: Peter Zijlstra <[email protected]> AuthorDate: Wed, 5 Aug 2009 20:41:04 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 6 Aug 2009 06:26:09 +0200
ftrace: Fix perf-tracepoint OOPS
Not all tracepoints are created equal, in specific the ftrace tracepoints are created with TRACE_EVENT_FORMAT() which does not generate the needed bits to tie them into perf counters.
For those events, don't create the 'id' file and fail ->profile_enable when their ID is specified through other means.
LKML-Reference: <1249497664.5890.4.camel@laptop>
--- include/linux/ftrace_event.h | 8 +++----- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-)
include/linux/ftrace_event.h +3 -5
@@ -119,11 +119,9 @@ struct ftrace_event_call {void *filter;void *mod; -#ifdef CONFIG_EVENT_PROFILE -atomic_t profile_count; -int (*profile_enable)(struct ftrace_event_call *); -void (*profile_disable)(struct ftrace_event_call *); -#endif +atomic_t profile_count; +int (*profile_enable)(struct ftrace_event_call *); +void (*profile_disable)(struct ftrace_event_call *); };#define MAX_FILTER_PRED 32
kernel/trace/trace_event_profile.c +1 -1
@@ -14,7 +14,7 @@ int ftrace_profile_enable(int event_id)mutex_lock (&event_mutex);list_for_each_entry (event, &ftrace_events, list) { -if (event->id == event_id) { +if (event->id == event_id && event->profile_enable) { ret = event->profile_enable (event);break ; }
kernel/trace/trace_events.c +1 -1
@@ -940,7 +940,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable); -if (call->id) +if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);
Re: [tip:perfcounters/urgent] ftrace: Fix perf-tracepoint OOPS
Quoted 18 lines
[ v2: fix build error in the !CONFIG_EVENT_PROFILE case ]Signed-off-by Ingo Molnar <[email protected]>--- include/linux/ftrace_event.h | 8 +++----- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-)include/linux/ftrace_event.h +0
-0@@ -119,11 +119,9 @@ struct ftrace_event_call {void *filter;void *mod;
include/linux/ftrace_event.h struct ftrace_event_call { · line 125
Quoted 10 lines
include/linux/ftrace_event.h +3
-5-#ifdef CONFIG_EVENT_PROFILE -atomic_t profile_count; -int (*profile_enable)(struct ftrace_event_call *); -void (*profile_disable)(struct ftrace_event_call *); -#endif +atomic_t profile_count; +int (*profile_enable)(struct ftrace_event_call *); +void (*profile_disable)(struct ftrace_event_call *); };
Ah, I would have added ifdefs around the below bit.
kernel/trace/trace_events.c event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, · line 943
Quoted 9 lines
kernel/trace/trace_events.c +0
-1@@ -940,7 +940,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable); -if (call->id)
#ifdef CONFIG_EVENT_PROFILE
kernel/trace/trace_events.c event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, · line 945
Quoted 3 lines
kernel/trace/trace_events.c +1
-0+if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);
#endif
Like that, but I guess this works too ;-)
Re: [tip:perfcounters/urgent] ftrace: Fix perf-tracepoint OOPS
Quoted 39 lines · Peter Zijlstra
Quoted 18 lines
[ v2: fix build error in the !CONFIG_EVENT_PROFILE case ]Signed-off-by Ingo Molnar <[email protected]>--- include/linux/ftrace_event.h | 8 +++----- kernel/trace/trace_event_profile.c | 2 +- kernel/trace/trace_events.c | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-)include/linux/ftrace_event.h +0
-0@@ -119,11 +119,9 @@ struct ftrace_event_call {void *filter;void *mod;include/linux/ftrace_event.h struct ftrace_event_call { · line 125
Quoted 10 lines
include/linux/ftrace_event.h +3
-5-#ifdef CONFIG_EVENT_PROFILE -atomic_t profile_count; -int (*profile_enable)(struct ftrace_event_call *); -void (*profile_disable)(struct ftrace_event_call *); -#endif +atomic_t profile_count; +int (*profile_enable)(struct ftrace_event_call *); +void (*profile_disable)(struct ftrace_event_call *); };Ah, I would have added ifdefs around the below bit.
Quoted 8 lines
kernel/trace/trace_events.c +0
-0@@ -940,7 +940,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, entry =trace_create_file ("enable" ,0644 , call->dir, call, enable);
Quoted 8 lines
kernel/trace/trace_events.c event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, · line 943
Quoted 1 line
kernel/trace/trace_events.c +0
-1-if (call->id)#ifdef
CONFIG_EVENT_PROFILEkernel/trace/trace_events.c event_create_dir(struct ftrace_event_call *call, struct dentry *d_events, · line 945
Quoted 3 lines
kernel/trace/trace_events.c +1
-0+if (call->id && call->profile_enable) entry =trace_create_file ("id" ,0444 , call->dir, call, id);#endif
Like that, but I guess this works too ;-)
i think CONFIG_EVENT_PROFILE should go away - it's clearly a core functionality of ftrace. Whenever we hit a Kconfig induced build breakage we should first consider reducing the Kconfig complexity a bit.
Ingo