[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 08/17/2012 06:19 AM, Jan Beulich wrote:
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).

I didn't see any noticeable improvement but then I may not have run the right tests.

I imagine this may be helpful, for example, in webserver-type environments with frequent P-state transitions when new connections are requested and short-lived threads/processes are created. But I haven't been able to set this up to observe this.

I think post-4.2 is fine.

-boris


--- 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.