From patchwork Tue Sep 9 04:16:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 37021 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-vc0-f199.google.com (mail-vc0-f199.google.com [209.85.220.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 329F22054D for ; Tue, 9 Sep 2014 04:17:03 +0000 (UTC) Received: by mail-vc0-f199.google.com with SMTP id id10sf45405929vcb.10 for ; Mon, 08 Sep 2014 21:17:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:cc:subject:date:message-id :in-reply-to:references:in-reply-to:references:mime-version:sender :precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=1ZXloaFoPk9kALfuC7GNvfs9yhkZ+nvGjO4QkK57GyQ=; b=b0KIaRJvYblzUo6qchP+S2LBQdhKdN6vJjYXamg6IgsJsFqqaZzJ1WEqaMxKpQFROF OOLz1VWw+NrCoORovYd+PeVnnsAEQYXL0BqfasldUsrqoAs3eW7qSdvb6QQI7t8tRk5X /n7fpwnfLY6IxRCTD4lfBiTOtCj0lCLaA1c0E7dls1+LJ+Pl8oZoZmLQAH+k1lrpT5Pu EDvxGIIBGp+E29S1GX5nGKEJ3TX2yng4k5s79v17/Dyt7FGwYTNqKtufzULjd+226esq miFizWv4E7giRIdkJuLEOqsjv7gGNJ8n8HdeR9LlJR2hIt7Swf6pDuu2VwsCcnxTWKWb FSVw== X-Gm-Message-State: ALoCoQkb3kL7DYAhEIusEMMGHFncH5MhOQQhGIHU4OVPJ3NZHnHpnwvMGxt1TsL4wEmwyMla69kl X-Received: by 10.236.207.164 with SMTP id n24mr22439767yho.5.1410236222963; Mon, 08 Sep 2014 21:17:02 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.49.41 with SMTP id p38ls1750008qga.55.gmail; Mon, 08 Sep 2014 21:17:02 -0700 (PDT) X-Received: by 10.220.95.132 with SMTP id d4mr13294778vcn.33.1410236222884; Mon, 08 Sep 2014 21:17:02 -0700 (PDT) Received: from mail-vc0-f181.google.com (mail-vc0-f181.google.com [209.85.220.181]) by mx.google.com with ESMTPS id f2si5384227vdf.74.2014.09.08.21.17.02 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 08 Sep 2014 21:17:02 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.181 as permitted sender) client-ip=209.85.220.181; Received: by mail-vc0-f181.google.com with SMTP id ij19so2712618vcb.40 for ; Mon, 08 Sep 2014 21:17:02 -0700 (PDT) X-Received: by 10.220.186.196 with SMTP id ct4mr4401vcb.51.1410236222765; Mon, 08 Sep 2014 21:17:02 -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.221.45.67 with SMTP id uj3csp226821vcb; Mon, 8 Sep 2014 21:17:02 -0700 (PDT) X-Received: by 10.70.128.105 with SMTP id nn9mr54365854pdb.23.1410236221810; Mon, 08 Sep 2014 21:17:01 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id sn9si20943362pac.108.2014.09.08.21.17.00 for ; Mon, 08 Sep 2014 21:17:01 -0700 (PDT) Received-SPF: none (google.com: linux-pm-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751497AbaIIERA (ORCPT + 15 others); Tue, 9 Sep 2014 00:17:00 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:62588 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750879AbaIIEQ7 (ORCPT ); Tue, 9 Sep 2014 00:16:59 -0400 Received: by mail-pa0-f45.google.com with SMTP id rd3so3889900pab.4 for ; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-Received: by 10.70.43.201 with SMTP id y9mr53534878pdl.111.1410236219408; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) Received: from localhost ([122.167.129.166]) by mx.google.com with ESMTPSA id y1sm10318202pdk.90.2014.09.08.21.16.58 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 08 Sep 2014 21:16:58 -0700 (PDT) From: Viresh Kumar To: Rafael Wysocki Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, robert.schoene@tu-dresden.de, prarit@redhat.com, skannan@codeaurora.org, Viresh Kumar Subject: [PATCH 2/2] cpufreq: Track governor-state with 'policy->governor_state' Date: Tue, 9 Sep 2014 09:46:40 +0530 Message-Id: <0aa5f4741d082889309313ff3d9c70b39ddcb48d.1410235439.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.0.3.693.g996b0fd In-Reply-To: References: In-Reply-To: References: MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-pm@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=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.181 as permitted sender) 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: , Even after serializing calls to __cpufreq_governor() there are some races left. The races are around doing the invalid operation during some state of cpufreq governors. For example, while the governor is in CPUFREQ_GOV_POLICY_EXIT state, we can't do CPUFREQ_GOV_START without first doing CPUFREQ_GOV_POLICY_INIT. All these cases weren't handled elegantly in __cpufreq_governor() and so there were enough chances that things may go wrong when governors are changed with multiple thread in parallel. This patch renames an existing field 'policy->governor_enabled' as 'policy->governor_state' which can have values other than 0 & 1 now. Its type is also changed to 'int' from 'bool'. We maintain the current state of governors for each policy now and reject any invalid operation. Reported-and-tested-by: Robert Schöne Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 26 +++++++++++++------------- drivers/cpufreq/cpufreq_governor.c | 2 +- include/linux/cpufreq.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index a7ceae3..c597361 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -935,6 +935,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy) struct cpufreq_policy new_policy; int ret = 0; + policy->governor_state = CPUFREQ_GOV_POLICY_EXIT; memcpy(&new_policy, policy, sizeof(*policy)); /* Update governor of new_policy to the governor used before hotplug */ @@ -1976,7 +1977,7 @@ EXPORT_SYMBOL_GPL(cpufreq_driver_target); static int __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event) { - int ret; + int ret, state; /* Only must be defined when default governor is known to have latency restrictions, like e.g. conservative or ondemand. @@ -2012,19 +2013,21 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, policy->cpu, event); mutex_lock(&cpufreq_governor_lock); + state = policy->governor_state; + + /* Check if operation is permitted or not */ if (policy->governor_busy - || (policy->governor_enabled && event == CPUFREQ_GOV_START) - || (!policy->governor_enabled - && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) { + || (state == CPUFREQ_GOV_START && event != CPUFREQ_GOV_LIMITS && event != CPUFREQ_GOV_STOP) + || (state == CPUFREQ_GOV_STOP && event != CPUFREQ_GOV_START && event != CPUFREQ_GOV_POLICY_EXIT) + || (state == CPUFREQ_GOV_POLICY_INIT && event != CPUFREQ_GOV_START && event != CPUFREQ_GOV_POLICY_EXIT) + || (state == CPUFREQ_GOV_POLICY_EXIT && event != CPUFREQ_GOV_POLICY_INIT)) { mutex_unlock(&cpufreq_governor_lock); return -EBUSY; } policy->governor_busy = true; - if (event == CPUFREQ_GOV_STOP) - policy->governor_enabled = false; - else if (event == CPUFREQ_GOV_START) - policy->governor_enabled = true; + if (event != CPUFREQ_GOV_LIMITS) + policy->governor_state = event; mutex_unlock(&cpufreq_governor_lock); @@ -2035,13 +2038,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, policy->governor->initialized++; else if (event == CPUFREQ_GOV_POLICY_EXIT) policy->governor->initialized--; - } else { + } else if (event != CPUFREQ_GOV_LIMITS) { /* Restore original values */ mutex_lock(&cpufreq_governor_lock); - if (event == CPUFREQ_GOV_STOP) - policy->governor_enabled = true; - else if (event == CPUFREQ_GOV_START) - policy->governor_enabled = false; + policy->governor_state = state; mutex_unlock(&cpufreq_governor_lock); } diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 1b44496..d173181 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -174,7 +174,7 @@ void gov_queue_work(struct dbs_data *dbs_data, struct cpufreq_policy *policy, int i; mutex_lock(&cpufreq_governor_lock); - if (!policy->governor_enabled) + if (policy->governor_state != CPUFREQ_GOV_START) goto out_unlock; if (!all_cpus) { diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index c7aa96b..fb20fc5 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -81,7 +81,7 @@ struct cpufreq_policy { unsigned int policy; /* see above */ struct cpufreq_governor *governor; /* see below */ void *governor_data; - bool governor_enabled; /* governor start/stop flag */ + int governor_state; /* Governor's current state */ bool governor_busy; struct work_struct update; /* if update_policy() needs to be