[PATCH] rcu: convert rcupreempt trace to seq file

[PATCH] rcu: convert rcupreempt trace to seq file

[Li Zefan]

Impact: also fix a bug in show_rcustats()

Use seq file for simplification, and the global buffer and mutex can be
removed.

While doing this, I found rcustats will never show 'ggp' and 'rcc' fields
due to a bug in show_rcustats().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/rcupreempt_trace.c |  155 ++++++++++++++++++++-------------------------
 1 files changed, 69 insertions(+), 86 deletions(-)

diff --git a/kernel/rcupreempt_trace.c b/kernel/rcupreempt_trace.c
index 7c2665c..a0c8297 100644
--- a/kernel/rcupreempt_trace.c
+++ b/kernel/rcupreempt_trace.c
@@ -39,13 +39,9 @@
 #include <linux/percpu.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
-#include <linux/mutex.h>
 #include <linux/rcupreempt_trace.h>
 #include <linux/debugfs.h>
-
-static struct mutex rcupreempt_trace_mutex;
-static char *rcupreempt_trace_buf;
-#define RCUPREEMPT_TRACE_BUF_SIZE 4096
+#include <linux/seq_file.h>
 
 void rcupreempt_trace_move2done(struct rcupreempt_trace *trace)
 {
@@ -170,110 +166,107 @@ static void rcupreempt_trace_sum(struct rcupreempt_trace *sp)
 	}
 }
 
-static ssize_t rcustats_read(struct file *filp, char __user *buffer,
-				size_t count, loff_t *ppos)
+static ssize_t show_rcustats(struct seq_file *m, void *unused)
 {
 	struct rcupreempt_trace trace;
-	ssize_t bcount;
-	int cnt = 0;
 
 	rcupreempt_trace_sum(&trace);
-	mutex_lock(&rcupreempt_trace_mutex);
-	snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE - cnt,
-		 "ggp=%ld rcc=%ld\n",
-		 rcu_batches_completed(),
-		 trace.rcu_check_callbacks);
-	snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE - cnt,
-		 "na=%ld nl=%ld wa=%ld wl=%ld da=%ld dl=%ld dr=%ld di=%d\n"
-		 "1=%d e1=%d i1=%ld ie1=%ld g1=%ld a1=%ld ae1=%ld a2=%ld\n"
-		 "z1=%ld ze1=%ld z2=%ld m1=%ld me1=%ld m2=%ld\n",
 
-		 trace.next_add, trace.next_length,
-		 trace.wait_add, trace.wait_length,
-		 trace.done_add, trace.done_length,
-		 trace.done_remove, atomic_read(&trace.done_invoked),
-		 atomic_read(&trace.rcu_try_flip_1),
-		 atomic_read(&trace.rcu_try_flip_e1),
-		 trace.rcu_try_flip_i1, trace.rcu_try_flip_ie1,
-		 trace.rcu_try_flip_g1,
-		 trace.rcu_try_flip_a1, trace.rcu_try_flip_ae1,
+	seq_printf(m, "ggp=%ld rcc=%ld\n",
+		   rcu_batches_completed(),
+		   trace.rcu_check_callbacks);
+
+	seq_printf(m, "na=%ld nl=%ld wa=%ld wl=%ld da=%ld dl=%ld dr=%ld di=%d\n"
+		      "1=%d e1=%d i1=%ld ie1=%ld g1=%ld a1=%ld ae1=%ld a2=%ld\n"
+		      "z1=%ld ze1=%ld z2=%ld m1=%ld me1=%ld m2=%ld\n",
+		   trace.next_add, trace.next_length,
+		   trace.wait_add, trace.wait_length,
+		   trace.done_add, trace.done_length,
+		   trace.done_remove, atomic_read(&trace.done_invoked),
+		   atomic_read(&trace.rcu_try_flip_1),
+		   atomic_read(&trace.rcu_try_flip_e1),
+		   trace.rcu_try_flip_i1, trace.rcu_try_flip_ie1,
+		   trace.rcu_try_flip_g1,
+		   trace.rcu_try_flip_a1, trace.rcu_try_flip_ae1,
 			 trace.rcu_try_flip_a2,
-		 trace.rcu_try_flip_z1, trace.rcu_try_flip_ze1,
+		   trace.rcu_try_flip_z1, trace.rcu_try_flip_ze1,
 			 trace.rcu_try_flip_z2,
-		 trace.rcu_try_flip_m1, trace.rcu_try_flip_me1,
-			trace.rcu_try_flip_m2);
-	bcount = simple_read_from_buffer(buffer, count, ppos,
-			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
-	mutex_unlock(&rcupreempt_trace_mutex);
-	return bcount;
+		   trace.rcu_try_flip_m1, trace.rcu_try_flip_me1,
+			 trace.rcu_try_flip_m2);
+
+	return 0;
+}
+
+static int rcustats_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_rcustats, NULL);
 }
 
