Is such perf stuff supposed to go through x86-tip?
Can someone pick these two up, please.

Thanks,

    Thomas

--

[ message continues ]

This has been found be review.
Currently PRE/POSTCHANGE events seem to only be thrown for one core
if the BIOS tells us to run in CPU_ANY mode (switching one/any
of the depending cores is enough).

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: venki@google.com
CC: davej@redhat.com
CC: linux-kernel@vger.kernel.org
CC: cpufreq@vger.kernel.org
CC: linux-perf-users@vger.kernel.org
CC: linux-trace-users@vger.kernel.org
CC: x86@kernel.org
CC: robert.schoene@tu-dresden.de
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 4591680..c6de3a9 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -391,7 +391,7 @@ static int acpi_cpufreq_target(struct cpufreq_policy *policy,
 
 	freqs.old = perf->states[perf->state].core_frequency * 1000;
 	freqs.new = data->freq_table[next_state].frequency;
-	for_each_cpu(i, cmd.mask) {
+	for_each_cpu(i, policy->cpus) {
 		freqs.cpu = i;
 		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 	}
@@ -407,7 +407,7 @@ static int acpi_cpufreq_target(struct cpufreq_policy *policy,
 		}
 	}
 
-	for_each_cpu(i, cmd.mask) {
+	for_each_cpu(i, policy->cpus) {
 		freqs.cpu = i;
 		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 	}
-- 
1.6.3

--

[ message continues ]

Acked-by: Venkatesh Pallipadi <venki@google.com>

--

[ message continues ]

and fix the broken case if a core's frequency depends on others.

trace_power_frequency was only implemented in a rather ungeneric way
in acpi-cpufreq driver's target() function only.
-> Move the call to trace_power_frequency to
   cpufreq.c:cpufreq_notify_transition() where CPUFREQ_POSTCHANGE
   notifier is triggered.
   This will support power frequency tracing by all cpufreq drivers

trace_power_frequency did not trace frequency changes correctly when
the userspace governor was used or when CPU cores' frequency depend
on each other.
-> Moving this into the CPUFREQ_POSTCHANGE notifier and pass the cpu
   which gets switched automatically fixes this.

Robert Schoene provided some important fixes on top of my initial
quick shot version which are integrated in this patch:
- Forgot some changes in power_end trace (TP_printk/variable names)
- Variable dummy in power_end must now be cpu_id
- Use static 64 bit variable instead of unsigned int for cpu_id

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: davej@redhat.com
CC: arjan@infradead.org
CC: linux-kernel@vger.kernel.org
CC: robert.schoene@tu-dresden.de
CC: cpufreq@vger.kernel.org
Tested-by: robert.schoene@tu-dresden.de
CC: linux-perf-users@vger.kernel.org
CC: linux-trace-users@vger.kernel.org
CC: x86@kernel.org
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    3 ---
 arch/x86/kernel/process.c                  |    8 ++++----
 drivers/cpufreq/cpufreq.c                  |    5 +++++
 drivers/cpuidle/cpuidle.c                  |    2 +-
 include/trace/events/power.h               |   27 +++++++++++++++------------
 tools/perf/builtin-timechart.c             |   11 ++++++-----
 6 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index c6de3a9..9241402 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -34,7 +34,6 @@
 #include <linux/compiler.h>
 #include ...
[ message continues ]

Currently the C-states get triggered on the machine you are running on.
But you can have C-state dependencies (ACPI spec at least defines this 
the same way as done for P-states, not sure whether this exists in
reality already, Linux does not evaluate them yet).
The same way we run into trouble with P-states (SW_ALL, SW_ANY, HW).
Not sure how this will/could show up in reality in HW or implementation, but
as these dependencies are already defined in spec, it sounds like a good idea
to pass the CPU through the POWER_CSTATE events as well.

Compare with chapter 8.4.2.2 _CSD (C-State Dependency)
of an ACPI spec 3.0 or newer.

There are also other architectures starting to use processor sleep states.

So this is not actually used (smp_processor_id() is the same as already
tracked by trace event internally), but to make the interface more robust
Oops. Tell me if you like to push it and I send a checkpatch cleaned up version.
Sorry about that.

     Thomas
--

[ message continues ]

Extending power tracing to all cpufreq modules is obviously a good thing.

But why is trace_power_start() adding a CPU ID argument? CPU ids are already 
available and can be sampled via PERF_SAMPLE_CPU if needed. AFAICS only 

Timechart is maintained by Arjan so we need an ack from him as well. I've seen 
some back and forth in the discussions - what's the technical resolution of 
that?

(Also, there's some whitespace noise in the patch.)

Thanks,

	Ingo
--

[ message continues ]