lkml.org 
[lkml]   [2010]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH 2/3] PERF(kernel): Cleanup power events
    From
    On Thu, Nov 18, 2010 at 11:52 AM, Ingo Molnar <mingo@elte.hu> wrote:
    >
    > I am also getting build failures:
    >
    > drivers/cpufreq/cpufreq.c:357: error: 'POWER_PSTATE' undeclared (first use in this function)
    > drivers/cpufreq/cpufreq.c:357: error: (Each undeclared identifier is reported only once
    > drivers/cpufreq/cpufreq.c:357: error: for each function it appears in.)
    > arch/x86/kernel/process.c:375: error: 'POWER_CSTATE' undeclared (first use in this function)
    > arch/x86/kernel/process.c:375: error: (Each undeclared identifier is reported only once
    > arch/x86/kernel/process.c:375: error: for each function it appears in.)
    > arch/x86/kernel/process.c:446: error: 'POWER_CSTATE' undeclared (first use in this function)
    > arch/x86/kernel/process.c:463: error: 'POWER_CSTATE' undeclared (first use in this function)
    > arch/x86/kernel/process.c:485: error: 'POWER_CSTATE' undeclared (first use in this function)
    > include/trace/events/power.h:142: error: redefinition of 'trace_power_start'
    >
    > Config attached.

    The problem is because power.h gets included mutliple times, and so
    the POWER_ enum and the empty deprecated functions need to be
    protected from that.

    Here is a patch below that fixes it, compile tested with and without
    CONFIG_EVENT_POWER_TRACING_DEPRECATED set.

    Ingo, Thomas, please let me know if you want me tp refresh the patches
    with that fix.

    diff --git a/include/trace/events/power.h b/include/trace/events/power.h
    index 00d9819..89db5a1 100644
    --- a/include/trace/events/power.h
    +++ b/include/trace/events/power.h
    @@ -136,12 +136,24 @@ enum {
    POWER_PSTATE = 2,
    };
    #endif /* _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED */
    -#else
    +
    +#else /* CONFIG_EVENT_POWER_TRACING_DEPRECATED */
    +
    +#ifndef _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED
    +#define _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED
    +enum {
    + POWER_NONE = 0,
    + POWER_CSTATE = 1,
    + POWER_PSTATE = 2,
    +};
    +
    /* These dummy declaration have to be ripped out when the deprecated
    events get removed */
    static inline void trace_power_start(u64 type, u64 state, u64 cpuid) {};
    static inline void trace_power_end(u64 cpuid) {};
    static inline void trace_power_frequency(u64 type, u64 state, u64 cpuid) {};
    +#endif /* _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED */
    +
    #endif /* CONFIG_EVENT_POWER_TRACING_DEPRECATED */

    /*
    Thanks,
    Jean

    >
    > Note: please reuse the two commits from below for further work, i did some small
    > cleanups to the commit text and to the patches.
    >
    > Thanks,
    >
    >        Ingo
    >
    > ---------------->
    > From 87a2cfbda3f53c3bf00c424ce18d97b03b0c3aa0 Mon Sep 17 00:00:00 2001
    > From: Thomas Renninger <trenn@suse.de>
    > Date: Thu, 18 Nov 2010 10:25:13 +0100
    > Subject: [PATCH] perf: Clean up power events
    >
    > Add these new power trace events:
    >
    >  power:cpu_idle
    >  power:cpu_frequency
    >  power:machine_suspend
    >
    > The old C-state/idle accounting events:
    >  power:power_start
    >  power:power_end
    >
    > Have now a replacement (but we are still keeping the old
    > tracepoints for compatibility):
    >
    >  power:cpu_idle
    >
    > and
    >  power:power_frequency
    >
    > is replaced with:
    >  power:cpu_frequency
    >
    > power:machine_suspend is newly introduced.
    >
    > Jean Pihet has a patch integrated into the generic layer
    > (kernel/power/suspend.c) which will make use of it.
    >
    > the type= field got removed from both, it was never
    > used and the type is differed by the event type itself.
    >
    > perf timechart userspace tool gets adjusted in a separate patch.
    >
    > Signed-off-by: Thomas Renninger <trenn@suse.de>
    > Acked-by: Arjan van de Ven <arjan@linux.intel.com>
    > Acked-by: Jean Pihet <jean.pihet@newoldbits.com>
    > Cc: Linus Torvalds <torvalds@linux-foundation.org>
    > Cc: rjw@sisk.pl
    > LKML-Reference: <1290072314-31155-2-git-send-email-trenn@suse.de>
    > Signed-off-by: Ingo Molnar <mingo@elte.hu>
    > ---
    >  arch/x86/kernel/process.c    |    7 +++-
    >  arch/x86/kernel/process_32.c |    2 +-
    >  arch/x86/kernel/process_64.c |    2 +
    >  drivers/cpufreq/cpufreq.c    |    1 +
    >  drivers/cpuidle/cpuidle.c    |    1 +
    >  drivers/idle/intel_idle.c    |    1 +
    >  include/trace/events/power.h |   86 +++++++++++++++++++++++++++++++++++++----
    >  kernel/trace/Kconfig         |   15 +++++++
    >  kernel/trace/power-traces.c  |    3 +
    >  9 files changed, 107 insertions(+), 11 deletions(-)
    >
    > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
    > index 57d1868..155d975 100644
    > --- a/arch/x86/kernel/process.c
    > +++ b/arch/x86/kernel/process.c
    > @@ -374,6 +374,7 @@ void default_idle(void)
    >  {
    >        if (hlt_use_halt()) {
    >                trace_power_start(POWER_CSTATE, 1, smp_processor_id());
    > +               trace_cpu_idle(1, smp_processor_id());
    >                current_thread_info()->status &= ~TS_POLLING;
    >                /*
    >                 * TS_POLLING-cleared state must be visible before we
    > @@ -444,6 +445,7 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);
    >  void mwait_idle_with_hints(unsigned long ax, unsigned long cx)
    >  {
    >        trace_power_start(POWER_CSTATE, (ax>>4)+1, smp_processor_id());
    > +       trace_cpu_idle((ax>>4)+1, smp_processor_id());
    >        if (!need_resched()) {
    >                if (cpu_has(&current_cpu_data, X86_FEATURE_CLFLUSH_MONITOR))
    >                        clflush((void *)&current_thread_info()->flags);
    > @@ -460,6 +462,7 @@ static void mwait_idle(void)
    >  {
    >        if (!need_resched()) {
    >                trace_power_start(POWER_CSTATE, 1, smp_processor_id());
    > +               trace_cpu_idle(1, smp_processor_id());
    >                if (cpu_has(&current_cpu_data, X86_FEATURE_CLFLUSH_MONITOR))
    >                        clflush((void *)&current_thread_info()->flags);
    >
    > @@ -481,10 +484,12 @@ static void mwait_idle(void)
    >  static void poll_idle(void)
    >  {
    >        trace_power_start(POWER_CSTATE, 0, smp_processor_id());
    > +       trace_cpu_idle(0, smp_processor_id());
    >        local_irq_enable();
    >        while (!need_resched())
    >                cpu_relax();
    > -       trace_power_end(0);
    > +       trace_power_end(smp_processor_id());
    > +       trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
    >  }
    >
    >  /*
    > diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
    > index 96586c3..4b9befa 100644
    > --- a/arch/x86/kernel/process_32.c
    > +++ b/arch/x86/kernel/process_32.c
    > @@ -113,8 +113,8 @@ void cpu_idle(void)
    >                        stop_critical_timings();
    >                        pm_idle();
    >                        start_critical_timings();
    > -
    >                        trace_power_end(smp_processor_id());
    > +                       trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
    >                }
    >                tick_nohz_restart_sched_tick();
    >                preempt_enable_no_resched();
    > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
    > index b3d7a3a..4c818a7 100644
    > --- a/arch/x86/kernel/process_64.c
    > +++ b/arch/x86/kernel/process_64.c
    > @@ -142,6 +142,8 @@ void cpu_idle(void)
    >                        start_critical_timings();
    >
    >                        trace_power_end(smp_processor_id());
    > +                       trace_cpu_idle(PWR_EVENT_EXIT,
    > +                                      smp_processor_id());
    >
    >                        /* In many cases the interrupt that ended idle
    >                           has already called exit_idle. But some idle
    > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
    > index c63a438..1109f68 100644
    > --- a/drivers/cpufreq/cpufreq.c
    > +++ b/drivers/cpufreq/cpufreq.c
    > @@ -355,6 +355,7 @@ void cpufreq_notify_transition(struct cpufreq_freqs *freqs, unsigned int state)
    >                dprintk("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
    >                        (unsigned long)freqs->cpu);
    >                trace_power_frequency(POWER_PSTATE, freqs->new, freqs->cpu);
    > +               trace_cpu_frequency(freqs->new, freqs->cpu);
    >                srcu_notifier_call_chain(&cpufreq_transition_notifier_list,
    >                                CPUFREQ_POSTCHANGE, freqs);
    >                if (likely(policy) && likely(policy->cpu == freqs->cpu))
    > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
    > index a507108..08d5f05 100644
    > --- a/drivers/cpuidle/cpuidle.c
    > +++ b/drivers/cpuidle/cpuidle.c
    > @@ -107,6 +107,7 @@ static void cpuidle_idle_call(void)
    >        if (cpuidle_curr_governor->reflect)
    >                cpuidle_curr_governor->reflect(dev);
    >        trace_power_end(smp_processor_id());
    > +       trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
    >  }
    >
    >  /**
    > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
    > index 3c95325..ba5134f 100644
    > --- a/drivers/idle/intel_idle.c
    > +++ b/drivers/idle/intel_idle.c
    > @@ -221,6 +221,7 @@ static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
    >
    >        stop_critical_timings();
    >        trace_power_start(POWER_CSTATE, (eax >> 4) + 1, cpu);
    > +       trace_cpu_idle((eax >> 4) + 1, cpu);
    >        if (!need_resched()) {
    >
    >                __monitor((void *)&current_thread_info()->flags, 0, 0);
    > diff --git a/include/trace/events/power.h b/include/trace/events/power.h
    > index 286784d..00d9819 100644
    > --- a/include/trace/events/power.h
    > +++ b/include/trace/events/power.h
    > @@ -7,16 +7,67 @@
    >  #include <linux/ktime.h>
    >  #include <linux/tracepoint.h>
    >
    > -#ifndef _TRACE_POWER_ENUM_
    > -#define _TRACE_POWER_ENUM_
    > -enum {
    > -       POWER_NONE      = 0,
    > -       POWER_CSTATE    = 1,    /* C-State */
    > -       POWER_PSTATE    = 2,    /* Fequency change or DVFS */
    > -       POWER_SSTATE    = 3,    /* Suspend */
    > -};
    > +DECLARE_EVENT_CLASS(cpu,
    > +
    > +       TP_PROTO(unsigned int state, unsigned int cpu_id),
    > +
    > +       TP_ARGS(state, cpu_id),
    > +
    > +       TP_STRUCT__entry(
    > +               __field(        u32,            state           )
    > +               __field(        u32,            cpu_id          )
    > +       ),
    > +
    > +       TP_fast_assign(
    > +               __entry->state = state;
    > +               __entry->cpu_id = cpu_id;
    > +       ),
    > +
    > +       TP_printk("state=%lu cpu_id=%lu", (unsigned long)__entry->state,
    > +                 (unsigned long)__entry->cpu_id)
    > +);
    > +
    > +DEFINE_EVENT(cpu, cpu_idle,
    > +
    > +       TP_PROTO(unsigned int state, unsigned int cpu_id),
    > +
    > +       TP_ARGS(state, cpu_id)
    > +);
    > +
    > +/* This file can get included multiple times, TRACE_HEADER_MULTI_READ at top */
    > +#ifndef _PWR_EVENT_AVOID_DOUBLE_DEFINING
    > +#define _PWR_EVENT_AVOID_DOUBLE_DEFINING
    > +
    > +#define PWR_EVENT_EXIT -1
    >  #endif
    >
    > +DEFINE_EVENT(cpu, cpu_frequency,
    > +
    > +       TP_PROTO(unsigned int frequency, unsigned int cpu_id),
    > +
    > +       TP_ARGS(frequency, cpu_id)
    > +);
    > +
    > +TRACE_EVENT(machine_suspend,
    > +
    > +       TP_PROTO(unsigned int state),
    > +
    > +       TP_ARGS(state),
    > +
    > +       TP_STRUCT__entry(
    > +               __field(        u32,            state           )
    > +       ),
    > +
    > +       TP_fast_assign(
    > +               __entry->state = state;
    > +       ),
    > +
    > +       TP_printk("state=%lu", (unsigned long)__entry->state)
    > +);
    > +
    > +/* This code will be removed after deprecation time exceeded (2.6.41) */
    > +#ifdef CONFIG_EVENT_POWER_TRACING_DEPRECATED
    > +
    >  /*
    >  * The power events are used for cpuidle & suspend (power_start, power_end)
    >  *  and for cpufreq (power_frequency)
    > @@ -75,6 +126,24 @@ TRACE_EVENT(power_end,
    >
    >  );
    >
    > +/* Deprecated dummy functions must be protected against multi-declartion */
    > +#ifndef _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED
    > +#define _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED
    > +
    > +enum {
    > +       POWER_NONE = 0,
    > +       POWER_CSTATE = 1,
    > +       POWER_PSTATE = 2,
    > +};
    > +#endif /* _PWR_EVENT_AVOID_DOUBLE_DEFINING_DEPRECATED */
    > +#else
    > +/* These dummy declaration have to be ripped out when the deprecated
    > +   events get removed */
    > +static inline void trace_power_start(u64 type, u64 state, u64 cpuid) {};
    > +static inline void trace_power_end(u64 cpuid) {};
    > +static inline void trace_power_frequency(u64 type, u64 state, u64 cpuid) {};
    > +#endif /* CONFIG_EVENT_POWER_TRACING_DEPRECATED */
    > +
    >  /*
    >  * The clock events are used for clock enable/disable and for
    >  *  clock rate change
    > @@ -153,7 +222,6 @@ DEFINE_EVENT(power_domain, power_domain_target,
    >
    >        TP_ARGS(name, state, cpu_id)
    >  );
    > -
    >  #endif /* _TRACE_POWER_H */
    >
    >  /* This part must be outside protection */
    > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
    > index e04b8bc..59b44a1 100644
    > --- a/kernel/trace/Kconfig
    > +++ b/kernel/trace/Kconfig
    > @@ -69,6 +69,21 @@ config EVENT_TRACING
    >        select CONTEXT_SWITCH_TRACER
    >        bool
    >
    > +config EVENT_POWER_TRACING_DEPRECATED
    > +       depends on EVENT_TRACING
    > +       bool "Deprecated power event trace API, to be removed"
    > +       default y
    > +       help
    > +         Provides old power event types:
    > +         C-state/idle accounting events:
    > +         power:power_start
    > +         power:power_end
    > +         and old cpufreq accounting event:
    > +         power:power_frequency
    > +         This is for userspace compatibility
    > +         and will vanish after 5 kernel iterations,
    > +         namely 2.6.41.
    > +
    >  config CONTEXT_SWITCH_TRACER
    >        bool
    >
    > diff --git a/kernel/trace/power-traces.c b/kernel/trace/power-traces.c
    > index 0e0497d..f55fcf6 100644
    > --- a/kernel/trace/power-traces.c
    > +++ b/kernel/trace/power-traces.c
    > @@ -13,5 +13,8 @@
    >  #define CREATE_TRACE_POINTS
    >  #include <trace/events/power.h>
    >
    > +#ifdef EVENT_POWER_TRACING_DEPRECATED
    >  EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
    > +#endif
    > +EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle);
    >
    >
    > From b989c51b6f1989a834eecd9a64a7bd52ed230ea0 Mon Sep 17 00:00:00 2001
    > From: Thomas Renninger <trenn@suse.de>
    > Date: Thu, 18 Nov 2010 10:25:12 +0100
    > Subject: [PATCH] perf: Do not export power_frequency, but power_start event
    >
    > power_frequency moved to drivers/cpufreq/cpufreq.c which has
    > to be compiled in, no need to export it.
    >
    > intel_idle can a be module though...
    >
    > Signed-off-by: Thomas Renninger <trenn@suse.de>
    > Acked-by: Jean Pihet <jean.pihet@newoldbits.com>
    > CC: Arjan van de Ven <arjan@linux.intel.com>
    > Cc: rjw@sisk.pl
    > LKML-Reference: <1290072314-31155-2-git-send-email-trenn@suse.de>
    > Signed-off-by: Ingo Molnar <mingo@elte.hu>
    > ---
    >  drivers/idle/intel_idle.c   |    2 --
    >  kernel/trace/power-traces.c |    2 +-
    >  2 files changed, 1 insertions(+), 3 deletions(-)
    >
    > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
    > index 41665d2..3c95325 100644
    > --- a/drivers/idle/intel_idle.c
    > +++ b/drivers/idle/intel_idle.c
    > @@ -220,9 +220,7 @@ static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
    >        kt_before = ktime_get_real();
    >
    >        stop_critical_timings();
    > -#ifndef MODULE
    >        trace_power_start(POWER_CSTATE, (eax >> 4) + 1, cpu);
    > -#endif
    >        if (!need_resched()) {
    >
    >                __monitor((void *)&current_thread_info()->flags, 0, 0);
    > diff --git a/kernel/trace/power-traces.c b/kernel/trace/power-traces.c
    > index a22582a..0e0497d 100644
    > --- a/kernel/trace/power-traces.c
    > +++ b/kernel/trace/power-traces.c
    > @@ -13,5 +13,5 @@
    >  #define CREATE_TRACE_POINTS
    >  #include <trace/events/power.h>
    >
    > -EXPORT_TRACEPOINT_SYMBOL_GPL(power_frequency);
    > +EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
    >
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-11-18 17:37    [W:0.054 / U:57.528 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site