-static ssize_t rcugp_read(struct file *filp, char __user *buffer,
-				size_t count, loff_t *ppos)
+static ssize_t show_rcugp(struct seq_file *m, void *unused)
 {
 	long oldgp = rcu_batches_completed();
-	ssize_t bcount;
 
-	mutex_lock(&rcupreempt_trace_mutex);
 	synchronize_rcu();
-	snprintf(rcupreempt_trace_buf, RCUPREEMPT_TRACE_BUF_SIZE,
-		"oldggp=%ld  newggp=%ld\n", oldgp, rcu_batches_completed());
-	bcount = simple_read_from_buffer(buffer, count, ppos,
-			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
-	mutex_unlock(&rcupreempt_trace_mutex);
-	return bcount;
+	seq_printf(m, "oldggp=%ld  newggp=%ld\n",
+		   oldgp, rcu_batches_completed());
+
+	return 0;
 }
 
-static ssize_t rcuctrs_read(struct file *filp, char __user *buffer,
-				size_t count, loff_t *ppos)
+static int rcugp_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_rcugp, NULL);
+}
+
+static ssize_t show_rcuctrs(struct seq_file *m, void *unused)
 {
-	int cnt = 0;
 	int cpu;
 	int f = rcu_batches_completed() & 0x1;
-	ssize_t bcount;
-
-	mutex_lock(&rcupreempt_trace_mutex);
 
-	cnt += snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE,
-				"CPU last cur F M\n");
+	seq_puts(m, "CPU last cur F M\n");
 	for_each_online_cpu(cpu) {
 		long *flipctr = rcupreempt_flipctr(cpu);
-		cnt += snprintf(&rcupreempt_trace_buf[cnt],
-				RCUPREEMPT_TRACE_BUF_SIZE - cnt,
-					"%3d %4ld %3ld %d %d\n",
-			       cpu,
-			       flipctr[!f],
-			       flipctr[f],
-			       rcupreempt_flip_flag(cpu),
-			       rcupreempt_mb_flag(cpu));
+		seq_printf(m, "%3d %4ld %3ld %d %d\n",
+			   cpu,
+			   flipctr[!f],
+			   flipctr[f],
+			   rcupreempt_flip_flag(cpu),
+			   rcupreempt_mb_flag(cpu));
 	}
-	cnt += snprintf(&rcupreempt_trace_buf[cnt],
-			RCUPREEMPT_TRACE_BUF_SIZE - cnt,
-			"ggp = %ld, state = %s\n",
-			rcu_batches_completed(),
-			rcupreempt_try_flip_state_name());
-	cnt += snprintf(&rcupreempt_trace_buf[cnt],
-			RCUPREEMPT_TRACE_BUF_SIZE - cnt,
-			"\n");
-	bcount = simple_read_from_buffer(buffer, count, ppos,
-			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
-	mutex_unlock(&rcupreempt_trace_mutex);
-	return bcount;
+	seq_printf(m, "ggp = %ld, state = %s\n",
+		   rcu_batches_completed(),
+		   rcupreempt_try_flip_state_name());
+
+	return 0;
+}
+
+static int rcuctrs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_rcuctrs, NULL);
 }
 
 static struct file_operations rcustats_fops = {
 	.owner = THIS_MODULE,
-	.read = rcustats_read,
+	.open = rcustats_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
 };
 
 static struct file_operations rcugp_fops = {
 	.owner = THIS_MODULE,
-	.read = rcugp_read,
+	.open = rcugp_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
 };
 
 static struct file_operations rcuctrs_fops = {
 	.owner = THIS_MODULE,
-	.read = rcuctrs_read,
+	.open = rcuctrs_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
 };
 
 static struct dentry *rcudir, *statdir, *ctrsdir, *gpdir;
