tracing: make a snapshot feature available from userspace

Currently, read functions for trace buffer use static "old_tracer" for detecting changes of current tracer. This is because we can assume that these functions are used from only one file ("trace").

But we are adding snapshot feature for ftrace, then those functions are called from two files. So we remove all static "old_tracer", and replace those with string comparison between current and previous tracers.

Signed-off-by Hiraku Toyooka <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
 kernel/trace/trace.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

kernel/trace/trace.c +0 -1

index a8ce008..8d05a44 100644
@@ -1948,7 +1948,6 @@ void tracing_iter_reset(struct trace_iterator *iter, int cpu)
 static void *s_start(struct seq_file *m, loff_t *pos)
 {
 	struct trace_iterator *iter = m->private;
-	static struct tracer *old_tracer;
 	int cpu_file = iter->cpu_file;
 	void *p = NULL;
 	loff_t l = 0;

kernel/trace/trace.c +4 -9

-	}
 	mutex_unlock(&trace_types_lock);
 
 	atomic_inc(&trace_record_cmdline_disabled);

@@ -3481,7 +3479,6 @@ tracing_read_pipe(struct file *filp, char __user *ubuf,
 		  size_t cnt, loff_t *ppos)
 {
 	struct trace_iterator *iter = filp->private_data;
-	static struct tracer *old_tracer;
 	ssize_t sret;
 
 	/* return any leftover data */

@@ -3493,10 +3490,9 @@ tracing_read_pipe(struct file *filp, char __user *ubuf,
 
 	/* copy the tracer to avoid using a global lock all around */
 	mutex_lock(&trace_types_lock);
-	if (unlikely(old_tracer != current_trace && current_trace)) {
-		old_tracer = current_trace;
+	if (unlikely(current_trace &&
+		     strcmp(iter->trace->name, current_trace->name) != 0))
 		*iter->trace = *current_trace;
-	}
 	mutex_unlock(&trace_types_lock);
 
 	/*

@@ -3652,7 +3648,6 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
 		.ops		= &tracing_pipe_buf_ops,
 		.spd_release	= tracing_spd_release_pipe,
 	};
-	static struct tracer *old_tracer;
 	ssize_t ret;
 	size_t rem;
 	unsigned int i;

@@ -3662,10 +3657,9 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
 
 	/* copy the tracer to avoid using a global lock all around */
 	mutex_lock(&trace_types_lock);
-	if (unlikely(old_tracer != current_trace && current_trace)) {
-		old_tracer = current_trace;
+	if (unlikely(current_trace &&
+		     strcmp(iter->trace->name, current_trace->name) != 0))
 		*iter->trace = *current_trace;
-	}
 	mutex_unlock(&trace_types_lock);
 
 	mutex_lock(&iter->mutex);

Quoted 4 lines · Hiraku Toyooka

Currently, read functions for trace buffer use static "old_tracer" for detecting changes of current tracer. This is because we can assume that these functions are used from only one file ("trace").

Quoted 4 lines

But we are adding snapshot feature for ftrace, then those functions are called from two files. So we remove all static "old_tracer", and replace those with string comparison between current and previous tracers.

I reworded what you wrote:

---
Currently the trace buffer read functions use a static variable "old_tracer" for detecting if the current tracer changes. This was suitable for a single trace file ("trace"), but to add a snapshot feature that will use the same function for its file, a check against a static variable is not sufficient.

To use the output functions for two different files, instead of storing the current tracer in a static variable, as the trace iterator descriptor
contains a pointer to the original current tracer's name, that pointer can now be used to check if the current tracer has changed between different reads of the trace file.
---

And I also realized a more efficient approach.

Quoted 23 lines

Signed-off-by Hiraku Toyooka <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc [email protected]

---
  kernel/trace/trace.c |   18 ++++++------------
  1 file changed, 6 insertions(+), 12 deletions(-)

kernel/trace/trace.c +0 -1

index a8ce008..8d05a44 100644
@@ -1948,7 +1948,6 @@ void tracing_iter_reset(struct trace_iterator *iter, int cpu)
  static void *s_start(struct seq_file *m, loff_t *pos)
  {
  	struct trace_iterator *iter = m->private;
-	static struct tracer *old_tracer;
  	int cpu_file = iter->cpu_file;
  	void *p = NULL;
  	loff_t l = 0;

kernel/trace/trace.c static void *s_start(struct seq_file *m, loff_t *pos) · line 1960

Quoted 8 lines

kernel/trace/trace.c +2 -2

@@ -1956,10 +1955,9 @@ static void *s_start(struct seq_file *m, loff_t *pos)
 
  	/* copy the tracer to avoid using a global lock all around */
  	mutex_lock(&trace_types_lock);
-	if (unlikely(old_tracer != current_trace && current_trace)) {
-		old_tracer = current_trace;
+	if (unlikely(current_trace &&
+		     strcmp(iter->trace->name, current_trace->name) != 0))
  		*iter->trace = *current_trace;

As iter->trace is a copy of current_trace, it points to everything that the current_trace pointed to. As the current_trace->name is a pointer to a const char string that never changes, we don't need to do the strcmp() you can simply do a direct comparison:

	if (unlikely(current_trace && iter->trace->name != current_trace->name)) {

I would add a comment about that too:
/*
* iter->trace is a copy of current_trace, the pointer to the
* name may be used instead of a strcmp(), as iter->trace->name
* will point to the same string as current_trace->name. */