From patchwork Thu Jun 11 10:51:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 49746 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f71.google.com (mail-wg0-f71.google.com [74.125.82.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 512CF20C81 for ; Thu, 11 Jun 2015 10:53:03 +0000 (UTC) Received: by wgla2 with SMTP id a2sf954822wgl.1 for ; Thu, 11 Jun 2015 03:53: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: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=95ud3uoY5rrlcS5hAcshNyoA8viZTUfplZwmy2y+NVE=; b=W0+5VI6qzwApDxBk1csBK6rgJ7OayXQzdlr3TbCoPefVFs9q4H5Qq0eCSlHTh9itlz 0A5jvXl9jBlQWcQGWY286i7jNn/DbxlyLq7CuebgHpnh73q7avhh7slgeKJ7fthaN3Bp SW6f/fZbn3tsjZ3pkMmb/VEa2TG5gQMZaUyhQyWyAwMwKSoRsuk3DQRE1OHyw3Ywi3uC j8zQ0VlU8iCv1tLo8b3JAjcNlxdWGq6E6gA8aQys61Q0UO5mS0XCpFokuKzZKQ2pC5iQ ObwsYwoKdoPTTT1jgpgZI8U5PhJp2lId3nWRGlNj+XGw6OLsWLZEwF+7qVtY40ObS3qQ D1Bw== X-Gm-Message-State: ALoCoQnHps2AfZx5Ag7g48yaj0QSeJDh9RTTYGNvnmoT6Z1o+eyiut48+7PSRKyb3BkvE93Jhr0B X-Received: by 10.180.187.175 with SMTP id ft15mr26910930wic.4.1434019982597; Thu, 11 Jun 2015 03:53:02 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.87.171 with SMTP id az11ls395554lab.107.gmail; Thu, 11 Jun 2015 03:53:02 -0700 (PDT) X-Received: by 10.152.204.7 with SMTP id ku7mr9184757lac.38.1434019982427; Thu, 11 Jun 2015 03:53:02 -0700 (PDT) Received: from mail-la0-f53.google.com (mail-la0-f53.google.com. [209.85.215.53]) by mx.google.com with ESMTPS id k11si263466lbz.74.2015.06.11.03.53.02 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Jun 2015 03:53:02 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.53 as permitted sender) client-ip=209.85.215.53; Received: by laew7 with SMTP id w7so2419020lae.1 for ; Thu, 11 Jun 2015 03:53:02 -0700 (PDT) X-Received: by 10.112.93.37 with SMTP id cr5mr9284654lbb.106.1434019982295; Thu, 11 Jun 2015 03:53: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.112.108.230 with SMTP id hn6csp3952659lbb; Thu, 11 Jun 2015 03:53:01 -0700 (PDT) X-Received: by 10.70.89.199 with SMTP id bq7mr13675561pdb.168.1434019970451; Thu, 11 Jun 2015 03:52:50 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id xz3si375664pab.181.2015.06.11.03.52.49; Thu, 11 Jun 2015 03:52:50 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-pm-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 S1752717AbbFKKws (ORCPT + 11 others); Thu, 11 Jun 2015 06:52:48 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:32872 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752680AbbFKKws (ORCPT ); Thu, 11 Jun 2015 06:52:48 -0400 Received: by padev16 with SMTP id ev16so2330755pad.0 for ; Thu, 11 Jun 2015 03:52:48 -0700 (PDT) X-Received: by 10.70.41.78 with SMTP id d14mr13954703pdl.35.1434019968011; Thu, 11 Jun 2015 03:52:48 -0700 (PDT) Received: from localhost ([122.167.219.251]) by mx.google.com with ESMTPSA id pw9sm387059pac.27.2015.06.11.03.52.46 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 11 Jun 2015 03:52:47 -0700 (PDT) From: Viresh Kumar To: Rafael Wysocki , Preeti U Murthy , ke.wang@spreadtrum.com Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, ego@linux.vnet.ibm.com, paulus@samba.org, shilpa.bhat@linux.vnet.ibm.com, prarit@redhat.com, robert.schoene@tu-dresden.de, skannan@codeaurora.org, Viresh Kumar Subject: [PATCH 09/12] cpufreq: governor: Avoid invalid states with additional checks Date: Thu, 11 Jun 2015 16:21:52 +0530 Message-Id: <280186b68a8b906365316876a7b6d9eafb28296b.1434019473.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.4.0 In-Reply-To: References: In-Reply-To: References: 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.215.53 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: , There can be races where the request has come to a wrong state. For example INIT followed by STOP (instead of START) or START followed by EXIT (instead of STOP). Also make sure, once we have started canceling queued works, we don't queue any new works. That can lead to the case where the work-handler finds many data structures are freed and so NULL pointer exceptions. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq_governor.c | 56 ++++++++++++++++++++++++++++++-------- drivers/cpufreq/cpufreq_governor.h | 1 + 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index aa24aa9a9eb3..ee2e19a1218a 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -169,8 +169,12 @@ static inline void __gov_queue_work(int cpu, struct dbs_data *dbs_data, void gov_queue_work(struct dbs_data *dbs_data, struct cpufreq_policy *policy, unsigned int delay, bool all_cpus) { + struct cpu_dbs_info *cdbs = dbs_data->cdata->get_cpu_cdbs(policy->cpu); int i; + if (!cdbs->ccdbs->enabled) + return; + mutex_lock(&cpufreq_governor_lock); if (!policy->governor_enabled) goto out_unlock; @@ -234,6 +238,8 @@ static void dbs_timer(struct work_struct *work) bool modify_all = true; mutex_lock(&dbs_data->cdata->mutex); + if (!cdbs->ccdbs->enabled) + goto unlock; if (dbs_data->cdata->governor == GOV_CONSERVATIVE) { struct cs_dbs_tuners *cs_tuners = dbs_data->tuners; @@ -251,6 +257,7 @@ static void dbs_timer(struct work_struct *work) delay = dbs_data->cdata->gov_dbs_timer(cdbs, dbs_data, modify_all); gov_queue_work(dbs_data, policy, delay, modify_all); +unlock: mutex_unlock(&dbs_data->cdata->mutex); } @@ -376,10 +383,15 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy, return ret; } -static void cpufreq_governor_exit(struct cpufreq_policy *policy, - struct dbs_data *dbs_data) +static int cpufreq_governor_exit(struct cpufreq_policy *policy, + struct dbs_data *dbs_data) { struct common_dbs_data *cdata = dbs_data->cdata; + struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(policy->cpu); + + /* STOP should have been called by now */ + if (cdbs->ccdbs->enabled) + return -EBUSY; policy->governor_data = NULL; if (!--dbs_data->usage_count) { @@ -395,6 +407,8 @@ static void cpufreq_governor_exit(struct cpufreq_policy *policy, free_ccdbs(policy, cdata); kfree(dbs_data); } + + return 0; } static int cpufreq_governor_start(struct cpufreq_policy *policy, @@ -409,6 +423,10 @@ static int cpufreq_governor_start(struct cpufreq_policy *policy, if (!policy->cur) return -EINVAL; + /* START shouldn't be already called */ + if (ccdbs->enabled) + return -EBUSY; + if (cdata->governor == GOV_CONSERVATIVE) { struct cs_dbs_tuners *cs_tuners = dbs_data->tuners; @@ -458,17 +476,25 @@ static int cpufreq_governor_start(struct cpufreq_policy *policy, od_ops->powersave_bias_init_cpu(cpu); } + ccdbs->enabled = true; gov_queue_work(dbs_data, policy, delay_for_sampling_rate(sampling_rate), true); return 0; } -static void cpufreq_governor_stop(struct cpufreq_policy *policy, - struct dbs_data *dbs_data) +static int cpufreq_governor_stop(struct cpufreq_policy *policy, + struct dbs_data *dbs_data) { struct common_dbs_data *cdata = dbs_data->cdata; unsigned int cpu = policy->cpu; + struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu); + struct cpu_common_dbs_info *ccdbs = cdbs->ccdbs; + + /* Shouldn't be already stopped */ + if (!ccdbs->enabled) + return -EBUSY; + ccdbs->enabled = false; gov_cancel_work(dbs_data, policy); if (cdata->governor == GOV_CONSERVATIVE) { @@ -477,17 +503,19 @@ static void cpufreq_governor_stop(struct cpufreq_policy *policy, cs_dbs_info->enable = 0; } + + return 0; } -static void cpufreq_governor_limits(struct cpufreq_policy *policy, - struct dbs_data *dbs_data) +static int cpufreq_governor_limits(struct cpufreq_policy *policy, + struct dbs_data *dbs_data) { struct common_dbs_data *cdata = dbs_data->cdata; unsigned int cpu = policy->cpu; struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu); - if (!cdbs->ccdbs) - return; + if (!cdbs->ccdbs->enabled) + return -EBUSY; if (policy->max < cdbs->ccdbs->policy->cur) __cpufreq_driver_target(cdbs->ccdbs->policy, policy->max, @@ -496,13 +524,15 @@ static void cpufreq_governor_limits(struct cpufreq_policy *policy, __cpufreq_driver_target(cdbs->ccdbs->policy, policy->min, CPUFREQ_RELATION_L); dbs_check_cpu(dbs_data, cpu); + + return 0; } int cpufreq_governor_dbs(struct cpufreq_policy *policy, struct common_dbs_data *cdata, unsigned int event) { struct dbs_data *dbs_data; - int ret = 0; + int ret; /* Lock governor to block concurrent initialization of governor */ mutex_lock(&cdata->mutex); @@ -522,17 +552,19 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy, ret = cpufreq_governor_init(policy, dbs_data, cdata); break; case CPUFREQ_GOV_POLICY_EXIT: - cpufreq_governor_exit(policy, dbs_data); + ret = cpufreq_governor_exit(policy, dbs_data); break; case CPUFREQ_GOV_START: ret = cpufreq_governor_start(policy, dbs_data); break; case CPUFREQ_GOV_STOP: - cpufreq_governor_stop(policy, dbs_data); + ret = cpufreq_governor_stop(policy, dbs_data); break; case CPUFREQ_GOV_LIMITS: - cpufreq_governor_limits(policy, dbs_data); + ret = cpufreq_governor_limits(policy, dbs_data); break; + default: + ret = -EINVAL; } unlock: diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h index 2b2884f91fe7..7da5aedb8174 100644 --- a/drivers/cpufreq/cpufreq_governor.h +++ b/drivers/cpufreq/cpufreq_governor.h @@ -130,6 +130,7 @@ static void *get_cpu_dbs_info_s(int cpu) \ /* Common to all CPUs of a policy */ struct cpu_common_dbs_info { + bool enabled; struct cpufreq_policy *policy; ktime_t time_stamp; };