@@ -308,16 +301,7 @@ out:
 
 static int __init rcupreempt_trace_init(void)
 {
-	int ret;
-
-	mutex_init(&rcupreempt_trace_mutex);
-	rcupreempt_trace_buf = kmalloc(RCUPREEMPT_TRACE_BUF_SIZE, GFP_KERNEL);
-	if (!rcupreempt_trace_buf)
-		return 1;
-	ret = rcupreempt_debugfs_init();
-	if (ret)
-		kfree(rcupreempt_trace_buf);
-	return ret;
+	return rcupreempt_debugfs_init();
 }
 
 static void __exit rcupreempt_trace_cleanup(void)
@@ -326,7 +310,6 @@ static void __exit rcupreempt_trace_cleanup(void)
 	debugfs_remove(gpdir);
 	debugfs_remove(ctrsdir);
 	debugfs_remove(rcudir);
-	kfree(rcupreempt_trace_buf);
 }
 
 
-- 
1.5.4.rc3

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Paul E. McKenney]

On Sat, Jan 10, 2009 at 03:48:46PM +0800, Li Zefan wrote:
> Impact: also fix a bug in show_rcustats()
> 
> Use seq file for simplification, and the global buffer and mutex can be
> removed.

Looks very good, and passes my tests.

> While doing this, I found rcustats will never show 'ggp' and 'rcc' fields
> due to a bug in show_rcustats().

Good eyes!

The only change I ask is that you pull the contents of
rcupreempt_debugfs_init() into the now-one-line rcupreempt_trace_init().

