[PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Steven Rostedt]

Ingo,

The following patch is in:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: tip/devel


Li Zefan (1):
      tracing/ftrace: add missing unlock in register_stat_tracer()

----
 kernel/trace/trace_stat.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
---------------------------
commit 6af81ac4701bf80dde4b723f28a777daefdf512c
Author: Li Zefan <lizf@cn.fujitsu.com>
Date:   Thu Jan 15 09:30:52 2009 +0800

    tracing/ftrace: add missing unlock in register_stat_tracer()
    
    We should unlock all_stat_sessions_mutex before returning failure.
    
    Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
    Signed-off-by: Steven Rostedt <srostedt@redhat.com>

diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index cb29282..8669f3c 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -73,8 +73,10 @@ int register_stat_tracer(struct tracer_stat *trace)
 	/* Already registered? */
 	mutex_lock(&all_stat_sessions_mutex);
 	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
-		if (node->ts == trace)
+		if (node->ts == trace) {
+			mutex_unlock(all_stat_sessions_mutex);
 			return -EINVAL;
+		}
 	}
 	mutex_unlock(&all_stat_sessions_mutex);
 

Re: [PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Ingo,
> 
> The following patch is in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: tip/devel
> 
> 
> Li Zefan (1):
>       tracing/ftrace: add missing unlock in register_stat_tracer()

hm, not there yet - forgot to push out?

	Ingo

Re: [PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Steven Rostedt]

On Thu, 15 Jan 2009, Ingo Molnar wrote:

> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > 
> > Ingo,
> > 
> > The following patch is in:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> > 
> >     branch: tip/devel
> > 
> > 
> > Li Zefan (1):
> >       tracing/ftrace: add missing unlock in register_stat_tracer()
> 
> hm, not there yet - forgot to push out?

Yep, sorry ;-)

Pushed, may take a few minutes to sync.

-- Steve

Re: [PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Steven Rostedt]

Ingo, revert this patch (or don't pull it yet)
I just noticed that it does not compile. I should not be doing git pulls 
at night :-(


On Wed, 14 Jan 2009, Steven Rostedt wrote:

> 
> Ingo,
> 
> The following patch is in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: tip/devel
> 
> 
> Li Zefan (1):
>       tracing/ftrace: add missing unlock in register_stat_tracer()
> 
> ----
>  kernel/trace/trace_stat.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> ---------------------------
> commit 6af81ac4701bf80dde4b723f28a777daefdf512c
> Author: Li Zefan <lizf@cn.fujitsu.com>
> Date:   Thu Jan 15 09:30:52 2009 +0800
> 
>     tracing/ftrace: add missing unlock in register_stat_tracer()
>     
>     We should unlock all_stat_sessions_mutex before returning failure.
>     
>     Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> 
> diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
> index cb29282..8669f3c 100644
> --- a/kernel/trace/trace_stat.c
> +++ b/kernel/trace/trace_stat.c
> @@ -73,8 +73,10 @@ int register_stat_tracer(struct tracer_stat *trace)
>  	/* Already registered? */
>  	mutex_lock(&all_stat_sessions_mutex);
>  	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
> -		if (node->ts == trace)
> +		if (node->ts == trace) {
> +			mutex_unlock(all_stat_sessions_mutex);

missing &

-- Steve

>  			return -EINVAL;
> +		}
>  	}
>  	mutex_unlock(&all_stat_sessions_mutex);
>  
> 
> 

Re: [PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Steven Rostedt]

On Thu, 15 Jan 2009, Steven Rostedt wrote:

> 
> Ingo, revert this patch (or don't pull it yet)
> I just noticed that it does not compile. I should not be doing git pulls 
> at night :-(

Probably explains why I did not push it. I think I noticed it last night, 
but sent out the email by mistake. I was too tired to fix it up.

-- Steve

Re: [PATCH] tracing/ftrace: add missing unlock in register_stat_tracer()

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Ingo, revert this patch (or don't pull it yet) I just noticed that it 
> does not compile. I should not be doing git pulls at night :-(

you just caught me in the middle of writing up a bugreport ;-)

kernel/trace/trace_stat.c:77: error: incompatible type for argument 1 of 'mutex_unlock'

easy enough to fix - and i did that.

	Ingo

[PATCH] tracing: trace_stat.c cleanup

[Ingo Molnar]

btw., while looking into the kernel/trace/trace_stat.c file to fix this 
build bug i also did the cleanups below.

	Ingo

------------>
>From 55922173f1f63903b6de03711ab8ff980cbe58d2 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 15 Jan 2009 11:31:21 +0100
Subject: [PATCH] tracing: trace_stat.c cleanup

Impact: cleanup

- whitespace / code alignment cleanups
- avoid unnecessary forward prototype by reordering functions

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/trace/trace_stat.c |  147 ++++++++++++++++++++++-----------------------
 1 files changed, 71 insertions(+), 76 deletions(-)

diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index 2110cea..eae9cef 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -17,16 +17,16 @@
 
 /* List of stat entries from a tracer */
 struct trace_stat_list {
-	struct list_head 	list;
-	void 			*stat;
+	struct list_head	list;
+	void			*stat;
 };
 
 /* A stat session is the stats output in one file */
 struct tracer_stat_session {
 	struct list_head	session_list;
-	struct tracer_stat 	*ts;
-	struct list_head 	stat_list;
-	struct mutex 		stat_mutex;
+	struct tracer_stat	*ts;
+	struct list_head	stat_list;
+	struct mutex		stat_mutex;
 	struct dentry		*file;
 };
 
@@ -35,7 +35,7 @@ static LIST_HEAD(all_stat_sessions);
 static DEFINE_MUTEX(all_stat_sessions_mutex);
 
 /* The root directory for all stat files */
-static struct dentry *stat_dir;
+static struct dentry		*stat_dir;
 
 
 static void reset_stat_session(struct tracer_stat_session *session)
@@ -56,71 +56,6 @@ static void destroy_session(struct tracer_stat_session *session)
 	kfree(session);
 }
 
-
-static int init_stat_file(struct tracer_stat_session *session);
-
-int register_stat_tracer(struct tracer_stat *trace)
-{
-	struct tracer_stat_session *session, *node, *tmp;
-	int ret;
-
-	if (!trace)
-		return -EINVAL;
-
-	if (!trace->stat_start || !trace->stat_next || !trace->stat_show)
-		return -EINVAL;
-
-	/* Already registered? */
-	mutex_lock(&all_stat_sessions_mutex);
-	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
-		if (node->ts == trace) {
-			mutex_unlock(&all_stat_sessions_mutex);
-			return -EINVAL;
-		}
-	}
-	mutex_unlock(&all_stat_sessions_mutex);
-
-	/* Init the session */
-	session = kmalloc(sizeof(struct tracer_stat_session), GFP_KERNEL);
-	if (!session)
-		return -ENOMEM;
-
-	session->ts = trace;
-	INIT_LIST_HEAD(&session->session_list);
-	INIT_LIST_HEAD(&session->stat_list);
-	mutex_init(&session->stat_mutex);
-	session->file = NULL;
-
-	ret = init_stat_file(session);
-	if (ret) {
-		destroy_session(session);
-		return ret;
-	}
-
-	/* Register */
-	mutex_lock(&all_stat_sessions_mutex);
-	list_add_tail(&session->session_list, &all_stat_sessions);
-	mutex_unlock(&all_stat_sessions_mutex);
-
-	return 0;
-}
-
-void unregister_stat_tracer(struct tracer_stat *trace)
-{
-	struct tracer_stat_session *node, *tmp;
-
-	mutex_lock(&all_stat_sessions_mutex);
-	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
-		if (node->ts == trace) {
-			list_del(&node->session_list);
-			destroy_session(node);
-			break;
-		}
-	}
-	mutex_unlock(&all_stat_sessions_mutex);
-}
-
-
 /*
  * For tracers that don't provide a stat_cmp callback.
  * This one will force an immediate insertion on tail of
@@ -252,10 +187,10 @@ static int stat_seq_show(struct seq_file *s, void *v)
 }
 
 static const struct seq_operations trace_stat_seq_ops = {
-	.start = stat_seq_start,
-	.next = stat_seq_next,
-	.stop = stat_seq_stop,
-	.show = stat_seq_show
+	.start		= stat_seq_start,
+	.next		= stat_seq_next,
+	.stop		= stat_seq_stop,
+	.show		= stat_seq_show
 };
 
 /* The session stat is refilled and resorted at each stat file opening */
@@ -275,7 +210,6 @@ static int tracing_stat_open(struct inode *inode, struct file *file)
 	return ret;
 }
 
