lkml.org 
[lkml]   [2009]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: PATCH] ftrace: Add a C/P state tracer to help power optimization
On Mon, Oct 27, 2008 at 02:06:30PM -0700, Arjan van de Ven wrote:
> > Arjan van de Ven <arjan@infradead.org> writes:
> >
> > > [...]
> > > --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > > +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > > [...]
> > > @@ -427,6 +429,8 @@ static int acpi_cpufreq_target(struct
> > > cpufreq_policy *policy, }
> > > }
> > >
> > > + trace_power_mark(&it, POWER_PSTATE, next_perf_state);
> > > +
> > > switch (data->cpu_feature) {
> > > case SYSTEM_INTEL_MSR_CAPABLE:
> > > cmd.type = SYSTEM_INTEL_MSR_CAPABLE;
> > > [...]
> >
> > Is there some reason that this doesn't use tracepoints instead
> > of such a single-backend hook?
>
> because it's a ton simpler this way? do simple things simpe and all
> that....
>

hi,

I wrote a patch to make c/p state tracer dependent on tracepoints and
then realized that the discussion had already been had. However, the
patch to use tracepoints is fairly simple, allows for other consumers,
and avoids a function call in the off case. please consider.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@redhat.com>

---

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 3 +++
arch/x86/kernel/process.c | 4 ++++
include/linux/ftrace.h | 15 ---------------
include/trace/power.h | 18 ++++++++++++++++++
kernel/trace/trace_power.c | 28 ++++++++++++++++++++--------
5 files changed, 45 insertions(+), 23 deletions(-)
create mode 100644 include/trace/power.h


diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 4b1c319..4540ddc 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -34,6 +34,7 @@
#include <linux/compiler.h>
#include <linux/dmi.h>
#include <linux/ftrace.h>
+#include <trace/power.h>

#include <linux/acpi.h>
#include <acpi/processor.h>
@@ -70,6 +71,8 @@ struct acpi_cpufreq_data {

static DEFINE_PER_CPU(struct acpi_cpufreq_data *, drv_data);

+DEFINE_TRACE(power_mark);
+
/* acpi_perf_data is a pointer to percpu data. */
static struct acpi_processor_performance *acpi_perf_data;

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index e68bb9e..09cfd5d 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -9,6 +9,7 @@
#include <linux/pm.h>
#include <linux/clockchips.h>
#include <linux/ftrace.h>
+#include <trace/power.h>
#include <asm/system.h>
#include <asm/apic.h>

@@ -19,6 +20,9 @@ EXPORT_SYMBOL(idle_nomwait);

struct kmem_cache *task_xstate_cachep;

+DEFINE_TRACE(power_start);
+DEFINE_TRACE(power_end);
+
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
{
*dst = *src;
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 677432b..044e0fd 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -342,21 +342,6 @@ struct power_trace {
#endif
};

-#ifdef CONFIG_POWER_TRACER
-extern void trace_power_start(struct power_trace *it, unsigned int type,
- unsigned int state);
-extern void trace_power_mark(struct power_trace *it, unsigned int type,
- unsigned int state);
-extern void trace_power_end(struct power_trace *it);
-#else
-static inline void trace_power_start(struct power_trace *it, unsigned int type,
- unsigned int state) { }
-static inline void trace_power_mark(struct power_trace *it, unsigned int type,
- unsigned int state) { }
-static inline void trace_power_end(struct power_trace *it) { }
-#endif
-
-
/*
* Structure that defines an entry function trace.
*/
diff --git a/include/trace/power.h b/include/trace/power.h
new file mode 100644
index 0000000..c3225ff
--- /dev/null
+++ b/include/trace/power.h
@@ -0,0 +1,18 @@
+#ifndef _TRACE_POWER_H
+#define _TRACE_POWER_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_TRACE(power_start,
+ TPPROTO(struct power_trace *it, unsigned int type, unsigned int state),
+ TPARGS(it, type, state));
+
+DECLARE_TRACE(power_mark,
+ TPPROTO(struct power_trace *it, unsigned int type, unsigned int state),
+ TPARGS(it, type, state));
+
+DECLARE_TRACE(power_end,
+ TPPROTO(struct power_trace *it),
+ TPARGS(it));
+
+#endif
diff --git a/kernel/trace/trace_power.c b/kernel/trace/trace_power.c
index 7bda248..18e97c9 100644
--- a/kernel/trace/trace_power.c
+++ b/kernel/trace/trace_power.c
@@ -14,30 +14,45 @@
#include <linux/ftrace.h>
#include <linux/kallsyms.h>
#include <linux/module.h>
+#include <linux/module.h>
+#include <trace/power.h>

#include "trace.h"

static struct trace_array *power_trace;
static int __read_mostly trace_power_enabled;

+static void trace_power_start_callback(struct power_trace *it,
+ unsigned int type, unsigned int state);
+static void trace_power_mark_callback(struct power_trace *it, unsigned int type,
+ unsigned int state);
+static void trace_power_end_callback(struct power_trace *it);

static void start_power_trace(struct trace_array *tr)
{
trace_power_enabled = 1;
+ register_trace_power_start(trace_power_start_callback);
+ register_trace_power_end(trace_power_end_callback);
+ register_trace_power_mark(trace_power_mark_callback);
}

static void stop_power_trace(struct trace_array *tr)
{
trace_power_enabled = 0;
+ unregister_trace_power_start(trace_power_start_callback);
+ unregister_trace_power_end(trace_power_end_callback);
+ unregister_trace_power_mark(trace_power_mark_callback);
}

-
static int power_trace_init(struct trace_array *tr)
{
int cpu;
power_trace = tr;

trace_power_enabled = 1;
+ register_trace_power_start(trace_power_start_callback);
+ register_trace_power_end(trace_power_end_callback);
+ register_trace_power_mark(trace_power_mark_callback);

for_each_cpu(cpu, cpu_possible_mask)
tracing_reset(tr, cpu);
@@ -95,8 +110,8 @@ static int init_power_trace(void)
}
device_initcall(init_power_trace);

-void trace_power_start(struct power_trace *it, unsigned int type,
- unsigned int level)
+static void trace_power_start_callback(struct power_trace *it,
+ unsigned int type, unsigned int level)
{
if (!trace_power_enabled)
return;
@@ -106,10 +121,9 @@ void trace_power_start(struct power_trace *it, unsigned int type,
it->type = type;
it->stamp = ktime_get();
}
-EXPORT_SYMBOL_GPL(trace_power_start);


-void trace_power_end(struct power_trace *it)
+static void trace_power_end_callback(struct power_trace *it)
{
struct ring_buffer_event *event;
struct trace_power *entry;
@@ -139,9 +153,8 @@ void trace_power_end(struct power_trace *it)
out:
preempt_enable();
}
-EXPORT_SYMBOL_GPL(trace_power_end);

-void trace_power_mark(struct power_trace *it, unsigned int type,
+static void trace_power_mark_callback(struct power_trace *it, unsigned int type,
unsigned int level)
{
struct ring_buffer_event *event;
@@ -176,4 +189,3 @@ void trace_power_mark(struct power_trace *it, unsigned int type,
out:
preempt_enable();
}
-EXPORT_SYMBOL_GPL(trace_power_mark);

\
 
 \ /
  Last update: 2009-02-10 22:01    [W:0.186 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site