With that change:

Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  kernel/rcupreempt_trace.c |  155 ++++++++++++++++++++-------------------------
>  1 files changed, 69 insertions(+), 86 deletions(-)
> 
> diff --git a/kernel/rcupreempt_trace.c b/kernel/rcupreempt_trace.c
> index 7c2665c..a0c8297 100644
> --- a/kernel/rcupreempt_trace.c
> +++ b/kernel/rcupreempt_trace.c
> @@ -39,13 +39,9 @@
>  #include <linux/percpu.h>
>  #include <linux/notifier.h>
>  #include <linux/cpu.h>
> -#include <linux/mutex.h>
>  #include <linux/rcupreempt_trace.h>
>  #include <linux/debugfs.h>
> -
> -static struct mutex rcupreempt_trace_mutex;
> -static char *rcupreempt_trace_buf;
> -#define RCUPREEMPT_TRACE_BUF_SIZE 4096
> +#include <linux/seq_file.h>
> 
>  void rcupreempt_trace_move2done(struct rcupreempt_trace *trace)
>  {
> @@ -170,110 +166,107 @@ static void rcupreempt_trace_sum(struct rcupreempt_trace *sp)
>  	}
>  }
> 
> -static ssize_t rcustats_read(struct file *filp, char __user *buffer,
> -				size_t count, loff_t *ppos)
> +static ssize_t show_rcustats(struct seq_file *m, void *unused)
>  {
>  	struct rcupreempt_trace trace;
> -	ssize_t bcount;
> -	int cnt = 0;
> 
>  	rcupreempt_trace_sum(&trace);
> -	mutex_lock(&rcupreempt_trace_mutex);
> -	snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE - cnt,
> -		 "ggp=%ld rcc=%ld\n",
> -		 rcu_batches_completed(),
> -		 trace.rcu_check_callbacks);
> -	snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE - cnt,
> -		 "na=%ld nl=%ld wa=%ld wl=%ld da=%ld dl=%ld dr=%ld di=%d\n"
> -		 "1=%d e1=%d i1=%ld ie1=%ld g1=%ld a1=%ld ae1=%ld a2=%ld\n"
> -		 "z1=%ld ze1=%ld z2=%ld m1=%ld me1=%ld m2=%ld\n",
> 
> -		 trace.next_add, trace.next_length,
> -		 trace.wait_add, trace.wait_length,
> -		 trace.done_add, trace.done_length,
> -		 trace.done_remove, atomic_read(&trace.done_invoked),
> -		 atomic_read(&trace.rcu_try_flip_1),
> -		 atomic_read(&trace.rcu_try_flip_e1),
> -		 trace.rcu_try_flip_i1, trace.rcu_try_flip_ie1,
> -		 trace.rcu_try_flip_g1,
> -		 trace.rcu_try_flip_a1, trace.rcu_try_flip_ae1,
> +	seq_printf(m, "ggp=%ld rcc=%ld\n",
> +		   rcu_batches_completed(),
> +		   trace.rcu_check_callbacks);
> +
> +	seq_printf(m, "na=%ld nl=%ld wa=%ld wl=%ld da=%ld dl=%ld dr=%ld di=%d\n"
> +		      "1=%d e1=%d i1=%ld ie1=%ld g1=%ld a1=%ld ae1=%ld a2=%ld\n"
> +		      "z1=%ld ze1=%ld z2=%ld m1=%ld me1=%ld m2=%ld\n",
> +		   trace.next_add, trace.next_length,
> +		   trace.wait_add, trace.wait_length,
> +		   trace.done_add, trace.done_length,
> +		   trace.done_remove, atomic_read(&trace.done_invoked),
> +		   atomic_read(&trace.rcu_try_flip_1),
> +		   atomic_read(&trace.rcu_try_flip_e1),
> +		   trace.rcu_try_flip_i1, trace.rcu_try_flip_ie1,
> +		   trace.rcu_try_flip_g1,
> +		   trace.rcu_try_flip_a1, trace.rcu_try_flip_ae1,
>  			 trace.rcu_try_flip_a2,
> -		 trace.rcu_try_flip_z1, trace.rcu_try_flip_ze1,
> +		   trace.rcu_try_flip_z1, trace.rcu_try_flip_ze1,
>  			 trace.rcu_try_flip_z2,
> -		 trace.rcu_try_flip_m1, trace.rcu_try_flip_me1,
> -			trace.rcu_try_flip_m2);
> -	bcount = simple_read_from_buffer(buffer, count, ppos,
> -			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
> -	mutex_unlock(&rcupreempt_trace_mutex);
> -	return bcount;
> +		   trace.rcu_try_flip_m1, trace.rcu_try_flip_me1,
> +			 trace.rcu_try_flip_m2);
> +
> +	return 0;
> +}
> +
> +static int rcustats_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, show_rcustats, NULL);
>  }
> 
> -static ssize_t rcugp_read(struct file *filp, char __user *buffer,
> -				size_t count, loff_t *ppos)
> +static ssize_t show_rcugp(struct seq_file *m, void *unused)
>  {
>  	long oldgp = rcu_batches_completed();
> -	ssize_t bcount;
> 
> -	mutex_lock(&rcupreempt_trace_mutex);
>  	synchronize_rcu();
> -	snprintf(rcupreempt_trace_buf, RCUPREEMPT_TRACE_BUF_SIZE,
> -		"oldggp=%ld  newggp=%ld\n", oldgp, rcu_batches_completed());
> -	bcount = simple_read_from_buffer(buffer, count, ppos,
> -			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
> -	mutex_unlock(&rcupreempt_trace_mutex);
> -	return bcount;
> +	seq_printf(m, "oldggp=%ld  newggp=%ld\n",
> +		   oldgp, rcu_batches_completed());
> +
> +	return 0;
>  }
> 
> -static ssize_t rcuctrs_read(struct file *filp, char __user *buffer,
> -				size_t count, loff_t *ppos)
> +static int rcugp_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, show_rcugp, NULL);
> +}
> +
> +static ssize_t show_rcuctrs(struct seq_file *m, void *unused)
>  {
> -	int cnt = 0;
>  	int cpu;
>  	int f = rcu_batches_completed() & 0x1;
> -	ssize_t bcount;
> -
> -	mutex_lock(&rcupreempt_trace_mutex);
> 
> -	cnt += snprintf(&rcupreempt_trace_buf[cnt], RCUPREEMPT_TRACE_BUF_SIZE,
> -				"CPU last cur F M\n");
> +	seq_puts(m, "CPU last cur F M\n");
>  	for_each_online_cpu(cpu) {
>  		long *flipctr = rcupreempt_flipctr(cpu);
> -		cnt += snprintf(&rcupreempt_trace_buf[cnt],
> -				RCUPREEMPT_TRACE_BUF_SIZE - cnt,
> -					"%3d %4ld %3ld %d %d\n",
> -			       cpu,
> -			       flipctr[!f],
> -			       flipctr[f],
> -			       rcupreempt_flip_flag(cpu),
> -			       rcupreempt_mb_flag(cpu));
> +		seq_printf(m, "%3d %4ld %3ld %d %d\n",
> +			   cpu,
> +			   flipctr[!f],
> +			   flipctr[f],
> +			   rcupreempt_flip_flag(cpu),
> +			   rcupreempt_mb_flag(cpu));
>  	}
> -	cnt += snprintf(&rcupreempt_trace_buf[cnt],
> -			RCUPREEMPT_TRACE_BUF_SIZE - cnt,
> -			"ggp = %ld, state = %s\n",
> -			rcu_batches_completed(),
> -			rcupreempt_try_flip_state_name());
> -	cnt += snprintf(&rcupreempt_trace_buf[cnt],
> -			RCUPREEMPT_TRACE_BUF_SIZE - cnt,
> -			"\n");
> -	bcount = simple_read_from_buffer(buffer, count, ppos,
> -			rcupreempt_trace_buf, strlen(rcupreempt_trace_buf));
> -	mutex_unlock(&rcupreempt_trace_mutex);
> -	return bcount;
> +	seq_printf(m, "ggp = %ld, state = %s\n",
> +		   rcu_batches_completed(),
> +		   rcupreempt_try_flip_state_name());
> +
> +	return 0;
> +}
> +
> +static int rcuctrs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, show_rcuctrs, NULL);
>  }
> 
>  static struct file_operations rcustats_fops = {
>  	.owner = THIS_MODULE,
> -	.read = rcustats_read,
> +	.open = rcustats_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
>  };
> 
>  static struct file_operations rcugp_fops = {
>  	.owner = THIS_MODULE,
> -	.read = rcugp_read,
> +	.open = rcugp_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
>  };
> 
>  static struct file_operations rcuctrs_fops = {
>  	.owner = THIS_MODULE,
> -	.read = rcuctrs_read,
> +	.open = rcuctrs_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
>  };
> 
>  static struct dentry *rcudir, *statdir, *ctrsdir, *gpdir;
> @@ -308,16 +301,7 @@ out:
> 
>  static int __init rcupreempt_trace_init(void)
>  {
> -	int ret;
> -
> -	mutex_init(&rcupreempt_trace_mutex);
> -	rcupreempt_trace_buf = kmalloc(RCUPREEMPT_TRACE_BUF_SIZE, GFP_KERNEL);
> -	if (!rcupreempt_trace_buf)
> -		return 1;
> -	ret = rcupreempt_debugfs_init();
> -	if (ret)
> -		kfree(rcupreempt_trace_buf);
> -	return ret;
> +	return rcupreempt_debugfs_init();
>  }
> 
>  static void __exit rcupreempt_trace_cleanup(void)
> @@ -326,7 +310,6 @@ static void __exit rcupreempt_trace_cleanup(void)
>  	debugfs_remove(gpdir);
>  	debugfs_remove(ctrsdir);
>  	debugfs_remove(rcudir);
> -	kfree(rcupreempt_trace_buf);
>  }
> 
> 
> -- 
> 1.5.4.rc3
> 

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> On Sat, Jan 10, 2009 at 03:48:46PM +0800, Li Zefan wrote:
> > Impact: also fix a bug in show_rcustats()
> > 
> > Use seq file for simplification, and the global buffer and mutex can be
> > removed.
> 
> Looks very good, and passes my tests.
> 
> > While doing this, I found rcustats will never show 'ggp' and 'rcc' fields
> > due to a bug in show_rcustats().
> 
> Good eyes!
> 
> The only change I ask is that you pull the contents of
> rcupreempt_debugfs_init() into the now-one-line rcupreempt_trace_init().
> 
> With that change:
> 
> Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> > ---
> >  kernel/rcupreempt_trace.c |  155 ++++++++++++++++++++-------------------------

