Re: [PATCH V2 -tip 3/4] Tracing/ftrace: Adapt mmiotrace to the new type of print_line

Previous thread: [PATCH -tip 2/4] Tracing/ftrace: Fix pipe breaking by Frederic Weisbecker on Monday, September 29, 2008 - 11:23 am. (1 message)

Next thread: [PATCH] ACPI: Reduce blacklist-by-date failures to KERN_INFO by Adam Jackson on Monday, September 29, 2008 - 11:29 am. (1 message)
From: Frederic Weisbecker
Date: Monday, September 29, 2008 - 11:27 am

Adapt mmiotrace to the new print_line type.
By default, it ignores (and consumes) types it doesn't support.

Acked-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace_mmiotrace.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index a108c32..be0a6b0 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -171,7 +171,7 @@ print_out:
 	return (ret == -EBUSY) ? 0 : ret;
 }
 
-static int mmio_print_rw(struct trace_iterator *iter)
+static enum print_line_t mmio_print_rw(struct trace_iterator *iter)
 {
 	struct trace_entry *entry = iter->ent;
 	struct mmiotrace_rw *rw	= &entry->field.mmiorw;
@@ -209,11 +209,11 @@ static int mmio_print_rw(struct trace_iterator *iter)
 		break;
 	}
 	if (ret)
-		return 1;
-	return 0;
+		return TRACE_TYPE_HANDLED;
+	return TRACE_TYPE_PARTIAL_LINE;
 }
 
-static int mmio_print_map(struct trace_iterator *iter)
+static enum print_line_t mmio_print_map(struct trace_iterator *iter)
 {
 	struct trace_entry *entry = iter->ent;
 	struct mmiotrace_map *m	= &entry->field.mmiomap;
@@ -221,7 +221,7 @@ static int mmio_print_map(struct trace_iterator *iter)
 	unsigned long long t	= ns2usecs(entry->field.t);
 	unsigned long usec_rem	= do_div(t, 1000000ULL);
 	unsigned secs		= (unsigned long)t;
-	int ret = 1;
+	int ret;
 
 	switch (entry->field.mmiorw.opcode) {
 	case MMIO_PROBE:
@@ -241,11 +241,11 @@ static int mmio_print_map(struct trace_iterator *iter)
 		break;
 	}
 	if (ret)
-		return 1;
-	return 0;
+		return TRACE_TYPE_HANDLED;
+	return TRACE_TYPE_PARTIAL_LINE;
 }
 
