All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 4/5] tracing/core: drop the cpu field from trace_entry
Date: Sat, 7 Feb 2009 23:32:51 +0100	[thread overview]
Message-ID: <498e176a.0637560a.3cea.3ff8@mx.google.com> (raw)

Impact: fix a bug on cpu output

While looking on the function tracer traces, I first cheered up:

            TASK-PID    CPU#    TIMESTAMP  FUNCTION
               | |       |          |         |
         firefox-4622  [081]  1315.790405: lapic_next_event <-clockevents_program_event
         firefox-4622  [178]  1315.790405: native_apic_mem_write <-lapic_next_event
         firefox-4622  [020]  1315.790406: perf_counter_unthrottle <-smp_apic_timer_interrupt

But no, actually I don't have so many cpus...

This is a small bug: the cpu field on trace entry is never set up. Lots of tracers use it
on their context info to display the cpu number. But the cpu number on which a trace occured
is already known elsewhere: the ring-buffer is per cpu and we know
which cpu was used to store an event on the ring buffer, this information is on the
struct trace_iterator.

So both are overlaping. The cpu field on trace_entry is useless.
Unfortunatly we are not gaining any bytes, because the cpu is an unsigned char, the structure
will be padded to stay well memory aligned.

After this patch:

          <idle>-0     [001]    67.452518: __rcu_read_unlock <-input_pass_event
          <idle>-0     [000]    67.452518: ktime_get <-tick_nohz_stop_idle
          <idle>-0     [000]    67.452518: ktime_get_ts <-ktime_get
          <idle>-0     [001]    67.452518: _spin_unlock_irqrestore <-input_event

Sadly I've retrieved my two cpus....

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c        |    2 +-
 kernel/trace/trace.h        |    1 -
 kernel/trace/trace_output.c |    6 +++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ef4dbac..03fbd4c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1519,7 +1519,7 @@ static enum print_line_t print_bin_fmt(struct trace_iterator *iter)
 
 	if (trace_flags & TRACE_ITER_CONTEXT_INFO) {
 		SEQ_PUT_FIELD_RET(s, entry->pid);
-		SEQ_PUT_FIELD_RET(s, entry->cpu);
+		SEQ_PUT_FIELD_RET(s, iter->cpu);
 		SEQ_PUT_FIELD_RET(s, iter->ts);
 	}
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 95dff7d..1cd7f7f 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -46,7 +46,6 @@ enum trace_type {
  */
 struct trace_entry {
 	unsigned char		type;
-	unsigned char		cpu;
 	unsigned char		flags;
 	unsigned char		preempt_count;
 	int			pid;
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index b6e99af..9fc8150 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -333,7 +333,7 @@ int trace_print_context(struct trace_iterator *iter)
 	unsigned long secs = (unsigned long)t;
 
 	return trace_seq_printf(s, "%16s-%-5d [%03d] %5lu.%06lu: ",
-				comm, entry->pid, entry->cpu, secs, usec_rem);
+				comm, entry->pid, iter->cpu, secs, usec_rem);
 }
 
 int trace_print_lat_context(struct trace_iterator *iter)
@@ -356,7 +356,7 @@ int trace_print_lat_context(struct trace_iterator *iter)
 		char *comm = trace_find_cmdline(entry->pid);
 		ret = trace_seq_printf(s, "%16s %5d %3d %d %08x %08lx [%08lx]"
 				       " %ld.%03ldms (+%ld.%03ldms): ", comm,
-				       entry->pid, entry->cpu, entry->flags,
+				       entry->pid, iter->cpu, entry->flags,
 				       entry->preempt_count, iter->idx,
 				       ns2usecs(iter->ts),
 				       abs_usecs / USEC_PER_MSEC,
@@ -364,7 +364,7 @@ int trace_print_lat_context(struct trace_iterator *iter)
 				       rel_usecs / USEC_PER_MSEC,
 				       rel_usecs % USEC_PER_MSEC);
 	} else {
-		ret = lat_print_generic(s, entry, entry->cpu);
+		ret = lat_print_generic(s, entry, iter->cpu);
 		if (ret)
 			ret = lat_print_timestamp(s, abs_usecs, rel_usecs);
 	}
-- 
1.6.1



             reply	other threads:[~2009-02-07 23:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-07 22:32 Frederic Weisbecker [this message]
2009-02-09  9:53 ` [PATCH 4/5] tracing/core: drop the cpu field from trace_entry Ingo Molnar
2009-02-09 13:47   ` Frederic Weisbecker
2009-02-09 14:03     ` Ingo Molnar
2009-02-09 16:07       ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=498e176a.0637560a.3cea.3ff8@mx.google.com \
    --to=fweisbec@gmail.com \
    --cc=acme@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.