i think the whole kernel/rcupreempt_trace.c file might be a good candidate 
for the 'statistical tracing' extensions to ftrace that Frederice is 
working on.

That way there would just be a few tracepoints, and the rest would be done 
in kernel/tracing/trace_rcupreempt.c, or so.
	Ingo

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Frederic Weisbecker]

On Sun, Jan 11, 2009 at 02:39:40AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Sat, Jan 10, 2009 at 03:48:46PM +0800, Li Zefan wrote:
> > > Impact: also fix a bug in show_rcustats()
> > > 
> > > Use seq file for simplification, and the global buffer and mutex can be
> > > removed.
> > 
> > Looks very good, and passes my tests.
> > 
> > > While doing this, I found rcustats will never show 'ggp' and 'rcc' fields
> > > due to a bug in show_rcustats().
> > 
> > Good eyes!
> > 
> > The only change I ask is that you pull the contents of
> > rcupreempt_debugfs_init() into the now-one-line rcupreempt_trace_init().
> > 
> > With that change:
> > 
> > Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > > ---
> > >  kernel/rcupreempt_trace.c |  155 ++++++++++++++++++++-------------------------
> 
> i think the whole kernel/rcupreempt_trace.c file might be a good candidate 
> for the 'statistical tracing' extensions to ftrace that Frederice is 
> working on.
> 
> That way there would just be a few tracepoints, and the rest would be done 
> in kernel/tracing/trace_rcupreempt.c, or so.
> 	Ingo

