Re: [PATCH -tip FIXED] ftrace: add nop tracer

Previous thread: [PATCH -tip] ftrace: add nop tracer by Steven Noonan on Friday, September 19, 2008 - 3:02 am. (1 message)

Next thread: [GIT PULL] AMD IOMMU updates for 2.6.28 by Joerg Roedel on Friday, September 19, 2008 - 3:07 am. (11 messages)
From: Steven Noonan
Date: Friday, September 19, 2008 - 3:06 am

A no-op tracer which can serve two purposes:
 1. A template for development of a new tracer.
 2. A convenient way to see ftrace_printk() calls without
    an irrelevant trace making the output messy.

Signed-off-by: Steven Noonan <steven@uplinklabs.net>
---
 kernel/trace/Kconfig          |   10 ++++++
 kernel/trace/Makefile         |    1 +
 kernel/trace/trace.h          |    4 ++
 kernel/trace/trace_nop.c      |   65 +++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_selftest.c |    9 ++++++
 5 files changed, 89 insertions(+), 0 deletions(-)
 create mode 100644 kernel/trace/trace_nop.c

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 16e5bb5..d7b2de7 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -101,6 +101,16 @@ config SCHED_TRACER
 	  This tracer tracks the latency of the highest priority task
 	  to be scheduled in, starting from the point it has woken up.
 
+config NOP_TRACER
+	bool "NOP Tracer"
+	depends on HAVE_FTRACE
+	depends on DEBUG_KERNEL
+	select TRACING
+	help
+	  This tracer does nothing. The primary purpose for it is to
+	  politely print the output of ftrace_printk() calls without
+	  the overhead of an irrelevant trace taking place.
+
 config CONTEXT_SWITCH_TRACER
 	bool "Trace process context switches"
 	depends on HAVE_FTRACE
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 58ec61c..73ba13f 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_FTRACE) += trace_functions.o
 obj-$(CONFIG_IRQSOFF_TRACER) += trace_irqsoff.o
 obj-$(CONFIG_PREEMPT_TRACER) += trace_irqsoff.o
 obj-$(CONFIG_SCHED_TRACER) += trace_sched_wakeup.o
+obj-$(CONFIG_NOP_TRACER) += trace_nop.o
 obj-$(CONFIG_STACK_TRACER) += trace_stack.o
 obj-$(CONFIG_MMIOTRACE) += trace_mmiotrace.o
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 42f65d0..447d4b9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -339,6 +339,10 @@ extern int ...
From: Ingo Molnar
Date: Friday, September 19, 2008 - 3:43 am

that was quick :) Applied to tip/tracing/ftrace, thanks Steven!

	Ingo
--

From: Frédéric Weisbecker
Date: Friday, September 19, 2008 - 3:48 am

Hi Steven and Ingo!

It seems we have two "none" tracers now.
Why not seeing this nop tracer as an improvment of the current "none tracer" ?

As a result the none tracer contained in trace.c could be replaced by
this new one as the default tracer. And that's said, that will enable
trace_printk by default even when no tracer is selected by the user.

What do you think about it?
--

From: Ingo Molnar
Date: Friday, September 19, 2008 - 4:04 am

hmm ... good idea. There are some special properties of the none tracer 
(it inhibits trace output for example), and those should be discontinued 
i guess.

	Ingo
--

From: Frédéric Weisbecker
Date: Friday, September 19, 2008 - 4:30 am

Ok. I will prepare a patch to submit this idea.
--

From: Andrew Morton
Date: Friday, September 19, 2008 - 2:23 pm

On Fri, 19 Sep 2008 03:06:43 -0700

Consider omitting the ifdefs around the declarations.

pro: the code looks nicer and is less likely to suffer build errors
with weird config combinations.

con: build errors are reported at link-time rather than at compile-time.

Personally, I think pro>con here.
--

From: Steven Noonan
Date: Friday, September 19, 2008 - 4:35 pm

On Fri, Sep 19, 2008 at 2:23 PM, Andrew Morton

I merely based this on one of the other tracers, so the ifdefs are
basically copied and renamed.
--

From: Ingo Molnar
Date: Friday, September 19, 2008 - 11:16 pm

yeah. Feel free to send a separate patch that implements Andrew's 
clean-up suggestion. I agree that pro>con.

	Ingo
--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 12:33 am

How about this patch gets applied to -tip and I submit one that drops
the ifdefs on this and the other tracers?
--

From: Andrew Morton
Date: Saturday, September 20, 2008 - 12:47 am

yup.  That's in fact the preferred way of separating patches.  "one
concept per patch" is the old mantra.
--

From: Ingo Molnar
Date: Saturday, September 20, 2008 - 4:31 am

yeah - it's been part of tip/master since yesterday.

	Ingo
--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 8:04 am

I saw it was applied about a minute after sending that. I should've
checked first. ;)
--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 1:00 am

When CONFIG_DYNAMIC_FTRACE isn't used, neither is mcount_addr. This
patch eliminates that warning.

Signed-off-by: Steven Noonan <steven@uplinklabs.net>
---
 kernel/trace/ftrace.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 89ed43b..f484909 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -37,14 +37,6 @@ int ftrace_enabled __read_mostly;
 static int last_ftrace_enabled;
 
 /*
- * Since MCOUNT_ADDR may point to mcount itself, we do not want
- * to get it confused by reading a reference in the code as we
- * are parsing on objcopy output of text. Use a variable for
- * it instead.
- */
-static unsigned long mcount_addr = MCOUNT_ADDR;
-
-/*
  * ftrace_disabled is set when an anomaly is discovered.
  * ftrace_disabled is much stronger than ftrace_enabled.
  */
@@ -178,6 +170,14 @@ static DEFINE_SPINLOCK(ftrace_hash_lock);
 #define ftrace_hash_unlock(flags) do { } while(0)
 #endif
 
+/*
+ * Since MCOUNT_ADDR may point to mcount itself, we do not want
+ * to get it confused by reading a reference in the code as we
+ * are parsing on objcopy output of text. Use a variable for
+ * it instead.
+ */
+static unsigned long mcount_addr = MCOUNT_ADDR;
+
 static struct task_struct *ftraced_task;
 
 enum {
-- 
1.6.0.2

--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 1:00 am

The functions are already 'extern' anyway, so there's no problem
with linkage. Removing these ifdefs also helps find any potential
compiler errors.

Signed-off-by: Steven Noonan <steven@uplinklabs.net>
---
 kernel/trace/trace.h |   16 ----------------
 1 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 447d4b9..c8c6870 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -319,38 +319,22 @@ extern int DYN_FTRACE_TEST_NAME(void);
 #endif
 
 #ifdef CONFIG_FTRACE_STARTUP_TEST
-#ifdef CONFIG_FTRACE
 extern int trace_selftest_startup_function(struct tracer *trace,
 					   struct trace_array *tr);
-#endif
-#ifdef CONFIG_IRQSOFF_TRACER
 extern int trace_selftest_startup_irqsoff(struct tracer *trace,
 					  struct trace_array *tr);
-#endif
-#ifdef CONFIG_PREEMPT_TRACER
 extern int trace_selftest_startup_preemptoff(struct tracer *trace,
 					     struct trace_array *tr);
-#endif
-#if defined(CONFIG_IRQSOFF_TRACER) && defined(CONFIG_PREEMPT_TRACER)
 extern int trace_selftest_startup_preemptirqsoff(struct tracer *trace,
 						 struct trace_array *tr);
-#endif
-#ifdef CONFIG_SCHED_TRACER
 extern int trace_selftest_startup_wakeup(struct tracer *trace,
 					 struct trace_array *tr);
-#endif
-#ifdef CONFIG_NOP_TRACER
 extern int trace_selftest_startup_nop(struct tracer *trace,
 					 struct trace_array *tr);
-#endif
-#ifdef CONFIG_CONTEXT_SWITCH_TRACER
 extern int trace_selftest_startup_sched_switch(struct tracer *trace,
 					       struct trace_array *tr);
-#endif
-#ifdef CONFIG_SYSPROF_TRACER
 extern int trace_selftest_startup_sysprof(struct tracer *trace,
 					       struct trace_array *tr);
-#endif
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
 extern void *head_page(struct trace_array_cpu *data);
-- 
1.6.0.2

--

From: Ingo Molnar
Date: Saturday, September 20, 2008 - 4:33 am

applied to tip/tracing/ftrace, thanks Steven,

	Ingo
--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 1:31 am

This line looks like a typo. I think I screwed up a comment block
there. I'll re-submit a patch in a moment.
--

From: Steven Noonan
Date: Saturday, September 20, 2008 - 1:33 am

Nevermind, I read it wrong. diff is messing with my head.
--

From: Ingo Molnar
Date: Saturday, September 20, 2008 - 4:33 am

applied to tip/tracing/ftrace, thanks Steven!

	Ingo
--

Previous thread: [PATCH -tip] ftrace: add nop tracer by Steven Noonan on Friday, September 19, 2008 - 3:02 am. (1 message)

Next thread: [GIT PULL] AMD IOMMU updates for 2.6.28 by Joerg Roedel on Friday, September 19, 2008 - 3:07 am. (11 messages)