-static int mmio_print_mark(struct trace_iterator *iter)
+static enum print_line_t mmio_print_mark(struct trace_iterator *iter)
 {
 	struct trace_entry *entry = iter->ent;
 	const char *msg		= entry->field.print.buf;
@@ -258,16 +258,15 @@ static int mmio_print_mark(struct ...
From: Pekka Paalanen
Date: Monday, September 29, 2008 - 12:19 pm

On Mon, 29 Sep 2008 20:27:42 +0200

Ack!
All four patches looking good.




-- 
Pekka Paalanen
http://www.iki.fi/pq/
--

From: Ingo Molnar
Date: Tuesday, September 30, 2008 - 1:52 am

thanks guys!

Steve's latest trace-ringbuffer code against -tip complicates life a 
little bit.

We dont yet know whether tip/tracing/ring-buffer is ready for wider 
consumption, so it's in a separate feature branch. These four patches 
conflict with the ring-buffer code quite widely.

So ... i did an optimistic merge approach: i merged these 4 patches 
ontop of the new tip/tracing/ring-buffer code, and have put it into a 
new tip/tracing/pipe branch:

 # f19d495: tracing/ftrace: adapt the boot tracer to the new print_line type
 # 92261f6: tracing/ftrace: adapt mmiotrace to the new type of print_line
 # 157190e: tracing/ftrace: fix pipe breaking
 # 92e359a: tracing/ftrace: change the type of the print_line callback

i'm merging this into tip/master for a bit of testing, but if it breaks 
something (or if i havent pushed it out yet) you can merge it locally 
too, by doing this ontop of tip/master:

  git-merge tip/tracing/pipe

it should merge up cleanly and you should be able to check the end 
result. (and send me any fixes for merge mistakes)

( Once it's all in reasonably well-tested shape it will all show up in 
  tip/master and you can stop dealing with tip/tracing/* sub-branches.)

	Ingo
--

From: Frédéric Weisbecker
Date: Tuesday, September 30, 2008 - 5:39 am

Thanks Ingo.

I'm currently testing tip/master, since tracing/pipe is merged into.
It seems there are some locking issues that I hadn't when I tested the patches.
Perhaps it's because of the merge with the new ring buffer.

I just launched the sched_switch tracer and it worked well until 4
seconds of execution. And after that it blocked.
I tested too a marker message adding and it is never displayed. Worse:
I can't break the pipe with Ctrl + C after that.

When I will have some time, I will try to debug all what I find.
--

From: Steven Rostedt
Date: Tuesday, September 30, 2008 - 5:45 am

OK, I'll nuke the ring_buffer_lock :-/

The trace_pipe calls that and then calls print_trace_line which will 
eventually lock the buffer again. This is a bug on my part. I'll fix that 
today.

Thanks,

-- Steve

--

From: Frédéric Weisbecker
Date: Tuesday, September 30, 2008 - 6:25 am

Strange, I can't see any case where print_trace_line could call the
ring_buffer_lock.
Hmm, I will see in your patch.

Ingo, I just saw one damage from the merging, trace_empty() returns
TRACE_TYPE_HANDLED. The type is wrong, it should return 1.
It's not urgent since the value is the same. Should I send a patch for
such a small error?

Thanks.
--

From: Steven Rostedt
Date: Tuesday, September 30, 2008 - 6:33 am

Opps, I mean the find_next_entry_inc will do the lock.

The ring_buffer_lock locks all CPU buffers, and the find_next_entry_inc
will read from the ring buffer which will also lock the buffer. Hence
a deadlock.

--

From: Ingo Molnar
Date: Tuesday, September 30, 2008 - 8:39 am

yes, please do. One too many patch is far better than one too few ;-)

	Ingo
--

From: Frederic Weisbecker
Date: Tuesday, September 30, 2008 - 9:13 am

Here it is!

Subject: [PATCH -tip] Tracing/ftrace: correct return value of trace_empty

Correct the value's type of trace_empty function

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6a1c76b..da3789d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1686,7 +1686,7 @@ static int trace_empty(struct trace_iterator *iter)
 		if (!ring_buffer_iter_empty(iter->buffer_iter[cpu]))
 			return 0;
 	}
-	return TRACE_TYPE_HANDLED;
+	return 1;
 }
 
 static enum print_line_t print_trace_line(struct trace_iterator *iter)


diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6a1c76b..da3789d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1686,7 +1686,7 @@ static int trace_empty(struct trace_iterator *iter)
 		if (!ring_buffer_iter_empty(iter->buffer_iter[cpu]))
 			return 0;
 	}
-	return TRACE_TYPE_HANDLED;
+	return 1;
 }
 
 static enum print_line_t print_trace_line(struct trace_iterator *iter)
--

From: Frédéric Weisbecker
Date: Tuesday, September 30, 2008 - 9:16 am

Oops sorry...
Cut in the middle....

--

From: Ingo Molnar
Date: Wednesday, October 1, 2008 - 1:06 am

applied the commit below to tip/tracing/ring-buffer, thanks Frederic! 
(had to do manual merging due to flux in that function due to the 
locking changes - please double-check that i got it right.)


	Ingo

------------>
From a2e221682b91ff83dc8a5e7fbb60a9d87a4e83f2 Mon Sep 17 00:00:00 2001
From: Frederic Weisbecker <fweisbec@gmail.com>
Date: Tue, 30 Sep 2008 18:13:45 +0200
Subject: [PATCH] tracing/ftrace: Adapt mmiotrace to the new type of print_line, fix

Correct the value's type of trace_empty function

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/trace/trace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b542f88..c163406 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1750,7 +1750,7 @@ static int trace_empty(struct trace_iterator *iter)
 		}
 	}
 
-	return TRACE_TYPE_HANDLED;
+	return 1;
 }
 
 static enum print_line_t print_trace_line(struct trace_iterator *iter)
--

Previous thread: [PATCH -tip 2/4] Tracing/ftrace: Fix pipe breaking by Frederic Weisbecker on Monday, September 29, 2008 - 11:23 am. (1 message)

Next thread: [PATCH] ACPI: Reduce blacklist-by-date failures to KERN_INFO by Adam Jackson on Monday, September 29, 2008 - 11:29 am. (1 message)