Indeed!

The debugfs and seqfile handling and handled in the statistical
tracing engine.

You just have to provide an iterator for your stat entries through two callbacks:

_ stat_start() -> gives the first entry
_ stat_next() -> iterates over the next entry

And an output callback

_ stat_show() -> print one entry from your stat list

And two optional things:

_ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
_ stat_headers() -> provide the first line in your stat file, typically to describe your columns

The last thing you need is to give a name to your trace file.
You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
of your stats.

It is currently used by the branch tracer, and by a pending patch for a new workqueue
tracer which will provide you a simple example.

If you have any question about how to use it, don't hesitate to ask.

Frederic.

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Paul E. McKenney]

On Sun, Jan 11, 2009 at 06:01:31AM +0100, Frederic Weisbecker wrote:
> On Sun, Jan 11, 2009 at 02:39:40AM +0100, Ingo Molnar wrote:
> > 
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > On Sat, Jan 10, 2009 at 03:48:46PM +0800, Li Zefan wrote:
> > > > Impact: also fix a bug in show_rcustats()
> > > > 
> > > > Use seq file for simplification, and the global buffer and mutex can be
> > > > removed.
> > > 
> > > Looks very good, and passes my tests.
> > > 
> > > > While doing this, I found rcustats will never show 'ggp' and 'rcc' fields
> > > > due to a bug in show_rcustats().
> > > 
> > > Good eyes!
> > > 
> > > The only change I ask is that you pull the contents of
> > > rcupreempt_debugfs_init() into the now-one-line rcupreempt_trace_init().
> > > 
> > > With that change:
> > > 
> > > Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > >
> > > > ---
> > > >  kernel/rcupreempt_trace.c |  155 ++++++++++++++++++++-------------------------
> > 
> > i think the whole kernel/rcupreempt_trace.c file might be a good candidate 
> > for the 'statistical tracing' extensions to ftrace that Frederice is 
> > working on.
> > 
> > That way there would just be a few tracepoints, and the rest would be done 
> > in kernel/tracing/trace_rcupreempt.c, or so.
> > 	Ingo
> 
> Indeed!
> 
> The debugfs and seqfile handling and handled in the statistical
> tracing engine.
> 
> You just have to provide an iterator for your stat entries through two callbacks:
> 
> _ stat_start() -> gives the first entry
> _ stat_next() -> iterates over the next entry
> 
> And an output callback
> 
> _ stat_show() -> print one entry from your stat list
> 
> And two optional things:
> 
> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
> 
> The last thing you need is to give a name to your trace file.
> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
> of your stats.
> 
> It is currently used by the branch tracer, and by a pending patch for a new workqueue
> tracer which will provide you a simple example.
> 
> If you have any question about how to use it, don't hesitate to ask.

Li Zefan, is this something you would be willing to try?  Sounds like
it might be a good addition.

							Thanx, Paul

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Li Zefan]

>> The debugfs and seqfile handling and handled in the statistical
>> tracing engine.
>>
>> You just have to provide an iterator for your stat entries through two callbacks:
>>
>> _ stat_start() -> gives the first entry
>> _ stat_next() -> iterates over the next entry
>>
>> And an output callback
>>
>> _ stat_show() -> print one entry from your stat list
>>
>> And two optional things:
>>
>> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
>> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
>>
>> The last thing you need is to give a name to your trace file.
>> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
>> of your stats.
>>
>> It is currently used by the branch tracer, and by a pending patch for a new workqueue
>> tracer which will provide you a simple example.
>>
>> If you have any question about how to use it, don't hesitate to ask.
> 
> Li Zefan, is this something you would be willing to try?  Sounds like
> it might be a good addition.
> 

Sure, I'll try. :)

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Paul E. McKenney]

