[PATCH 1/3][RESEND] function-graph: use ftrace_graph_funcs directly

[PATCH 1/3][RESEND] function-graph: use ftrace_graph_funcs directly

[Li Zefan]

No need to store ftrace_graph_funcs in file->private.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---

The first 2 patches were sent 2 month ago..

---
 kernel/trace/ftrace.c |   23 ++++-------------------
 1 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cc615f8..c71e91b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2414,11 +2414,9 @@ unsigned long ftrace_graph_funcs[FTRACE_GRAPH_MAX_FUNCS] __read_mostly;
 static void *
 __g_next(struct seq_file *m, loff_t *pos)
 {
-	unsigned long *array = m->private;
-
 	if (*pos >= ftrace_graph_count)
 		return NULL;
-	return &array[*pos];
+	return &ftrace_graph_funcs[*pos];
 }
 
 static void *
@@ -2482,16 +2480,10 @@ ftrace_graph_open(struct inode *inode, struct file *file)
 		ftrace_graph_count = 0;
 		memset(ftrace_graph_funcs, 0, sizeof(ftrace_graph_funcs));
 	}
+	mutex_unlock(&graph_lock);
 
-	if (file->f_mode & FMODE_READ) {
+	if (file->f_mode & FMODE_READ)
 		ret = seq_open(file, &ftrace_graph_seq_ops);
-		if (!ret) {
-			struct seq_file *m = file->private_data;
-			m->private = ftrace_graph_funcs;
-		}
-	} else
-		file->private_data = ftrace_graph_funcs;
-	mutex_unlock(&graph_lock);
 
 	return ret;
 }
@@ -2560,7 +2552,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		   size_t cnt, loff_t *ppos)
 {
 	struct trace_parser parser;
-	unsigned long *array;
 	size_t read = 0;
 	ssize_t ret;
 
@@ -2574,12 +2565,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		goto out;
 	}
 
