[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c62985530811270518n161ed600k930eefce7291ccdd@mail.gmail.com>
Date: Thu, 27 Nov 2008 14:18:41 +0100
From: "Frédéric Weisbecker" <fweisbec@...il.com>
To: "Ingo Molnar" <mingo@...e.hu>
Cc: "Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
"Steven Rostedt" <rostedt@...dmis.org>,
"Tim Bird" <tim.bird@...sony.com>,
"Linux Kernel" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing/function-branch-tracer: enhancements for the trace output
2008/11/27 Ingo Molnar <mingo@...e.hu>:
> That extra column should be a trace_option though - default-off. (it
> uses up quite some screen real estate) It would have to have a width
> of 14 characters to be practical:
>
> ssssssss-12345
>
> mockup:
>
> ----------------------------------------------------------------
> CPU) task-PID | cost | function
> ----------------------------------------------------------------
>
> 0) bash-54320 | | sys_read() {
> 0) bash-54320 | 0.331 us | fget_light();
> 0) bash-54320 | | vfs_read() {
> 0) mycomman-1234 | | rw_verify_area() {
> 0) mycomman-1234 | 121.444 us | func();
>
> Note some details:
>
> 1) longer comm names can be stripped off at 7 chars - 7 chars is
> enough to recognize most of the things and the PID is good enough
> for extra separation)
>
> 2) the ssssssss-12345 task identifier is _center_ aligned. It's a rare
> alignment case but useful here: most people look at the command
> portion and the whole string must be greppable and useful in traces
> as an identifier - so right-aligning the PID is not a good
> solution.
>
> 3) there's vertical separation after the task portion as well. This
> helps us ignore the task portion when we want to look at only the
> code and the cost of it. (the common case)
Yeah! I agree with all of that.
And as you said, that should be an option deactivated by default.
>> comm/pid only in the separator is useless, you cannot grep stuff
>> like that, also, what's up with those '8's ?
>
> yeah, that context-switch separator is horrible.
>
> We can keep the separator, but as something saner, like:
>
> ---------------------------------------------------------
> CPU) cost | function
> ---------------------------------------------------------
>
> 0) | sys_read() {
> 0) 0.331 us | fget_light();
> 0) | vfs_read() {
> 0) | rw_verify_area() {
> 0) | security_file_permission() {
> 0) 0.306 us | cap_file_permission();
> 0) 0.300 us | cap_file_permission();
> 0) 8.909 us | }
> 0) 0.993 us | }
> 0) 11.649 us | }
> 0) | do_sync_read() {
> . -------------------------
> 0) ==> [ bash-2794 => cat-1234 ]
> -------------------------
>
> 0) | __sock_recvmsg() {
> 0) | security_socket_recvmsg() {
> 0) 100.319 us | cap_socket_recvmsg();
> ---------------------------------------------------------
>
> Note the six small details:
>
> 1) '==>' sign put to the vertical separator. This shows that something
> flow-intrusive (context-switch) happened.
>
> 2) The context-switch printout is in a little box that separates it
> from the flow of the outgoing task.
>
> 3) there's an extra newline for the incoming flow. Context-switches
> are usually attached to the outgoing flow - the incoming flow is
> picked semi-randomly, so the newline is a visual separator for that
> fact.
>
> 4) it uses the ssssssss-12345 taskname convention for both the
> outgoing and the incoming task. That makes it greppable
> generally.
>
> 5) the '----------' lines of the 'box' have no CPU nor task or cost
> field. That makes it easy to grep them out - but they are still
> helpful visual cues. They are variable length but that's solvable
> by using an 'open box' simplification:
>
> 0) 0.300 us | cap_file_permission();
> 0) 8.909 us | }
> 0) 0.993 us | }
> 0) 11.649 us | }
> 0) | do_sync_read() {
> . -----------------------------------
> 0) ==> [ bash-2794 => cat-1234
> -----------------------------------
>
> Note how the box top and bottom is 35 characters, fixed. That looks
> good with most task identifiers. Even in the worst-case length
> length situation it looks good:
>
> . -----------------------------------
> 0) ==> [ mycomman-12345 => mycomman-12345
> -----------------------------------
>
> Or it could be printed out variable length as well - it's a
> solvable task :-)
>
> 6) note the small dot in the leadin to a context-switch: it's
> intentional.
>
> Ingo
>
That's fair.
I will apply these advices, probably divided in multiple patches....
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists