Re: [PATCH] trace power_frequency events on the correct cpu (for Intel x86 CPUs)

["Robert Schöne" <robert.schoene@tu-dresden.de>]

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@suse.de>
> CC: Robert Schöne <robert.schoene@tu-dresden.de>
> CC: x86@kernel.org
> CC: cpufreq <cpufreq@vger.kernel.org>
> CC: Arjan van de Ven <arjan@linux.intel.com>
> 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