-	if (file->f_mode & FMODE_READ) {
-		struct seq_file *m = file->private_data;
-		array = m->private;
-	} else
-		array = file->private_data;
-
 	if (trace_parser_get_init(&parser, FTRACE_BUFF_MAX)) {
 		ret = -ENOMEM;
 		goto out;
@@ -2591,7 +2576,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		parser.buffer[parser.idx] = 0;
 
 		/* we allow only one expression at a time */
-		ret = ftrace_set_func(array, &ftrace_graph_count,
+		ret = ftrace_set_func(ftrace_graph_funcs, &ftrace_graph_count,
 					parser.buffer);
 		if (ret)
 			goto out;
-- 
1.6.3

[PATCH 2/3][RESEND] tracing: remove max_tracer_type_len

[Li Zefan]

Limit the length of a tracer's name within 100 chars, and then we
don't have to play with max_tracer_type_len.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace.c |   49 ++++++++++++++++---------------------------------
 1 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index fd52a19..8613080 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -125,13 +125,13 @@ int ftrace_dump_on_oops;
 
 static int tracing_set_tracer(const char *buf);
 
-#define BOOTUP_TRACER_SIZE		100
-static char bootup_tracer_buf[BOOTUP_TRACER_SIZE] __initdata;
+#define MAX_TRACER_SIZE		100
+static char bootup_tracer_buf[MAX_TRACER_SIZE] __initdata;
 static char *default_bootup_tracer;
 
 static int __init set_ftrace(char *str)
 {
-	strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE);
+	strncpy(bootup_tracer_buf, str, MAX_TRACER_SIZE);
 	default_bootup_tracer = bootup_tracer_buf;
 	/* We are using ftrace early, expand it */
 	ring_buffer_expanded = 1;
@@ -242,13 +242,6 @@ static struct tracer		*trace_types __read_mostly;
 static struct tracer		*current_trace __read_mostly;
 
 /*
- * max_tracer_type_len is used to simplify the allocating of
- * buffers to read userspace tracer names. We keep track of
- * the longest tracer name registered.
- */
-static int			max_tracer_type_len;
-
-/*
  * trace_types_lock is used to protect the trace_types list.
  * This lock is also used to keep user access serialized.
  * Accesses from userspace will grab this lock while userspace
@@ -619,7 +612,6 @@ __releases(kernel_lock)
 __acquires(kernel_lock)
 {
 	struct tracer *t;
-	int len;
 	int ret = 0;
 
 	if (!type->name) {
@@ -627,6 +619,11 @@ __acquires(kernel_lock)
 		return -1;
 	}
 
+	if (strlen(type->name) > MAX_TRACER_SIZE) {
+		pr_info("Tracer has a name longer than %d\n", MAX_TRACER_SIZE);
+		return -1;
+	}
+
 	/*
 	 * When this gets called we hold the BKL which means that
 	 * preemption is disabled. Various trace selftests however
@@ -641,7 +638,7 @@ __acquires(kernel_lock)
 	for (t = trace_types; t; t = t->next) {
 		if (strcmp(type->name, t->name) == 0) {
 			/* already found */
-			pr_info("Trace %s already registered\n",
+			pr_info("Tracer %s already registered\n",
 				type->name);
 			ret = -1;
 			goto out;
@@ -692,9 +689,6 @@ __acquires(kernel_lock)
 
 	type->next = trace_types;
 	trace_types = type;
-	len = strlen(type->name);
-	if (len > max_tracer_type_len)
-		max_tracer_type_len = len;
 
  out:
 	tracing_selftest_running = false;
@@ -703,7 +697,7 @@ __acquires(kernel_lock)
 	if (ret || !default_bootup_tracer)
 		goto out_unlock;
 
-	if (strncmp(default_bootup_tracer, type->name, BOOTUP_TRACER_SIZE))
+	if (strncmp(default_bootup_tracer, type->name, MAX_TRACER_SIZE))
 		goto out_unlock;
 
 	printk(KERN_INFO "Starting tracer '%s'\n", type->name);
@@ -725,14 +719,13 @@ __acquires(kernel_lock)
 void unregister_tracer(struct tracer *type)
 {
 	struct tracer **t;
-	int len;
 
 	mutex_lock(&trace_types_lock);
 	for (t = &trace_types; *t; t = &(*t)->next) {
 		if (*t == type)
 			goto found;
 	}
-	pr_info("Trace %s not registered\n", type->name);
+	pr_info("Tracer %s not registered\n", type->name);
 	goto out;
 
  found:
@@ -745,17 +738,7 @@ void unregister_tracer(struct tracer *type)
 			current_trace->stop(&global_trace);
 		current_trace = &nop_trace;
 	}
-
-	if (strlen(type->name) != max_tracer_type_len)
-		goto out;
-
-	max_tracer_type_len = 0;
-	for (t = &trace_types; *t; t = &(*t)->next) {
-		len = strlen((*t)->name);
-		if (len > max_tracer_type_len)
-			max_tracer_type_len = len;
-	}
- out:
+out:
 	mutex_unlock(&trace_types_lock);
 }
 
@@ -2604,7 +2587,7 @@ static ssize_t
 tracing_set_trace_read(struct file *filp, char __user *ubuf,
 		       size_t cnt, loff_t *ppos)
 {
-	char buf[max_tracer_type_len+2];
+	char buf[MAX_TRACER_SIZE+2];
 	int r;
 
 	mutex_lock(&trace_types_lock);
@@ -2754,15 +2737,15 @@ static ssize_t
 tracing_set_trace_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
 {
-	char buf[max_tracer_type_len+1];
+	char buf[MAX_TRACER_SIZE+1];
 	int i;
 	size_t ret;
 	int err;
 
 	ret = cnt;
 
-	if (cnt > max_tracer_type_len)
-		cnt = max_tracer_type_len;
+	if (cnt > MAX_TRACER_SIZE)
+		cnt = MAX_TRACER_SIZE;
 
 	if (copy_from_user(&buf, ubuf, cnt))
 		return -EFAULT;
-- 
1.6.3

[PATCH 3/3] tracing/events: use list_for_entry_continue

[Li Zefan]

Simplify s_next() and t_next().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace_events.c |   49 +++++++++++++-----------------------------
 1 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 56c260b..6f03c8a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -271,42 +271,32 @@ ftrace_event_write(struct file *file, const char __user *ubuf,
 static void *
 t_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct list_head *list = m->private;
-	struct ftrace_event_call *call;
+	struct ftrace_event_call *call = v;
 
 	(*pos)++;
 
-	for (;;) {
-		if (list == &ftrace_events)
-			return NULL;
-
-		call = list_entry(list, struct ftrace_event_call, list);
-
+	list_for_each_entry_continue(call, &ftrace_events, list) {
 		/*
 		 * The ftrace subsystem is for showing formats only.
 		 * They can not be enabled or disabled via the event files.
 		 */
 		if (call->regfunc)
-			break;
-
-		list = list->next;
+			return call;
 	}
 
-	m->private = list->next;
-
-	return call;
+	return NULL;
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)
 {
-	struct ftrace_event_call *call = NULL;
+	struct ftrace_event_call *call;
 	loff_t l;
 
 	mutex_lock(&event_mutex);
 
-	m->private = ftrace_events.next;
+	call = list_entry(&ftrace_events, struct ftrace_event_call, list);
 	for (l = 0; l <= *pos; ) {
-		call = t_next(m, NULL, &l);
+		call = t_next(m, call, &l);
 		if (!call)
 			break;
 	}
@@ -316,37 +306,28 @@ static void *t_start(struct seq_file *m, loff_t *pos)
 static void *
 s_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct list_head *list = m->private;
-	struct ftrace_event_call *call;
+	struct ftrace_event_call *call = v;
 
 	(*pos)++;
 
- retry:
-	if (list == &ftrace_events)
-		return NULL;
-
-	call = list_entry(list, struct ftrace_event_call, list);
-
-	if (!call->enabled) {
-		list = list->next;
-		goto retry;
+	list_for_each_entry_continue(call, &ftrace_events, list) {
+		if (call->enabled)
+			return call;
 	}
 
-	m->private = list->next;
-
-	return call;
+	return NULL;
 }
 
 static void *s_start(struct seq_file *m, loff_t *pos)
 {
-	struct ftrace_event_call *call = NULL;
+	struct ftrace_event_call *call;
 	loff_t l;
 
 	mutex_lock(&event_mutex);
 
-	m->private = ftrace_events.next;
+	call = list_entry(&ftrace_events, struct ftrace_event_call, list);
 	for (l = 0; l <= *pos; ) {
-		call = s_next(m, NULL, &l);
+		call = s_next(m, call, &l);
 		if (!call)
 			break;
 	}
-- 
1.6.3

Re: [PATCH 2/3][RESEND] tracing: remove max_tracer_type_len

[Frédéric Weisbecker]

2009/9/18 Li Zefan <lizf@cn.fujitsu.com>:
> Limit the length of a tracer's name within 100 chars, and then we
> don't have to play with max_tracer_type_len.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>


Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
>  kernel/trace/trace.c |   49 ++++++++++++++++---------------------------------
>  1 files changed, 16 insertions(+), 33 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index fd52a19..8613080 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -125,13 +125,13 @@ int ftrace_dump_on_oops;
>
>  static int tracing_set_tracer(const char *buf);
>
> -#define BOOTUP_TRACER_SIZE             100
> -static char bootup_tracer_buf[BOOTUP_TRACER_SIZE] __initdata;
> +#define MAX_TRACER_SIZE                100
> +static char bootup_tracer_buf[MAX_TRACER_SIZE] __initdata;
>  static char *default_bootup_tracer;
>
>  static int __init set_ftrace(char *str)
>  {
> -       strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE);
> +       strncpy(bootup_tracer_buf, str, MAX_TRACER_SIZE);
>        default_bootup_tracer = bootup_tracer_buf;
>        /* We are using ftrace early, expand it */
>        ring_buffer_expanded = 1;
> @@ -242,13 +242,6 @@ static struct tracer               *trace_types __read_mostly;
>  static struct tracer           *current_trace __read_mostly;
>
>  /*
> - * max_tracer_type_len is used to simplify the allocating of
> - * buffers to read userspace tracer names. We keep track of
> - * the longest tracer name registered.
> - */
> -static int                     max_tracer_type_len;
> -
> -/*
>  * trace_types_lock is used to protect the trace_types list.
>  * This lock is also used to keep user access serialized.
>  * Accesses from userspace will grab this lock while userspace
> @@ -619,7 +612,6 @@ __releases(kernel_lock)
>  __acquires(kernel_lock)
>  {
>        struct tracer *t;
> -       int len;
>        int ret = 0;
>
>        if (!type->name) {
> @@ -627,6 +619,11 @@ __acquires(kernel_lock)
>                return -1;
>        }
>
> +       if (strlen(type->name) > MAX_TRACER_SIZE) {
> +               pr_info("Tracer has a name longer than %d\n", MAX_TRACER_SIZE);
> +               return -1;
> +       }
> +
>        /*
>         * When this gets called we hold the BKL which means that
>         * preemption is disabled. Various trace selftests however
> @@ -641,7 +638,7 @@ __acquires(kernel_lock)
>        for (t = trace_types; t; t = t->next) {
>                if (strcmp(type->name, t->name) == 0) {
>                        /* already found */
> -                       pr_info("Trace %s already registered\n",
> +                       pr_info("Tracer %s already registered\n",
>                                type->name);
>                        ret = -1;
>                        goto out;
> @@ -692,9 +689,6 @@ __acquires(kernel_lock)
>
>        type->next = trace_types;
>        trace_types = type;
> -       len = strlen(type->name);
> -       if (len > max_tracer_type_len)
> -               max_tracer_type_len = len;
>
>  out:
>        tracing_selftest_running = false;
> @@ -703,7 +697,7 @@ __acquires(kernel_lock)
>        if (ret || !default_bootup_tracer)
>                goto out_unlock;
>
> -       if (strncmp(default_bootup_tracer, type->name, BOOTUP_TRACER_SIZE))
> +       if (strncmp(default_bootup_tracer, type->name, MAX_TRACER_SIZE))
>                goto out_unlock;
>
>        printk(KERN_INFO "Starting tracer '%s'\n", type->name);
> @@ -725,14 +719,13 @@ __acquires(kernel_lock)
>  void unregister_tracer(struct tracer *type)
>  {
>        struct tracer **t;
> -       int len;
>
>        mutex_lock(&trace_types_lock);
>        for (t = &trace_types; *t; t = &(*t)->next) {
>                if (*t == type)
>                        goto found;
>        }
> -       pr_info("Trace %s not registered\n", type->name);
> +       pr_info("Tracer %s not registered\n", type->name);
>        goto out;
>
>  found:
> @@ -745,17 +738,7 @@ void unregister_tracer(struct tracer *type)
>                        current_trace->stop(&global_trace);
>                current_trace = &nop_trace;
>        }
> -
> -       if (strlen(type->name) != max_tracer_type_len)
> -               goto out;
> -
> -       max_tracer_type_len = 0;
> -       for (t = &trace_types; *t; t = &(*t)->next) {
> -               len = strlen((*t)->name);
> -               if (len > max_tracer_type_len)
> -                       max_tracer_type_len = len;
> -       }
> - out:
> +out:
>        mutex_unlock(&trace_types_lock);
>  }
>
> @@ -2604,7 +2587,7 @@ static ssize_t
>  tracing_set_trace_read(struct file *filp, char __user *ubuf,
>                       size_t cnt, loff_t *ppos)
>  {
> -       char buf[max_tracer_type_len+2];
> +       char buf[MAX_TRACER_SIZE+2];
>        int r;
>
>        mutex_lock(&trace_types_lock);
> @@ -2754,15 +2737,15 @@ static ssize_t
>  tracing_set_trace_write(struct file *filp, const char __user *ubuf,
>                        size_t cnt, loff_t *ppos)
>  {
> -       char buf[max_tracer_type_len+1];
> +       char buf[MAX_TRACER_SIZE+1];
>        int i;
>        size_t ret;
>        int err;
>
>        ret = cnt;
>
> -       if (cnt > max_tracer_type_len)
> -               cnt = max_tracer_type_len;
> +       if (cnt > MAX_TRACER_SIZE)
> +               cnt = MAX_TRACER_SIZE;
>
>        if (copy_from_user(&buf, ubuf, cnt))
>                return -EFAULT;
> --
> 1.6.3
>
>

Re: [PATCH 2/3][RESEND] tracing: remove max_tracer_type_len

[Steven Rostedt]

On Fri, 2009-09-18 at 14:06 +0800, Li Zefan wrote:
> Limit the length of a tracer's name within 100 chars, and then we
> don't have to play with max_tracer_type_len.

What? You don't like my over-engineering? ;-)

> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Thanks, I'll pull them.

Ingo, This patch series is mainly cleanups. Do we still have time to add
this for v2.6.32, or should we wait till 33?.

-- Steve

Re: [PATCH 1/3][RESEND] function-graph: use ftrace_graph_funcs directly

[Frederic Weisbecker]

On Fri, Sep 18, 2009 at 02:06:28PM +0800, Li Zefan wrote:
> No need to store ftrace_graph_funcs in file->private.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>


Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
> 
> The first 2 patches were sent 2 month ago..
> 
> ---
>  kernel/trace/ftrace.c |   23 ++++-------------------
>  1 files changed, 4 insertions(+), 19 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index cc615f8..c71e91b 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2414,11 +2414,9 @@ unsigned long ftrace_graph_funcs[FTRACE_GRAPH_MAX_FUNCS] __read_mostly;
>  static void *
>  __g_next(struct seq_file *m, loff_t *pos)
>  {
> -	unsigned long *array = m->private;
> -
>  	if (*pos >= ftrace_graph_count)
>  		return NULL;
> -	return &array[*pos];
> +	return &ftrace_graph_funcs[*pos];
>  }
>  
>  static void *
> @@ -2482,16 +2480,10 @@ ftrace_graph_open(struct inode *inode, struct file *file)
>  		ftrace_graph_count = 0;
>  		memset(ftrace_graph_funcs, 0, sizeof(ftrace_graph_funcs));
>  	}
> +	mutex_unlock(&graph_lock);
>  
> -	if (file->f_mode & FMODE_READ) {
> +	if (file->f_mode & FMODE_READ)
>  		ret = seq_open(file, &ftrace_graph_seq_ops);
> -		if (!ret) {
> -			struct seq_file *m = file->private_data;
> -			m->private = ftrace_graph_funcs;
> -		}
> -	} else
> -		file->private_data = ftrace_graph_funcs;
> -	mutex_unlock(&graph_lock);
>  
>  	return ret;
>  }
> @@ -2560,7 +2552,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
>  		   size_t cnt, loff_t *ppos)
>  {
>  	struct trace_parser parser;
> -	unsigned long *array;
>  	size_t read = 0;
>  	ssize_t ret;
>  
> @@ -2574,12 +2565,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
>  		goto out;
>  	}
>  
> -	if (file->f_mode & FMODE_READ) {
> -		struct seq_file *m = file->private_data;
> -		array = m->private;
> -	} else
> -		array = file->private_data;
> -
>  	if (trace_parser_get_init(&parser, FTRACE_BUFF_MAX)) {
>  		ret = -ENOMEM;
>  		goto out;
> @@ -2591,7 +2576,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
>  		parser.buffer[parser.idx] = 0;
>  
>  		/* we allow only one expression at a time */
> -		ret = ftrace_set_func(array, &ftrace_graph_count,
> +		ret = ftrace_set_func(ftrace_graph_funcs, &ftrace_graph_count,
>  					parser.buffer);
>  		if (ret)
>  			goto out;
> -- 
> 1.6.3
> 

Re: [PATCH 3/3] tracing/events: use list_for_entry_continue

[Frederic Weisbecker]

On Fri, Sep 18, 2009 at 02:07:05PM +0800, Li Zefan wrote:
> Simplify s_next() and t_next().
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---




Nice.
I wonder if that can be even more simplified with
the following existing seq helpers:

extern struct list_head *seq_list_start(struct list_head *head,
		loff_t pos);
extern struct list_head *seq_list_start_head(struct list_head *head,
		loff_t pos);
extern struct list_head *seq_list_next(void *v, struct list_head *head,
		loff_t *ppos);


That said, it's already a nice cleanup.

Re: [PATCH 2/3][RESEND] tracing: remove max_tracer_type_len

[Ingo Molnar]

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

> On Fri, 2009-09-18 at 14:06 +0800, Li Zefan wrote:
> > Limit the length of a tracer's name within 100 chars, and then we
> > don't have to play with max_tracer_type_len.
> 
> What? You don't like my over-engineering? ;-)
> 
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> 
> Thanks, I'll pull them.
> 
> Ingo, This patch series is mainly cleanups. Do we still have time to 
> add this for v2.6.32, or should we wait till 33?.

Yeah, the tracing tree's been pretty good so far, i think we can do 
cleanups still.

	Ingo

Re: [PATCH 3/3] tracing/events: use list_for_entry_continue

[Steven Rostedt]

On Fri, 2009-09-18 at 17:09 +0200, Frederic Weisbecker wrote:
> On Fri, Sep 18, 2009 at 02:07:05PM +0800, Li Zefan wrote:
> > Simplify s_next() and t_next().
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > ---
> 
> 
> 
> 
> Nice.
> I wonder if that can be even more simplified with
> the following existing seq helpers:
> 
> extern struct list_head *seq_list_start(struct list_head *head,
> 		loff_t pos);
> extern struct list_head *seq_list_start_head(struct list_head *head,
> 		loff_t pos);
> extern struct list_head *seq_list_next(void *v, struct list_head *head,
> 		loff_t *ppos);

This could be another patch someday.

> 
> 
> That said, it's already a nice cleanup.
> 

I'll take this as an Acked-by then?

-- Steve

[tip:tracing/urgent] function-graph: use ftrace_graph_funcs directly

[tip-bot for Li Zefan]

Commit-ID:  a4ec5e0c2681b8cf99ddabf118705847f7460f19
Gitweb:     http://git.kernel.org/tip/a4ec5e0c2681b8cf99ddabf118705847f7460f19
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Fri, 18 Sep 2009 14:06:28 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Sat, 19 Sep 2009 11:26:54 -0400

function-graph: use ftrace_graph_funcs directly

No need to store ftrace_graph_funcs in file->private.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4AB32364.7020602@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>


---
 kernel/trace/ftrace.c |   23 ++++-------------------
 1 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cc615f8..c71e91b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2414,11 +2414,9 @@ unsigned long ftrace_graph_funcs[FTRACE_GRAPH_MAX_FUNCS] __read_mostly;
 static void *
 __g_next(struct seq_file *m, loff_t *pos)
 {
-	unsigned long *array = m->private;
-
 	if (*pos >= ftrace_graph_count)
 		return NULL;
-	return &array[*pos];
+	return &ftrace_graph_funcs[*pos];
 }
 
 static void *
@@ -2482,16 +2480,10 @@ ftrace_graph_open(struct inode *inode, struct file *file)
 		ftrace_graph_count = 0;
 		memset(ftrace_graph_funcs, 0, sizeof(ftrace_graph_funcs));
 	}
+	mutex_unlock(&graph_lock);
 
-	if (file->f_mode & FMODE_READ) {
+	if (file->f_mode & FMODE_READ)
 		ret = seq_open(file, &ftrace_graph_seq_ops);
-		if (!ret) {
-			struct seq_file *m = file->private_data;
-			m->private = ftrace_graph_funcs;
-		}
-	} else
-		file->private_data = ftrace_graph_funcs;
-	mutex_unlock(&graph_lock);
 
 	return ret;
 }
@@ -2560,7 +2552,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		   size_t cnt, loff_t *ppos)
 {
 	struct trace_parser parser;
-	unsigned long *array;
 	size_t read = 0;
 	ssize_t ret;
 
@@ -2574,12 +2565,6 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		goto out;
 	}
 
-	if (file->f_mode & FMODE_READ) {
-		struct seq_file *m = file->private_data;
-		array = m->private;
-	} else
-		array = file->private_data;
-
 	if (trace_parser_get_init(&parser, FTRACE_BUFF_MAX)) {
 		ret = -ENOMEM;
 		goto out;
@@ -2591,7 +2576,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 		parser.buffer[parser.idx] = 0;
 
 		/* we allow only one expression at a time */
-		ret = ftrace_set_func(array, &ftrace_graph_count,
+		ret = ftrace_set_func(ftrace_graph_funcs, &ftrace_graph_count,
 					parser.buffer);
 		if (ret)
 			goto out;

[tip:tracing/urgent] tracing: remove max_tracer_type_len

[tip-bot for Li Zefan]

Commit-ID:  ee6c2c1bd15e60a442d1861b66285f112ce4f25c
Gitweb:     http://git.kernel.org/tip/ee6c2c1bd15e60a442d1861b66285f112ce4f25c
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Fri, 18 Sep 2009 14:06:47 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Sat, 19 Sep 2009 11:28:19 -0400

tracing: remove max_tracer_type_len

Limit the length of a tracer's name within 100 chars, and then we
don't have to play with max_tracer_type_len.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4AB32377.9020601@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>


---
 kernel/trace/trace.c |   49 ++++++++++++++++---------------------------------
 1 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index fd52a19..8613080 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -125,13 +125,13 @@ int ftrace_dump_on_oops;
 
 static int tracing_set_tracer(const char *buf);
 
-#define BOOTUP_TRACER_SIZE		100
-static char bootup_tracer_buf[BOOTUP_TRACER_SIZE] __initdata;
+#define MAX_TRACER_SIZE		100
+static char bootup_tracer_buf[MAX_TRACER_SIZE] __initdata;
 static char *default_bootup_tracer;
 
 static int __init set_ftrace(char *str)
 {
-	strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE);
+	strncpy(bootup_tracer_buf, str, MAX_TRACER_SIZE);
 	default_bootup_tracer = bootup_tracer_buf;
 	/* We are using ftrace early, expand it */
 	ring_buffer_expanded = 1;
@@ -242,13 +242,6 @@ static struct tracer		*trace_types __read_mostly;
 static struct tracer		*current_trace __read_mostly;
 
 /*
- * max_tracer_type_len is used to simplify the allocating of
- * buffers to read userspace tracer names. We keep track of
- * the longest tracer name registered.
- */
-static int			max_tracer_type_len;
-
-/*
  * trace_types_lock is used to protect the trace_types list.
  * This lock is also used to keep user access serialized.
  * Accesses from userspace will grab this lock while userspace
@@ -619,7 +612,6 @@ __releases(kernel_lock)
 __acquires(kernel_lock)
 {
 	struct tracer *t;
-	int len;
 	int ret = 0;
 
 	if (!type->name) {
@@ -627,6 +619,11 @@ __acquires(kernel_lock)
 		return -1;
 	}
 
+	if (strlen(type->name) > MAX_TRACER_SIZE) {
+		pr_info("Tracer has a name longer than %d\n", MAX_TRACER_SIZE);
+		return -1;
+	}
+
 	/*
 	 * When this gets called we hold the BKL which means that
 	 * preemption is disabled. Various trace selftests however
@@ -641,7 +638,7 @@ __acquires(kernel_lock)
 	for (t = trace_types; t; t = t->next) {
 		if (strcmp(type->name, t->name) == 0) {
 			/* already found */
-			pr_info("Trace %s already registered\n",
+			pr_info("Tracer %s already registered\n",
 				type->name);
 			ret = -1;
 			goto out;
@@ -692,9 +689,6 @@ __acquires(kernel_lock)
 
 	type->next = trace_types;
 	trace_types = type;
-	len = strlen(type->name);
-	if (len > max_tracer_type_len)
-		max_tracer_type_len = len;
 
  out:
 	tracing_selftest_running = false;
@@ -703,7 +697,7 @@ __acquires(kernel_lock)
 	if (ret || !default_bootup_tracer)
 		goto out_unlock;
 
-	if (strncmp(default_bootup_tracer, type->name, BOOTUP_TRACER_SIZE))
+	if (strncmp(default_bootup_tracer, type->name, MAX_TRACER_SIZE))
 		goto out_unlock;
 
 	printk(KERN_INFO "Starting tracer '%s'\n", type->name);
@@ -725,14 +719,13 @@ __acquires(kernel_lock)
 void unregister_tracer(struct tracer *type)
 {
 	struct tracer **t;
-	int len;
 
 	mutex_lock(&trace_types_lock);
 	for (t = &trace_types; *t; t = &(*t)->next) {
 		if (*t == type)
 			goto found;
 	}
-	pr_info("Trace %s not registered\n", type->name);
+	pr_info("Tracer %s not registered\n", type->name);
 	goto out;
 
  found:
@@ -745,17 +738,7 @@ void unregister_tracer(struct tracer *type)
 			current_trace->stop(&global_trace);
 		current_trace = &nop_trace;
 	}
-
-	if (strlen(type->name) != max_tracer_type_len)
-		goto out;
-
-	max_tracer_type_len = 0;
-	for (t = &trace_types; *t; t = &(*t)->next) {
-		len = strlen((*t)->name);
-		if (len > max_tracer_type_len)
-			max_tracer_type_len = len;
-	}
- out:
+out:
 	mutex_unlock(&trace_types_lock);
 }
 
@@ -2604,7 +2587,7 @@ static ssize_t
 tracing_set_trace_read(struct file *filp, char __user *ubuf,
 		       size_t cnt, loff_t *ppos)
 {
-	char buf[max_tracer_type_len+2];
+	char buf[MAX_TRACER_SIZE+2];
 	int r;
 
 	mutex_lock(&trace_types_lock);
@@ -2754,15 +2737,15 @@ static ssize_t
 tracing_set_trace_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
 {
-	char buf[max_tracer_type_len+1];
+	char buf[MAX_TRACER_SIZE+1];
 	int i;
 	size_t ret;
 	int err;
 
 	ret = cnt;
 
-	if (cnt > max_tracer_type_len)
-		cnt = max_tracer_type_len;
+	if (cnt > MAX_TRACER_SIZE)
+		cnt = MAX_TRACER_SIZE;
 
 	if (copy_from_user(&buf, ubuf, cnt))
 		return -EFAULT;

[tip:tracing/urgent] tracing/events: use list_for_entry_continue

[tip-bot for Li Zefan]

Commit-ID:  30bd39cd6244ffe3258c9203405286ef77b1c4eb
Gitweb:     http://git.kernel.org/tip/30bd39cd6244ffe3258c9203405286ef77b1c4eb
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Fri, 18 Sep 2009 14:07:05 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Sat, 19 Sep 2009 11:30:40 -0400

tracing/events: use list_for_entry_continue

Simplify s_next() and t_next().

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4AB32389.1030005@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>


---
 kernel/trace/trace_events.c |   49 +++++++++++++-----------------------------
 1 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 56c260b..6f03c8a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -271,42 +271,32 @@ ftrace_event_write(struct file *file, const char __user *ubuf,
 static void *
 t_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct list_head *list = m->private;
-	struct ftrace_event_call *call;
+	struct ftrace_event_call *call = v;
 
 	(*pos)++;
 
-	for (;;) {
-		if (list == &ftrace_events)
-			return NULL;
-
-		call = list_entry(list, struct ftrace_event_call, list);
-
+	list_for_each_entry_continue(call, &ftrace_events, list) {
 		/*
 		 * The ftrace subsystem is for showing formats only.
 		 * They can not be enabled or disabled via the event files.
 		 */
 		if (call->regfunc)
-			break;
-
-		list = list->next;
+			return call;
 	}
 
-	m->private = list->next;
-
-	return call;
+	return NULL;
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)
 {
-	struct ftrace_event_call *call = NULL;
+	struct ftrace_event_call *call;
 	loff_t l;
 
 	mutex_lock(&event_mutex);
 
-	m->private = ftrace_events.next;
+	call = list_entry(&ftrace_events, struct ftrace_event_call, list);
 	for (l = 0; l <= *pos; ) {
-		call = t_next(m, NULL, &l);
+		call = t_next(m, call, &l);
 		if (!call)
 			break;
 	}
@@ -316,37 +306,28 @@ static void *t_start(struct seq_file *m, loff_t *pos)
 static void *
 s_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct list_head *list = m->private;
-	struct ftrace_event_call *call;
+	struct ftrace_event_call *call = v;
 
 	(*pos)++;
 
- retry:
-	if (list == &ftrace_events)
-		return NULL;
-
-	call = list_entry(list, struct ftrace_event_call, list);
-
-	if (!call->enabled) {
-		list = list->next;
-		goto retry;
+	list_for_each_entry_continue(call, &ftrace_events, list) {
+		if (call->enabled)
+			return call;
 	}
 
-	m->private = list->next;
-
-	return call;
+	return NULL;
 }
 
 static void *s_start(struct seq_file *m, loff_t *pos)
 {
-	struct ftrace_event_call *call = NULL;
+	struct ftrace_event_call *call;
 	loff_t l;
 
 	mutex_lock(&event_mutex);
 
-	m->private = ftrace_events.next;
+	call = list_entry(&ftrace_events, struct ftrace_event_call, list);
 	for (l = 0; l <= *pos; ) {
-		call = s_next(m, NULL, &l);
+		call = s_next(m, call, &l);
 		if (!call)
 			break;
 	}

Re: [PATCH 3/3] tracing/events: use list_for_entry_continue

[Li Zefan]

Frederic Weisbecker wrote:
> On Fri, Sep 18, 2009 at 02:07:05PM +0800, Li Zefan wrote:
>> Simplify s_next() and t_next().
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> ---
> 
> Nice.
> I wonder if that can be even more simplified with
> the following existing seq helpers:
> 

Unfortunately they can't be used here, at least they can't
help make code simpler, because we need to skip some list
entries.

But we can add seq_list_start_at() and seq_list_next_at(),
which take a predicate and will return an entry which matches
the predicate. I thought about it, and I can try it out.

> extern struct list_head *seq_list_start(struct list_head *head,
> 		loff_t pos);
> extern struct list_head *seq_list_start_head(struct list_head *head,
> 		loff_t pos);
> extern struct list_head *seq_list_next(void *v, struct list_head *head,
> 		loff_t *ppos);
> 

Re: [PATCH 3/3] tracing/events: use list_for_entry_continue

[Frederic Weisbecker]

On Mon, Sep 21, 2009 at 10:34:57AM +0800, Li Zefan wrote:
> Frederic Weisbecker wrote:
> > On Fri, Sep 18, 2009 at 02:07:05PM +0800, Li Zefan wrote:
> >> Simplify s_next() and t_next().
> >>
> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> >> ---
> > 
> > Nice.
> > I wonder if that can be even more simplified with
> > the following existing seq helpers:
> > 
> 
> Unfortunately they can't be used here, at least they can't
> help make code simpler, because we need to skip some list
> entries.
> 
> But we can add seq_list_start_at() and seq_list_next_at(),
> which take a predicate and will return an entry which matches
> the predicate. I thought about it, and I can try it out.


Why not, if the pattern is already met by other subsystems.
Otherwise I guess it's not needed.

Thanks.