Re: system gets stuck in a lock during boot

[Li Zefan <lizf@cn.fujitsu.com>]

CC: Peter, Ingo

> Hey alright 2.6.31-rc6 booted.
> 
> I must admit, I've been afraid of doing
> a git bisect for some time now. but after learning
> to do a bisect, I cant see a better solution to finding a
> bug(biggest issue I see is having a huge .config).
> 
> Anyways the culprit for my issue is this commit:
> 
> af6af30c0fcd77e621638e53ef8b176bca8bd3b4
> 
> reverting this on rc6 booted the machine up.
> 

Thanks!

Hmm...I don't see how this commit can cause oops:

> @@ -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 *);

In your .config, CONFIG_EVENT_PROFILE=y, so this change makes
no difference.

>  };
> 
>  #define MAX_FILTER_PRED                32
> diff --git a/kernel/trace/trace_event_profile.c b/kernel/trace/trace_event_profil
> index 5b5895a..11ba5bb 100644
> --- a/kernel/trace/trace_event_profile.c
> +++ b/kernel/trace/trace_event_profile.c
> @@ -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) {

You will run into this hunk only when you run the perf tool,
so should have nothing to do with the boot failure.

>                         ret = event->profile_enable(event);
>                         break;
>                 }
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 23d2972..e75276a 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -940,7 +940,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentr
>                 entry = trace_create_file("enable", 0644, call->dir, call,
>                                           enable);
> 
> -       if (call->id)
> +       if (call->id && call->profile_enable)

We do an extra check on ->profile_enable, shouldn't cause bug..

>                 entry = trace_create_file("id", 0444, call->dir, call,
>                                           id);

Any way, I don't think this commit does the right thing:

  - If CONFIG_EVENT_PROFILE=y, we'll create events/<dir>/<event>/id,
    except events/ftrace/<event>/id.

  - if CONFIG_EVENT_PROFILE=n, there's no 'id' file at all!

I think it's better to skip ftrace/ dir in perf tool code, instead of
skipping creating id files in ftrace code.

can you try this patch:

---
 include/linux/ftrace_event.h   |    2 ++
 kernel/trace/trace_events.c    |    2 +-
 tools/perf/util/parse-events.c |    8 +++++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index a81170d..398c925 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -121,9 +121,11 @@ 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
 };
 
 #define MAX_FILTER_PRED		32
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e75276a..23d2972 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -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 && call->profile_enable)
+	if (call->id)
 		entry = trace_create_file("id", 0444, call->dir, call,
 					  id);
 
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0441784..1077ebd 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -113,13 +113,19 @@ static unsigned long hw_cache_stat[C(MAX)] = {
  [C(BPU)]	= (CACHE_READ),
 };
 
+static int is_tp_subsystem(struct dirent *sys_dir)
+{
+	return !!strcmp(sys_dir->d_name, "ftrace");
+}
+
 #define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	       \
 	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	       \
 	if (snprintf(file, MAXPATHLEN, "%s/%s", debugfs_path,	       	       \
 			sys_dirent.d_name) &&		       		       \
 	   (!stat(file, &st)) && (S_ISDIR(st.st_mode)) &&		       \
 	   (strcmp(sys_dirent.d_name, ".")) &&				       \
-	   (strcmp(sys_dirent.d_name, "..")))
+	   (strcmp(sys_dirent.d_name, "..")) &&				       \
+	   (is_tp_subsystem(&sys_dirent)))
 
 static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
 {