From patchwork Mon Mar 24 08:05:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 26919 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ve0-f200.google.com (mail-ve0-f200.google.com [209.85.128.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 969E6202E7 for ; Mon, 24 Mar 2014 08:06:28 +0000 (UTC) Received: by mail-ve0-f200.google.com with SMTP id oy12sf13542944veb.3 for ; Mon, 24 Mar 2014 01:06:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:in-reply-to:references:in-reply-to:references :sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=lDVEf2xm8CqmwogmrEmO/SHLWaPrgpi2FkRlAY0W9lg=; b=jHukgaqi1lxMjJD2ciXaxYzUhddl4Y49Sk2tnGRuBAXaheQfCaqiPbFx4weTlhjeyr FStqhh9WYvvV5Wpm5v1MzA9bkOqMf8gk/e32J71b93Nbd3+brwEgxxFS9w/mVmasjvYe k1eK1cflYdjeFnhZhLGu4J0NWTsvqVbqK2v1MEx9aDbN3LKg5swXCyI3diKFU+X57rvY lanQ5JEO+7Jh6nJBo8tL8LXcHydSmQmLgi3Sf1IOdbYMM7enA2GigeNYIu0GWIaSWPpW ioyhpiZmXhCitlnelTbMsbC23HIZGAi/LSS0BP/GbDPfn2l+CPr/PbTDr65uHkepReZ3 aaiw== X-Gm-Message-State: ALoCoQnPKQFuAMONAFCmHK7dtElj4tYPnOuipxl+h3WX4UxdK+pIuU/yvboPOfhgso0iY+5Xd3iD X-Received: by 10.52.2.129 with SMTP id 1mr12552603vdu.4.1395648388161; Mon, 24 Mar 2014 01:06:28 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.47.239 with SMTP id m102ls1450604qga.8.gmail; Mon, 24 Mar 2014 01:06:28 -0700 (PDT) X-Received: by 10.52.108.228 with SMTP id hn4mr29271vdb.43.1395648388004; Mon, 24 Mar 2014 01:06:28 -0700 (PDT) Received: from mail-vc0-f179.google.com (mail-vc0-f179.google.com [209.85.220.179]) by mx.google.com with ESMTPS id w5si2837523vcn.134.2014.03.24.01.06.27 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 24 Mar 2014 01:06:27 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.179; Received: by mail-vc0-f179.google.com with SMTP id ij19so5308976vcb.38 for ; Mon, 24 Mar 2014 01:06:27 -0700 (PDT) X-Received: by 10.220.10.2 with SMTP id n2mr242706vcn.26.1395648387914; Mon, 24 Mar 2014 01:06:27 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.78.9 with SMTP id i9csp204622vck; Mon, 24 Mar 2014 01:06:27 -0700 (PDT) X-Received: by 10.66.148.230 with SMTP id tv6mr1420209pab.155.1395648386621; Mon, 24 Mar 2014 01:06:26 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id zh8si8753478pac.113.2014.03.24.01.06.25; Mon, 24 Mar 2014 01:06:25 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750872AbaCXIGI (ORCPT + 26 others); Mon, 24 Mar 2014 04:06:08 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:46306 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752221AbaCXIGC (ORCPT ); Mon, 24 Mar 2014 04:06:02 -0400 Received: by mail-wi0-f180.google.com with SMTP id hr14so647185wib.1 for ; Mon, 24 Mar 2014 01:06:00 -0700 (PDT) X-Received: by 10.180.210.171 with SMTP id mv11mr13123227wic.44.1395648360841; Mon, 24 Mar 2014 01:06:00 -0700 (PDT) Received: from localhost ([213.122.173.131]) by mx.google.com with ESMTPSA id u6sm34881689wif.6.2014.03.24.01.05.57 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 24 Mar 2014 01:05:59 -0700 (PDT) From: Viresh Kumar To: rjw@rjwysocki.net Cc: linaro-kernel@lists.linaro.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, srivatsa.bhat@linux.vnet.ibm.com, Viresh Kumar Subject: [PATCH V5 1/3] cpufreq: Make sure frequency transitions are serialized Date: Mon, 24 Mar 2014 13:35:44 +0530 Message-Id: X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e In-Reply-To: References: In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , From: "Srivatsa S. Bhat" Whenever we change the frequency of a CPU, we call the PRECHANGE and POSTCHANGE notifiers. They must be serialized, i.e. PRECHANGE and POSTCHANGE notifiers should strictly alternate, thereby preventing two different sets of PRECHANGE or POSTCHANGE notifiers from interleaving arbitrarily. The following examples illustrate why this is important: Scenario 1: ----------- A thread reading the value of cpuinfo_cur_freq, will call __cpufreq_cpu_get()->cpufreq_out_of_sync()->cpufreq_notify_transition() The ondemand governor can decide to change the frequency of the CPU at the same time and hence it can end up sending the notifications via ->target(). If the notifiers are not serialized, the following sequence can occur: - PRECHANGE Notification for freq A (from cpuinfo_cur_freq) - PRECHANGE Notification for freq B (from target()) - Freq changed by target() to B - POSTCHANGE Notification for freq B - POSTCHANGE Notification for freq A We can see from the above that the last POSTCHANGE Notification happens for freq A but the hardware is set to run at freq B. Where would we break then?: adjust_jiffies() in cpufreq.c & cpufreq_callback() in arch/arm/kernel/smp.c (which also adjusts the jiffies). All the loops_per_jiffy calculations will get messed up. Scenario 2: ----------- The governor calls __cpufreq_driver_target() to change the frequency. At the same time, if we change scaling_{min|max}_freq from sysfs, it will end up calling the governor's CPUFREQ_GOV_LIMITS notification, which will also call __cpufreq_driver_target(). And hence we end up issuing concurrent calls to ->target(). Typically, platforms have the following logic in their ->target() routines: (Eg: cpufreq-cpu0, omap, exynos, etc) A. If new freq is more than old: Increase voltage B. Change freq C. If new freq is less than old: decrease voltage Now, if the two concurrent calls to ->target() are X and Y, where X is trying to increase the freq and Y is trying to decrease it, we get the following race condition: X.A: voltage gets increased for larger freq Y.A: nothing happens Y.B: freq gets decreased Y.C: voltage gets decreased X.B: freq gets increased X.C: nothing happens Thus we can end up setting a freq which is not supported by the voltage we have set. That will probably make the clock to the CPU unstable and the system might not work properly anymore. This patch introduces a set of synchronization primitives to serialize frequency transitions, which are to be used as shown below: cpufreq_freq_transition_begin(); //Perform the frequency change cpufreq_freq_transition_end(); The _begin() call sends the PRECHANGE notification whereas the _end() call sends the POSTCHANGE notification. Also, all the necessary synchronization is handled within these calls. In particular, even drivers which set the ASYNC_NOTIFICATION flag can also use these APIs for performing frequency transitions (ie., you can call _begin() from one task, and call the corresponding _end() from a different task). The actual synchronization underneath is not that complicated: The key challenge is to allow drivers to begin the transition from one thread and end it in a completely different thread (this is to enable drivers that do asynchronous POSTCHANGE notification from bottom-halves, to also use the same interface). To achieve this, a 'transition_ongoing' flag, a 'transition_lock' spinlock and a wait-queue are added per-policy. The flag and the wait-queue are used in conjunction to create an "uninterrupted flow" from _begin() to _end(). The spinlock is used to ensure that only one such "flow" is in flight at any given time. Put together, this provides us all the necessary synchronization. Based-on-patch-by: Viresh Kumar Signed-off-by: Srivatsa S. Bhat Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 37 +++++++++++++++++++++++++++++++++++++ include/linux/cpufreq.h | 10 ++++++++++ 2 files changed, 47 insertions(+) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index d8d6bc9..d57806a 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -353,6 +353,41 @@ void cpufreq_notify_post_transition(struct cpufreq_policy *policy, } EXPORT_SYMBOL_GPL(cpufreq_notify_post_transition); +void cpufreq_freq_transition_begin(struct cpufreq_policy *policy, + struct cpufreq_freqs *freqs) +{ +wait: + wait_event(policy->transition_wait, !policy->transition_ongoing); + + spin_lock(&policy->transition_lock); + + if (unlikely(policy->transition_ongoing)) { + spin_unlock(&policy->transition_lock); + goto wait; + } + + policy->transition_ongoing = true; + + spin_unlock(&policy->transition_lock); + + cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE); +} +EXPORT_SYMBOL_GPL(cpufreq_freq_transition_begin); + +void cpufreq_freq_transition_end(struct cpufreq_policy *policy, + struct cpufreq_freqs *freqs, int transition_failed) +{ + if (unlikely(WARN_ON(!policy->transition_ongoing))) + return; + + cpufreq_notify_post_transition(policy, freqs, transition_failed); + + policy->transition_ongoing = false; + + wake_up(&policy->transition_wait); +} +EXPORT_SYMBOL_GPL(cpufreq_freq_transition_end); + /********************************************************************* * SYSFS INTERFACE * @@ -985,6 +1020,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void) INIT_LIST_HEAD(&policy->policy_list); init_rwsem(&policy->rwsem); + spin_lock_init(&policy->transition_lock); + init_waitqueue_head(&policy->transition_wait); return policy; diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 2d2e62c..e337602 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -16,6 +16,7 @@ #include #include #include +#include #include /********************************************************************* @@ -104,6 +105,11 @@ struct cpufreq_policy { * __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT); */ struct rw_semaphore rwsem; + + /* Synchronization for frequency transitions */ + bool transition_ongoing; /* Tracks transition status */ + spinlock_t transition_lock; + wait_queue_head_t transition_wait; }; /* Only for ACPI */ @@ -337,6 +343,10 @@ void cpufreq_notify_transition(struct cpufreq_policy *policy, struct cpufreq_freqs *freqs, unsigned int state); void cpufreq_notify_post_transition(struct cpufreq_policy *policy, struct cpufreq_freqs *freqs, int transition_failed); +void cpufreq_freq_transition_begin(struct cpufreq_policy *policy, + struct cpufreq_freqs *freqs); +void cpufreq_freq_transition_end(struct cpufreq_policy *policy, + struct cpufreq_freqs *freqs, int transition_failed); #else /* CONFIG_CPU_FREQ */ static inline int cpufreq_register_notifier(struct notifier_block *nb,