Messages in this thread | | | Date | Mon, 27 Oct 2008 16:59:21 +0100 | From | Ingo Molnar <> | Subject | Re: PATCH] ftrace: Add a C/P state tracer to help power optimization |
| |
* Arjan van de Ven <arjan@infradead.org> wrote:
> From: Arjan van de Ven <arjan@linux.intel.com> > Date: Fri, 3 Oct 2008 10:18:21 -0700 > Subject: [PATCH] ftrace: Add a C/P state tracer to help power optimization > > This patch adds a C/P-state ftrace plugin that will generate > detailed statistics about the C/P-states that are being used, > so that we can look at detailed decisions that the C/P-state > code is making, rather than the too high level "average" > that we have today.
nice! No fundamental objections - Len, do you concur? We could carry this in the ftrace tree, the impact on the rest of the kernel is minimal.
A few minor nits:
> static void poll_idle(void) > { > + struct power_trace it; > + trace_power_start(&it, POWER_CSTATE, 0);
please put a newline after variable definitions.
> --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -232,6 +232,32 @@ static inline void start_boot_trace(void) { } > static inline void stop_boot_trace(void) { } > #endif > > +#define POWER_NONE 0 > +#define POWER_CSTATE 1 > +#define POWER_PSTATE 2 > +struct power_trace {
the defines should be an enum and please put a newline after the enum definition.
> +#ifdef CONFIG_POWER_TRACER > + ktime_t stamp; > + ktime_t end; > + int type; > + int state; > +#endif > +};
> index 396aea1..1f797bd 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -134,6 +134,17 @@ config BOOT_TRACER > be enabled if this tracer is selected since only one tracer > should touch the tracing buffer at a time. > > +config POWER_TRACER > + bool "Trace power consumption behavior" > + depends on DEBUG_KERNEL > + depends on X86 > + select TRACING
this should be HAVE_POWER_TRACER defined in arch/x86/Kconfig, instead of a 'depends on X86'.
> + help > + This tracer helps developers to analyize and optimize the kernels > + power management decisions, specifically the C-state and P-state > + behavior.
s/analyize/analyse s/kernels/kernel's
> +static struct trace_array *power_trace; > +static int trace_power_enabled;
__read_mostly ?
Ingo
| |