-
 /*
  * Avoid consuming memory with our now useless list.
  */
@@ -322,3 +256,64 @@ static int init_stat_file(struct tracer_stat_session *session)
 		return -ENOMEM;
 	return 0;
 }
+
+int register_stat_tracer(struct tracer_stat *trace)
+{
+	struct tracer_stat_session *session, *node, *tmp;
+	int ret;
+
+	if (!trace)
+		return -EINVAL;
+
+	if (!trace->stat_start || !trace->stat_next || !trace->stat_show)
+		return -EINVAL;
+
+	/* Already registered? */
+	mutex_lock(&all_stat_sessions_mutex);
+	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
+		if (node->ts == trace) {
+			mutex_unlock(&all_stat_sessions_mutex);
+			return -EINVAL;
+		}
+	}
+	mutex_unlock(&all_stat_sessions_mutex);
+
+	/* Init the session */
+	session = kmalloc(sizeof(struct tracer_stat_session), GFP_KERNEL);
+	if (!session)
+		return -ENOMEM;
+
+	session->ts = trace;
+	INIT_LIST_HEAD(&session->session_list);
+	INIT_LIST_HEAD(&session->stat_list);
+	mutex_init(&session->stat_mutex);
+	session->file = NULL;
+
+	ret = init_stat_file(session);
+	if (ret) {
+		destroy_session(session);
+		return ret;
+	}
+
+	/* Register */
+	mutex_lock(&all_stat_sessions_mutex);
+	list_add_tail(&session->session_list, &all_stat_sessions);
+	mutex_unlock(&all_stat_sessions_mutex);
+
+	return 0;
+}
+
+void unregister_stat_tracer(struct tracer_stat *trace)
+{
+	struct tracer_stat_session *node, *tmp;
+
+	mutex_lock(&all_stat_sessions_mutex);
+	list_for_each_entry_safe(node, tmp, &all_stat_sessions, session_list) {
+		if (node->ts == trace) {
+			list_del(&node->session_list);
+			destroy_session(node);
+			break;
+		}
+	}
+	mutex_unlock(&all_stat_sessions_mutex);
+}