| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTimr91G+wrxj0p--2W0cLpoY06KDzDO=zDvG=3Ld@mail.gmail.com>
Date: Thu, 18 Nov 2010 17:34:15 +0100
From: Jean Pihet <jean.pihet@...oldbits.com>
To: Ingo Molnar <mingo@...e.hu>, Thomas Renninger <trenn@...e.de>
Cc: rjw@...k.pl, linux-kernel@...r.kernel.org, arjan@...ux.intel.com
Subject: Re: [PATCH 2/3] PERF(kernel): Cleanup power events
On Thu, Nov 18, 2010 at 11:52 AM, Ingo Molnar <mingo@...e.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@...e.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@...e.de>
> Acked-by: Arjan van de Ven <arjan@...ux.intel.com>
> Acked-by: Jean Pihet <jean.pihet@...oldbits.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: rjw@...k.pl
> LKML-Reference: <1290072314-31155-2-git-send-email-trenn@...e.de>
> Signed-off-by: Ingo Molnar <mingo@...e.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(¤t_cpu_data, X86_FEATURE_CLFLUSH_MONITOR))
> clflush((void *)¤t_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(¤t_cpu_data, X86_FEATURE_CLFLUSH_MONITOR))
> clflush((void *)¤t_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 *)¤t_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@...e.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@...e.de>
> Acked-by: Jean Pihet <jean.pihet@...oldbits.com>
> CC: Arjan van de Ven <arjan@...ux.intel.com>
> Cc: rjw@...k.pl
> LKML-Reference: <1290072314-31155-2-git-send-email-trenn@...e.de>
> Signed-off-by: Ingo Molnar <mingo@...e.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 *)¤t_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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists