[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1270017629.3428.31.camel@localhost>
Date: Wed, 31 Mar 2010 08:40:29 +0200
From: Robert Schöne <robert.schoene@...dresden.de>
To: Thomas Renninger <trenn@...e.de>
Cc: Arjan van de Ven <arjan@...ux.intel.com>,
Dave Jones <davej@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
cpufreq <cpufreq@...r.kernel.org>, x86@...nel.org
Subject: Re: [PATCH] trace power_frequency events on the correct cpu (for
Intel x86 CPUs)
Am Dienstag, den 30.03.2010, 09:56 +0100 schrieb Thomas Renninger:
> On Tuesday 30 March 2010 07:46:55 Robert Schöne wrote:
> ...
> > I really want to keep this diskussion alive until there's a soultion we
> > can all agree.
> > So Arjan and Thomas, are there any comments/preferences to the proposed
> > options?
> I'd like to extend the powertracer and pass the cpu.
> This is the only possibility I see to be able to support IO driven
> frequency switching drivers where the switching code must not be executed
> on the CPU that gets switched (without executing the tracer on each
> CPU explicitly which does not make sense).
As I understand you, you want to extend the event data of each power
trace event, which would be fine for me.
However, I think this would need some resorting of the events for the
perf tools.
@Arjan would this be feasible?
>
> The next problem where current implementation is unfixable broken with
> the tracer just passing the frequency is the fact that several CPUs
> could get switched with one MSR write to a depending CPU (SW_ANY).
> The same btw applies to C-states for which the tracer is used in
> the same way (compare with 8.4.2.2 _CSD (C-State Dependency) of a
> current ACPI spec).
>
> No idea what the impact on userspace tools is, if I find some time
> I can have a look at timechart how trace data gets read/used.
> But I fear the Cstate tracing is used in some more tools already?
> It would be great to get feedback/suggestions from people making use
> of it already.
>
> Below is still broken, but should make things at least a bit better:
>
> ---
> X86 cpufreq: Fix powertracer in acpi-cpufreq and exec it on the correct cpu(s)
>
> Several things are broken with the tracer currently.
> This patch fixes:
> - With the userspace governor the wrong cpu could get tracked if the target
> function is executed on a CPU which does not get switched
> - In SW_ALL CPU dependency case (CPUFREQ_SHARED_TYPE_ALL) only one CPU got
> tracked. Now all CPUs that depend on each other are tracked.
>
> What this patch does not fix:
> - In SW_ANY case (CPUFREQ_SHARED_TYPE_ANY) it's enough to write to a MSR of
> one of the depending CPUs. The power trace macro misses the ability
> to pass the cpu. Thus only one of the depending CPUs gets tracked correctly.
> To be able to fix this the power trace macro must get extended.
>
>
> Signed-off-by: Thomas Renninger <trenn@...e.de>
> CC: Robert Schöne <robert.schoene@...dresden.de>
> CC: x86@...nel.org
> CC: cpufreq <cpufreq@...r.kernel.org>
> CC: Arjan van de Ven <arjan@...ux.intel.com>
> CC: stable@...nel.org
>
> ---
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 1b1920f..259c49e 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -144,6 +144,7 @@ struct drv_cmd {
> struct io_addr io;
> } addr;
> u32 val;
> + unsigned int frequency;
> };
>
> /* Called via smp_call_function_single(), on the target CPU */
> @@ -177,11 +178,13 @@ static void do_drv_write(void *_cmd)
> rdmsr(cmd->addr.msr.reg, lo, hi);
> lo = (lo & ~INTEL_MSR_RANGE) | (cmd->val & INTEL_MSR_RANGE);
> wrmsr(cmd->addr.msr.reg, lo, hi);
> + trace_power_frequency(POWER_PSTATE, cmd->frequency);
> break;
> case SYSTEM_IO_CAPABLE:
> acpi_os_write_port((acpi_io_address)cmd->addr.io.port,
> cmd->val,
> (u32)cmd->addr.io.bit_width);
> + trace_power_frequency(POWER_PSTATE, cmd->frequency);
> break;
> default:
> break;
> @@ -363,7 +366,7 @@ static int acpi_cpufreq_target(struct cpufreq_policy *policy,
> }
> }
>
> - trace_power_frequency(POWER_PSTATE, data->freq_table[next_state].frequency);
> + cmd.frequency = data->freq_table[next_state].frequency;
>
> switch (data->cpu_feature) {
> case SYSTEM_INTEL_MSR_CAPABLE:
>
>
--
Robert Schoene
Technische Universitaet Dresden
Zentrum fuer Informationsdienste und Hochleistungsrechnen
01062 Dresden
Tel.: (0351) 463-42483, Fax: (0351) 463-37773
E-Mail: Robert.Schoene@...dresden.de
--
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