linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL][PATCH] tracing: Reorder display of TGID to be after PID
@ 2018-07-13 12:41 Steven Rostedt
  0 siblings, 0 replies; only message in thread
From: Steven Rostedt @ 2018-07-13 12:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, Andrew Morton, Joel Fernandes (Google), jreck, tkjos


Linus,

Joel Fernandes asked to add a feature in tracing that Android had its
own patch internally for. I took it back in 4.13. Now he realizes that
he had a mistake, and swapped the values from what Android had. This
means that the old Android tools will break when using a new kernel
that has the new feature on it.

The options are:

 1. To swap it back to what Android wants.
 2. Add a command line option or something to do the swap
 3. Just let Android carry a patch that swaps it back

Since it requires setting a tracing option to enable this anyway,
I doubt there are other users of this than Android. Thus, I've
decided to take option 1. If someone else is actually depending on the
order that is in the kernel, then we will have to revert this change
and go to option 2 or 3.


Please pull the latest trace-v4.18-rc3-3 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.18-rc3-3

Tag SHA1: 98d877a3a308950afaaf28a0b55699726b45ad7a
Head SHA1: f8494fa3dd10b52eab47a9666a8bc34719a129aa


Joel Fernandes (Google) (1):
      tracing: Reorder display of TGID to be after PID

----
 kernel/trace/trace.c        | 8 ++++----
 kernel/trace/trace_output.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)
---------------------------
commit f8494fa3dd10b52eab47a9666a8bc34719a129aa
Author: Joel Fernandes (Google) <joel@joelfernandes.org>
Date:   Mon Jun 25 17:08:22 2018 -0700

    tracing: Reorder display of TGID to be after PID
    
    Currently ftrace displays data in trace output like so:
    
                                           _-----=> irqs-off
                                          / _----=> need-resched
                                         | / _---=> hardirq/softirq
                                         || / _--=> preempt-depth
                                         ||| /     delay
                TASK-PID   CPU    TGID   ||||    TIMESTAMP  FUNCTION
                   | |       |      |    ||||       |         |
                bash-1091  [000] ( 1091) d..2    28.313544: sched_switch:
    
    However Android's trace visualization tools expect a slightly different
    format due to an out-of-tree patch patch that was been carried for a
    decade, notice that the TGID and CPU fields are reversed:
    
                                           _-----=> irqs-off
                                          / _----=> need-resched
                                         | / _---=> hardirq/softirq
                                         || / _--=> preempt-depth
                                         ||| /     delay
                TASK-PID    TGID   CPU   ||||    TIMESTAMP  FUNCTION
                   | |        |      |   ||||       |         |
                bash-1091  ( 1091) [002] d..2    64.965177: sched_switch:
    
    From kernel v4.13 onwards, during which TGID was introduced, tracing
    with systrace on all Android kernels will break (most Android kernels
    have been on 4.9 with Android patches, so this issues hasn't been seen
    yet). From v4.13 onwards things will break.
    
    The chrome browser's tracing tools also embed the systrace viewer which
    uses the legacy TGID format and updates to that are known to be
    difficult to make.
    
    Considering this, I suggest we make this change to the upstream kernel
    and backport it to all Android kernels. I believe this feature is merged
    recently enough into the upstream kernel that it shouldn't be a problem.
    Also logically, IMO it makes more sense to group the TGID with the
    TASK-PID and the CPU after these.
    
    Link: http://lkml.kernel.org/r/20180626000822.113931-1-joel@joelfernandes.org
    
    Cc: jreck@google.com
    Cc: tkjos@google.com
    Cc: stable@vger.kernel.org
    Fixes: 441dae8f2f29 ("tracing: Add support for display of tgid in trace output")
    Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
    Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f054bd6a1c66..87cf25171fb8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3365,8 +3365,8 @@ static void print_func_help_header(struct trace_buffer *buf, struct seq_file *m,
 
 	print_event_info(buf, m);
 
-	seq_printf(m, "#           TASK-PID   CPU#   %s  TIMESTAMP  FUNCTION\n", tgid ? "TGID     " : "");
-	seq_printf(m, "#              | |       |    %s     |         |\n",	 tgid ? "  |      " : "");
+	seq_printf(m, "#           TASK-PID   %s  CPU#   TIMESTAMP  FUNCTION\n", tgid ? "TGID     " : "");
+	seq_printf(m, "#              | |     %s    |       |         |\n",	 tgid ? "  |      " : "");
 }
 
 static void print_func_help_header_irq(struct trace_buffer *buf, struct seq_file *m,
@@ -3386,9 +3386,9 @@ static void print_func_help_header_irq(struct trace_buffer *buf, struct seq_file
 		   tgid ? tgid_space : space);
 	seq_printf(m, "#                          %s||| /     delay\n",
 		   tgid ? tgid_space : space);
-	seq_printf(m, "#           TASK-PID   CPU#%s||||    TIMESTAMP  FUNCTION\n",
+	seq_printf(m, "#           TASK-PID %sCPU#  ||||    TIMESTAMP  FUNCTION\n",
 		   tgid ? "   TGID   " : space);
-	seq_printf(m, "#              | |       | %s||||       |         |\n",
+	seq_printf(m, "#              | |   %s  |   ||||       |         |\n",
 		   tgid ? "     |    " : space);
 }
 
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 90db994ac900..1c8e30fda46a 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -594,8 +594,7 @@ int trace_print_context(struct trace_iterator *iter)
 
 	trace_find_cmdline(entry->pid, comm);
 
-	trace_seq_printf(s, "%16s-%-5d [%03d] ",
-			       comm, entry->pid, iter->cpu);
+	trace_seq_printf(s, "%16s-%-5d ", comm, entry->pid);
 
 	if (tr->trace_flags & TRACE_ITER_RECORD_TGID) {
 		unsigned int tgid = trace_find_tgid(entry->pid);
@@ -606,6 +605,8 @@ int trace_print_context(struct trace_iterator *iter)
 			trace_seq_printf(s, "(%5d) ", tgid);
 	}
 
+	trace_seq_printf(s, "[%03d] ", iter->cpu);
+
 	if (tr->trace_flags & TRACE_ITER_IRQ_INFO)
 		trace_print_lat_fmt(s, entry);
 

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2018-07-13 12:41 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 12:41 [GIT PULL][PATCH] tracing: Reorder display of TGID to be after PID Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).