On Mon, Jan 12, 2009 at 09:09:02AM +0800, Li Zefan wrote:
> >> The debugfs and seqfile handling and handled in the statistical
> >> tracing engine.
> >>
> >> You just have to provide an iterator for your stat entries through two callbacks:
> >>
> >> _ stat_start() -> gives the first entry
> >> _ stat_next() -> iterates over the next entry
> >>
> >> And an output callback
> >>
> >> _ stat_show() -> print one entry from your stat list
> >>
> >> And two optional things:
> >>
> >> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
> >> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
> >>
> >> The last thing you need is to give a name to your trace file.
> >> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
> >> of your stats.
> >>
> >> It is currently used by the branch tracer, and by a pending patch for a new workqueue
> >> tracer which will provide you a simple example.
> >>
> >> If you have any question about how to use it, don't hesitate to ask.
> > 
> > Li Zefan, is this something you would be willing to try?  Sounds like
> > it might be a good addition.
> 
> Sure, I'll try. :)

Very cool!  I look forward to seeing it!!!

							Thanx, Paul

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Li Zefan]

> Indeed!
> 
> The debugfs and seqfile handling and handled in the statistical
> tracing engine.
> 
> You just have to provide an iterator for your stat entries through two callbacks:
> 
> _ stat_start() -> gives the first entry
> _ stat_next() -> iterates over the next entry
> 
> And an output callback
> 
> _ stat_show() -> print one entry from your stat list
> 
> And two optional things:
> 
> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
> 
> The last thing you need is to give a name to your trace file.
> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
> of your stats.
> 
> It is currently used by the branch tracer, and by a pending patch for a new workqueue
> tracer which will provide you a simple example.
> 
> If you have any question about how to use it, don't hesitate to ask.
> 

Hi Frederic,

I've converted rcupreempt to use trace points, but I don't see much advantage to use
trace stat instead of using seq_file directly.. And can you add support to allow
me to provide stat_show() only ? I think it's common that the stat file has only
one entry.


Regards
Li Zefan

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Frederic Weisbecker]

On Tue, Jan 13, 2009 at 11:19:44AM +0800, Li Zefan wrote:
> > Indeed!
> > 
> > The debugfs and seqfile handling and handled in the statistical
> > tracing engine.
> > 
> > You just have to provide an iterator for your stat entries through two callbacks:
> > 
> > _ stat_start() -> gives the first entry
> > _ stat_next() -> iterates over the next entry
> > 
> > And an output callback
> > 
> > _ stat_show() -> print one entry from your stat list
> > 
> > And two optional things:
> > 
> > _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
> > _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
> > 
> > The last thing you need is to give a name to your trace file.
> > You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
> > of your stats.
> > 
> > It is currently used by the branch tracer, and by a pending patch for a new workqueue
> > tracer which will provide you a simple example.
> > 
> > If you have any question about how to use it, don't hesitate to ask.
> > 
> 
> Hi Frederic,
> 
> I've converted rcupreempt to use trace points, but I don't see much advantage to use
> trace stat instead of using seq_file directly.. And can you add support to allow
> me to provide stat_show() only ? I think it's common that the stat file has only
> one entry.
> 
> 
> Regards
> Li Zefan


It simplifies (I hope) a bit the seqfile.
The interface is very similar except that you don't need to deal
with debugfs stuffs, sorting, and position inside the seqfile.
And it unifies the stat files into a common directory instead of
having them grained into a mess of debugfs filesystem....

I could let it handle only stat_show when you have only one entry but that
would break the sense of stat_show.
If it's so common to have only one stat entry, perhaps I could provide a special callback
for that, something like stat_show_unique()...

Hm?

Frederic.

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Li Zefan]

Frederic Weisbecker wrote:
> On Tue, Jan 13, 2009 at 11:19:44AM +0800, Li Zefan wrote:
>>> Indeed!
>>>
>>> The debugfs and seqfile handling and handled in the statistical
>>> tracing engine.
>>>
>>> You just have to provide an iterator for your stat entries through two callbacks:
>>>
>>> _ stat_start() -> gives the first entry
>>> _ stat_next() -> iterates over the next entry
>>>
>>> And an output callback
>>>
>>> _ stat_show() -> print one entry from your stat list
>>>
>>> And two optional things:
>>>
>>> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
>>> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
>>>
>>> The last thing you need is to give a name to your trace file.
>>> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
>>> of your stats.
>>>
>>> It is currently used by the branch tracer, and by a pending patch for a new workqueue
>>> tracer which will provide you a simple example.
>>>
>>> If you have any question about how to use it, don't hesitate to ask.
>>>
>> Hi Frederic,
>>
>> I've converted rcupreempt to use trace points, but I don't see much advantage to use
>> trace stat instead of using seq_file directly.. And can you add support to allow
>> me to provide stat_show() only ? I think it's common that the stat file has only
>> one entry.
>>
>>
>> Regards
>> Li Zefan
> 
> 
> It simplifies (I hope) a bit the seqfile.
> The interface is very similar except that you don't need to deal
> with debugfs stuffs, sorting, and position inside the seqfile.
> And it unifies the stat files into a common directory instead of
> having them grained into a mess of debugfs filesystem....
> 

OK. It makes things easier for some cases. :)

But will trace_stat/ become a mess when there are many stat files in it ?

Does it make sense to support making subdir in trace_stat/ ?

> I could let it handle only stat_show when you have only one entry but that
> would break the sense of stat_show.
> If it's so common to have only one stat entry, perhaps I could provide a special callback
> for that, something like stat_show_unique()...
> 
> Hm?
> 

stat_show_single() ? I don't know which name is better.

Like it's common to use single_open with seq_file, I think it's needed as we add more
stat files into trace_stat.

Regards
Li Zefan

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Frederic Weisbecker]

On Tue, Jan 13, 2009 at 05:03:05PM +0800, Li Zefan wrote:
> Frederic Weisbecker wrote:
> > On Tue, Jan 13, 2009 at 11:19:44AM +0800, Li Zefan wrote:
> >>> Indeed!
> >>>
> >>> The debugfs and seqfile handling and handled in the statistical
> >>> tracing engine.
> >>>
> >>> You just have to provide an iterator for your stat entries through two callbacks:
> >>>
> >>> _ stat_start() -> gives the first entry
> >>> _ stat_next() -> iterates over the next entry
> >>>
> >>> And an output callback
> >>>
> >>> _ stat_show() -> print one entry from your stat list
> >>>
> >>> And two optional things:
> >>>
> >>> _ stat_cmp() -> compare two entries, useful if you want your stats to be sorted
> >>> _ stat_headers() -> provide the first line in your stat file, typically to describe your columns
> >>>
> >>> The last thing you need is to give a name to your trace file.
> >>> You will retrieve it into /debugfs/tracing/trace_stat/your_file_name as a current snapshot
> >>> of your stats.
> >>>
> >>> It is currently used by the branch tracer, and by a pending patch for a new workqueue
> >>> tracer which will provide you a simple example.
> >>>
> >>> If you have any question about how to use it, don't hesitate to ask.
> >>>
> >> Hi Frederic,
> >>
> >> I've converted rcupreempt to use trace points, but I don't see much advantage to use
> >> trace stat instead of using seq_file directly.. And can you add support to allow
> >> me to provide stat_show() only ? I think it's common that the stat file has only
> >> one entry.
> >>
> >>
> >> Regards
> >> Li Zefan
> > 
> > 
> > It simplifies (I hope) a bit the seqfile.
> > The interface is very similar except that you don't need to deal
> > with debugfs stuffs, sorting, and position inside the seqfile.
> > And it unifies the stat files into a common directory instead of
> > having them grained into a mess of debugfs filesystem....
> > 
> 
> OK. It makes things easier for some cases. :)
> 
> But will trace_stat/ become a mess when there are many stat files in it ?
> 
> Does it make sense to support making subdir in trace_stat/ ?


Yes it's on my projects :-)


> > I could let it handle only stat_show when you have only one entry but that
> > would break the sense of stat_show.
> > If it's so common to have only one stat entry, perhaps I could provide a special callback
> > for that, something like stat_show_unique()...
> > 
> > Hm?
> > 
> 
> stat_show_single() ? I don't know which name is better.
> 
> Like it's common to use single_open with seq_file, I think it's needed as we add more
> stat files into trace_stat.
> 
> Regards
> Li Zefan
> 

stat_show_single() is a better name.
Yes that makes sense, I will add this callback.

Thanks.

Re: [PATCH] rcu: convert rcupreempt trace to seq file

[Steven Rostedt]

On Tue, 13 Jan 2009, Li Zefan wrote:
> Frederic Weisbecker wrote:

> But will trace_stat/ become a mess when there are many stat files in it ?
> 
> Does it make sense to support making subdir in trace_stat/ ?
> 

Perhaps we need to allow for a hierarchy. That is, when registering a 
trace_stat struct, allow it to have a parent. If it does, then make a 
subdirectory from the parent. But then we need a way to distinguish 
between parents (directories) and files.

a trace_stat_mkdir ?

-- Steve