Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753383Ab0DLGxP (ORCPT ); Mon, 12 Apr 2010 02:53:15 -0400 Received: from mailout1.zih.tu-dresden.de ([141.30.67.72]:42766 "EHLO mailout1.zih.tu-dresden.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751932Ab0DLGxN convert rfc822-to-8bit (ORCPT ); Mon, 12 Apr 2010 02:53:13 -0400 Subject: Re: [PATCH] trace power_frequency events on the correct cpu (for Intel x86 CPUs) From: Robert =?ISO-8859-1?Q?Sch=F6ne?= To: Arjan van de Ven Cc: Thomas Renninger , Dave Jones , Thomas Gleixner , Ingo Molnar , linux-kernel , cpufreq , x86@kernel.org In-Reply-To: <1270017629.3428.31.camel@localhost> References: <1268399863.3407.15.camel@localhost> <1269414437.3355.6.camel@localhost> <1269928015.3384.2.camel@localhost> <201003301056.52967.trenn@suse.de> <1270017629.3428.31.camel@localhost> Content-Type: text/plain; charset="UTF-8" Date: Mon, 12 Apr 2010 08:53:03 +0200 Message-ID: <1271055183.3416.20.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 Content-Transfer-Encoding: 8BIT X-TUD-Virus-Scanned: mailout1.zih.tu-dresden.de Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5052 Lines: 124 Resend, to keep the diskussion alive Am Mittwoch, den 31.03.2010, 08:40 +0200 schrieb Robert Schöne: > 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 > > CC: Robert Schöne > > CC: x86@kernel.org > > CC: cpufreq > > CC: Arjan van de Ven > > CC: stable@kernel.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@tu-dresden.de > > -- > 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/ > -- 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/