* [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups @ 2016-12-30 14:54 Rafael J. Wysocki 2016-12-30 14:56 ` [PATCH 1/3] cpufreq: intel_pstate: Use locking in intel_pstate_resume() Rafael J. Wysocki ` (3 more replies) 0 siblings, 4 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2016-12-30 14:54 UTC (permalink / raw) To: Linux PM, Srinivas Pandruvada; +Cc: LKML Hi, This series fixes a couple of possible locking issues and limits synchronization in intel pstate. [1/3] Add locking to intel_pstate_resume(). [2/3] Add locking to intel_cpufreq_verify_policy(). [3/3] Always keep all limits settings in sync. Thanks, Rafael ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] cpufreq: intel_pstate: Use locking in intel_pstate_resume() 2016-12-30 14:54 [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Rafael J. Wysocki @ 2016-12-30 14:56 ` Rafael J. Wysocki 2016-12-30 14:57 ` [PATCH 2/3] cpufreq: intel_pstate: Use locking in intel_cpufreq_verify_policy() Rafael J. Wysocki ` (2 subsequent siblings) 3 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2016-12-30 14:56 UTC (permalink / raw) To: Linux PM; +Cc: Srinivas Pandruvada, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Theoretically, intel_pstate_resume() may be executed in parallel with intel_pstate_set_policy(), if the latter is invoked via cpufreq_update_policy() as a result of a notification, so use intel_pstate_limits_lock in there too to avoid race conditions. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -968,12 +968,20 @@ static int intel_pstate_hwp_save_state(s static int intel_pstate_resume(struct cpufreq_policy *policy) { + int ret; + if (!hwp_active) return 0; + mutex_lock(&intel_pstate_limits_lock); + all_cpu_data[policy->cpu]->epp_policy = 0; - return intel_pstate_hwp_set_policy(policy); + ret = intel_pstate_hwp_set_policy(policy); + + mutex_unlock(&intel_pstate_limits_lock); + + return ret; } static void intel_pstate_hwp_set_online_cpus(void) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] cpufreq: intel_pstate: Use locking in intel_cpufreq_verify_policy() 2016-12-30 14:54 [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Rafael J. Wysocki 2016-12-30 14:56 ` [PATCH 1/3] cpufreq: intel_pstate: Use locking in intel_pstate_resume() Rafael J. Wysocki @ 2016-12-30 14:57 ` Rafael J. Wysocki 2016-12-30 14:58 ` [PATCH 3/3] cpufreq: intel_pstate: Always keep all limits settings in sync Rafael J. Wysocki 2016-12-30 17:49 ` [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Srinivas Pandruvada 3 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2016-12-30 14:57 UTC (permalink / raw) To: Linux PM; +Cc: Srinivas Pandruvada, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Race conditions are possible if intel_cpufreq_verify_policy() is executed in parallel with global limits updates from sysfs, so the invocation of intel_pstate_update_perf_limits() in it should be carried out under intel_pstate_limits_lock. Make that happen. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 4 ++++ 1 file changed, 4 insertions(+) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -2157,8 +2157,12 @@ static int intel_cpufreq_verify_policy(s if (per_cpu_limits) perf_limits = cpu->perf_limits; + mutex_lock(&intel_pstate_limits_lock); + intel_pstate_update_perf_limits(policy, perf_limits); + mutex_unlock(&intel_pstate_limits_lock); + return 0; } ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] cpufreq: intel_pstate: Always keep all limits settings in sync 2016-12-30 14:54 [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Rafael J. Wysocki 2016-12-30 14:56 ` [PATCH 1/3] cpufreq: intel_pstate: Use locking in intel_pstate_resume() Rafael J. Wysocki 2016-12-30 14:57 ` [PATCH 2/3] cpufreq: intel_pstate: Use locking in intel_cpufreq_verify_policy() Rafael J. Wysocki @ 2016-12-30 14:58 ` Rafael J. Wysocki 2016-12-30 17:49 ` [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Srinivas Pandruvada 3 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2016-12-30 14:58 UTC (permalink / raw) To: Linux PM; +Cc: Srinivas Pandruvada, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Make intel_pstate update per-logical-CPU limits when the global settings are changed to ensure that they are always in sync and users will not see confusing values in per-logical-CPU sysfs attributes. This also fixes the problem that setting the "no_turbo" global attribute to 1 in the "passive" mode (ie. when intel_pstate acts as a regular cpufreq driver) when scaling_governor is set to "performance" has no effect. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -857,13 +857,13 @@ static struct freq_attr *hwp_cpufreq_att NULL, }; -static void intel_pstate_hwp_set(const struct cpumask *cpumask) +static void intel_pstate_hwp_set(struct cpufreq_policy *policy) { int min, hw_min, max, hw_max, cpu, range, adj_range; struct perf_limits *perf_limits = limits; u64 value, cap; - for_each_cpu(cpu, cpumask) { + for_each_cpu(cpu, policy->cpus) { int max_perf_pct, min_perf_pct; struct cpudata *cpu_data = all_cpu_data[cpu]; s16 epp; @@ -949,7 +949,7 @@ skip_epp: static int intel_pstate_hwp_set_policy(struct cpufreq_policy *policy) { if (hwp_active) - intel_pstate_hwp_set(policy->cpus); + intel_pstate_hwp_set(policy); return 0; } @@ -984,11 +984,12 @@ static int intel_pstate_resume(struct cp return ret; } -static void intel_pstate_hwp_set_online_cpus(void) +static void intel_pstate_update_policies(void) { - get_online_cpus(); - intel_pstate_hwp_set(cpu_online_mask); - put_online_cpus(); + int cpu; + + for_each_possible_cpu(cpu) + cpufreq_update_policy(cpu); } /************************** debugfs begin ************************/ @@ -1109,11 +1110,10 @@ static ssize_t store_no_turbo(struct kob limits->no_turbo = clamp_t(int, input, 0, 1); - if (hwp_active) - intel_pstate_hwp_set_online_cpus(); - mutex_unlock(&intel_pstate_limits_lock); + intel_pstate_update_policies(); + return count; } @@ -1138,11 +1138,10 @@ static ssize_t store_max_perf_pct(struct limits->max_perf_pct); limits->max_perf = div_ext_fp(limits->max_perf_pct, 100); - if (hwp_active) - intel_pstate_hwp_set_online_cpus(); - mutex_unlock(&intel_pstate_limits_lock); + intel_pstate_update_policies(); + return count; } @@ -1167,11 +1166,10 @@ static ssize_t store_min_perf_pct(struct limits->min_perf_pct); limits->min_perf = div_ext_fp(limits->min_perf_pct, 100); - if (hwp_active) - intel_pstate_hwp_set_online_cpus(); - mutex_unlock(&intel_pstate_limits_lock); + intel_pstate_update_policies(); + return count; } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups 2016-12-30 14:54 [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Rafael J. Wysocki ` (2 preceding siblings ...) 2016-12-30 14:58 ` [PATCH 3/3] cpufreq: intel_pstate: Always keep all limits settings in sync Rafael J. Wysocki @ 2016-12-30 17:49 ` Srinivas Pandruvada 3 siblings, 0 replies; 5+ messages in thread From: Srinivas Pandruvada @ 2016-12-30 17:49 UTC (permalink / raw) To: Rafael J. Wysocki, Linux PM; +Cc: LKML On Fri, 2016-12-30 at 15:54 +0100, Rafael J. Wysocki wrote: > Hi, > > This series fixes a couple of possible locking issues and limits > synchronization > in intel pstate. > > [1/3] Add locking to intel_pstate_resume(). > [2/3] Add locking to intel_cpufreq_verify_policy(). > [3/3] Always keep all limits settings in sync. > Looks good to me. Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Thanks, Srinivas ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-30 17:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-30 14:54 [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Rafael J. Wysocki 2016-12-30 14:56 ` [PATCH 1/3] cpufreq: intel_pstate: Use locking in intel_pstate_resume() Rafael J. Wysocki 2016-12-30 14:57 ` [PATCH 2/3] cpufreq: intel_pstate: Use locking in intel_cpufreq_verify_policy() Rafael J. Wysocki 2016-12-30 14:58 ` [PATCH 3/3] cpufreq: intel_pstate: Always keep all limits settings in sync Rafael J. Wysocki 2016-12-30 17:49 ` [PATCH 0/3] cpufreq: intel_pstate: Locking and limits fix-ups Srinivas Pandruvada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).