We need a kind of disambiguation when a print_line callback returns 0.
This value can significate both: 

_ There is not enough space to print all of the entry. Please flush the current seq and retry
_ I can't handle this type of entry.

Such a confusion can break the pipe.

This patch changes the type of this callback for a better information.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b28bf88..73cd7c5 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -180,6 +180,14 @@ struct trace_array {
 	struct trace_array_cpu	*data[NR_CPUS];
 };
 
+
+/* Return values for print_line callback */
+enum print_line_t {
+	TRACE_TYPE_PARTIAL_LINE	= 0,	/* Retry after flushing the seq */
+	TRACE_TYPE_HANDLED	= 1,
+	TRACE_TYPE_UNHANDLED	= 2	/* Relay to other output functions */
+};
+
 /*
  * A specific tracer, represented by methods that operate on a trace array:
  */
@@ -200,7 +208,7 @@ struct tracer {
 	int			(*selftest)(struct tracer *trace,
 					    struct trace_array *tr);
 #endif
-	int			(*print_line)(struct trace_iterator *iter);
+	enum print_line_t	(*print_line)(struct trace_iterator *iter);
 	struct tracer		*next;
 	int			print_max;
 };
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 50ac334..ca95ec3 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1510,7 +1510,7 @@ void trace_seq_print_cont(struct trace_seq *s, struct trace_iterator *iter)
 		trace_seq_putc(s, '\n');
 }
 
-static int
+static enum print_line_t
 print_lat_fmt(struct trace_iterator *iter, unsigned int trace_idx, int cpu)
 {
 	struct trace_seq *s = &iter->seq;
@@ -1530,7 +1530,7 @@ print_lat_fmt(struct trace_iterator *iter, unsigned int trace_idx, int cpu)
 		next_entry = entry;
 
 	if (entry->type == TRACE_CONT)
-		return 1;
+		return TRACE_TYPE_HANDLED;
 
 	rel_usecs = ns2usecs(next_entry->field.t - entry->field.t);
 	abs_usecs = ns2usecs(entry->field.t - ...
[ message continues ]

On Fri, 26 Sep 2008 17:25:21 +0200

Frederic, this looks good to me, except the very last hunk.

I might have chosen names like TRACE_PRINT_{RETRY,DONE,DEFAULT_FORMAT},
but it's your call. I'm not sure which one is more self-explanatory

Shouldn't the return type be bool?
If it's not, reading this function makes me wonder about the conversion

It would be shorter to write
if (ret != TRACE_TYPE_UNHANDLED)
and then one could even

Do these actually need checking? I don't think
the default print functions would ever return
TRACE_TYPE_UNHANDLED, could they?
And even if they did, do all the different default print


We have to find a proper way to prevent the pipe from closing
early. I'm trying to look into this. I'd like you to leave
that last hunk out. Other than that, very good.


Cheers.

btw. there might be a corner case, when a single line does not
fit even into an empty struct trace_seq in tracing_read_pipe(),
but I haven't thought of that yet. I'd expect it to hang.

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

[ message continues ]

Actually it should be enum print_line_t. I forgot to change its type.


At this moment they don't. But I just wanted to set a security in case

I made a new patch this week-end and I found a way to prevent from
closing the pipe.

I should look at this possible issue too. I didn't think about it yet.
--

[ message continues ]

On Mon, 29 Sep 2008 11:11:15 +0200


Is print_line_t necessary? Does it have to return any other information than
"this entry was handled somehow" vs. "this entry must be handled later"?
Currently it's just a flag to say "please flush and retry".

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

[ message continues ]

It would just make the code more readable in the read_pipe function.
if (sret == TRACE_TYPE_PARTIAL_LINE) tells us more than if (!sret).
And it would avoid some tests to return a boolean in print_trace_line.

No?
--

[ message continues ]