Re: [Patch -tip 1/3] Tracing/ftrace: Relay unhandled entry output

[Frédéric Weisbecker]

2008/9/25 Pekka Paalanen <pq@xxxxxx>:
> Frederic, in the future, could you just copy the patch
> into the email body (watch out for line wraps and other damage),
> attachments are not usually included in "reply with quote",
> so commenting on them is a tiny bit harder. Thanks.

Ok. I think I have to change my email client. I'm starting to get rid of all
these blank lines or other issues with the patches...
I will think about it...

> IMHO this breaks the trace_seq handling. trace_seq may contain
> the output of several entries, as far as they fit in it as a whole.
> E.g. trace_seq_printf() does not print partial lines but returns 0,
> so that the entry is not consumed right now. The user space reader
> must consume trace_seq content, before trace_seq_printf()
> is attempted again, hopefully with enough space in trace_seq to
> succeed. See tracing_read_pipe().
> print_line() callback works the same way. Returning 0 means "could
> not print it this time, call me back later". You can't use that to
> say "use the default output function instead". Note, that possibly
> the default output function will fail, too, so it could actually
> try many of the default output functions and still fail, eventually
> leading by luck to the correct behaviour in most cases.
> Note, that mmiotrace follows this convention: it deliberatly
> returns 1 without processing when it wants the entry discarded, and
> it returns 0 when there was not enough space to process the entry.
> This is explained in my other email.

Hmm you're right. I didn't thought about the partial line which must
not be printed.
The problem is that with this convention, 0 means two things: "I will
handle this entry
later" or "I can't handle it".
But if you return 0 because you can't handle it, and if the current
len of the seq is 0, the
pipe will be broken.

> What is the supposed return value of print_trace_line(), I do not
> know. Looks like it is used as boolean, so maybe the type should be
> changed to bool. Unless we want three options:
> - 0: could not print now, try again
> - 1: success, entry processed, it can be thrown away
> - 2: fall back to the default output formatting
> Cases 0 and 1 must exist like that, I do not know how useful 2 is,
> but it must be distinct from the first two.

I think it's a good solution.

Thanks to you! I didn't see these issues.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/