Re: [Xen-devel] [PATCH] AMD, powernow: Update P-state directly when _PSD's CoordType is DOMAIN_COORD_TYPE_HW_ALL

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.12 at 18:41, Boris Ostrovsky <boris.ostrovsky@xxxxxxx> wrote:
> AMD, powernow: Update P-state directly when _PSD's CoordType is 
> DOMAIN_COORD_TYPE_HW_ALL
> 
> When _PSD's CoordType is DOMAIN_COORD_TYPE_HW_ALL (i.e. shared_type is
> CPUFREQ_SHARED_TYPE_HW) which most often is the case on servers, there is no
> reason to go into on_selected_cpus() code, we call call transition_pstate()
> directly.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxx>

Looks good to me in general (one minor comment below, but no
need to resubmit just because of this). But it's not really clear
to me whether this actually improves anything (knowing of which
is needed to decide whether to put this in now or after 4.2).

> --- a/xen/arch/x86/acpi/cpufreq/powernow.c    Wed Aug 15 09:41:21 2012 +0100
> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c    Thu Aug 16 18:38:21 2012 +0200
> @@ -56,20 +56,9 @@
>  
>  static struct cpufreq_driver powernow_cpufreq_driver;
>  
> -struct drv_cmd {
> -    unsigned int type;
> -    const cpumask_t *mask;
> -    u32 val;
> -    int turbo;
> -};
> -
> -static void transition_pstate(void *drvcmd)
> +static void transition_pstate(void *pstate)
>  {
> -    struct drv_cmd *cmd;
> -    cmd = (struct drv_cmd *) drvcmd;
> -
> -
> -    wrmsrl(MSR_PSTATE_CTRL, cmd->val);
> +    wrmsrl(MSR_PSTATE_CTRL, *(int *)pstate);

The variable a pointer to which gets passed in for this function
is "unsigned int", so you surely would need to cast to that type
instead of plain "int".

Jan

>  }
>  
>  static void update_cpb(void *data)
> @@ -106,13 +95,11 @@ static int powernow_cpufreq_target(struc
>  {
>      struct acpi_cpufreq_data *data = cpufreq_drv_data[policy->cpu];
>      struct processor_performance *perf;
> -    struct cpufreq_freqs freqs;
>      cpumask_t online_policy_cpus;
> -    struct drv_cmd cmd;
> -    unsigned int next_state = 0; /* Index into freq_table */
> -    unsigned int next_perf_state = 0; /* Index into perf table */
> -    int result = 0;
> -    int j = 0;
> +    unsigned int next_state; /* Index into freq_table */
> +    unsigned int next_perf_state; /* Index into perf table */
> +    int result;
> +    int j;
>  
>      if (unlikely(data == NULL ||
>          data->acpi_data == NULL || data->freq_table == NULL)) {
> @@ -125,9 +112,7 @@ static int powernow_cpufreq_target(struc
>                                              target_freq,
>                                              relation, &next_state);
>      if (unlikely(result))
> -        return -ENODEV;
> -
> -    cpumask_and(&online_policy_cpus, policy->cpus, &cpu_online_map);
> +        return result;
>  
>      next_perf_state = data->freq_table[next_state].index;
>      if (perf->state == next_perf_state) {
> @@ -137,26 +122,28 @@ static int powernow_cpufreq_target(struc
>              return 0;
>      }
>  
> -    if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
> -        cmd.mask = &online_policy_cpus;
> -    else
> -        cmd.mask = cpumask_of(policy->cpu);
> +    if (policy->shared_type == CPUFREQ_SHARED_TYPE_HW &&
> +        likely(policy->cpu == smp_processor_id())) {
> +        transition_pstate(&next_perf_state);
> +        cpufreq_statistic_update(policy->cpu, perf->state, next_perf_state);
> +    } else {
> +        cpumask_and(&online_policy_cpus, policy->cpus, &cpu_online_map);
>  
> -    freqs.old = perf->states[perf->state].core_frequency * 1000;
> -    freqs.new = data->freq_table[next_state].frequency;
> +        if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL ||
> +            unlikely(policy->cpu != smp_processor_id()))
> +            on_selected_cpus(&online_policy_cpus, transition_pstate,
> +                             &next_perf_state, 1);
> +        else
> +            transition_pstate(&next_perf_state);
>  
> -    cmd.val = next_perf_state;
> -    cmd.turbo = policy->turbo;
> -
> -    on_selected_cpus(cmd.mask, transition_pstate, &cmd, 1);
> -
> -    for_each_cpu(j, &online_policy_cpus)
> -        cpufreq_statistic_update(j, perf->state, next_perf_state);
> +        for_each_cpu(j, &online_policy_cpus)
> +            cpufreq_statistic_update(j, perf->state, next_perf_state);
> +    }    
>  
>      perf->state = next_perf_state;
> -    policy->cur = freqs.new;
> +    policy->cur = data->freq_table[next_state].frequency;
>  
> -    return result;
> +    return 0;
>  }
>  
>  static int powernow_cpufreq_verify(struct cpufreq_policy *policy)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel