trace power_frequency events on the correct cpu (for Intel x86 CPUs)
in [Kernel]
|
Prev: [PATCH -v2] x86, k8 nb: Enable k8_northbridges unconditionally on AMD
Next: [RFC] resource, PCI: work around pci=use_crs conflicts
From: Robert Schöne on 31 Mar 2010 02:50 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(a)suse.de> > CC: Robert Schöne <robert.schoene(a)tu-dresden.de> > CC: x86(a)kernel.org > CC: cpufreq <cpufreq(a)vger.kernel.org> > CC: Arjan van de Ven <arjan(a)linux.intel.com> > CC: stable(a)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(a)tu-dresden.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Robert Schöne on 12 Apr 2010 03:00
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 <trenn(a)suse.de> > > CC: Robert Schöne <robert.schoene(a)tu-dresden.de> > > CC: x86(a)kernel.org > > CC: cpufreq <cpufreq(a)vger.kernel.org> > > CC: Arjan van de Ven <arjan(a)linux.intel.com> > > CC: stable(a)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(a)tu-dresden.de > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo(a)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(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |