lkml.org 
[lkml]   [2008]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 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


    \
     
     \ /
      Last update: 2008-10-27 17:01    [W:0.046 / U:61.344 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site