From patchwork Sat Jun 20 03:12:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 50119 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f72.google.com (mail-la0-f72.google.com [209.85.215.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A7998218C9 for ; Sat, 20 Jun 2015 03:12:19 +0000 (UTC) Received: by laer2 with SMTP id r2sf7389298lae.3 for ; Fri, 19 Jun 2015 20:12:18 -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:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=dO/2qTuLAw3xebz8KJ+pDrAn+mN6YJT7nluCCmZ31HU=; b=gDady83GPw/HmovNIFin44Pn8sypSpi8wuVeiK/JdNvyqQ1IUBVp3I+lnq7wSmrPmh fAaSmprPn+KiE6DAzEJTfpcwduX/EQHHMFuFa63Kn6MOCVrjDJaCOvmoexNjvSmTtSM0 fsNjbhFeMimOGUrVCcmjOjOARCAe6uvdZ+RPsinfYf9p6V00dRAPBwMM4u50v0Zzj4Z9 qY8ekws7qO9sWY6ntlsMdlV6/yWgKn88DkHaakqkI9SMRJHkiwMoZMEwA4zVruKnJTkh 8FxdUd323Z0B+Tmn9jwHVdJ5+FxihNd7f3sIU+g+Ao/AbX70drXytYOkksQt8nO1+YgR u0XA== X-Gm-Message-State: ALoCoQlKZV/e/oeleoDuIiJ400GWcRt8ytHk95MihdJsD6+XQEkhZ1hBkWdkezy8yQYLn7bSTGYv X-Received: by 10.112.14.101 with SMTP id o5mr17630027lbc.3.1434769938570; Fri, 19 Jun 2015 20:12:18 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.27.194 with SMTP id v2ls783697lag.59.gmail; Fri, 19 Jun 2015 20:12:18 -0700 (PDT) X-Received: by 10.152.6.1 with SMTP id w1mr20059005law.91.1434769938419; Fri, 19 Jun 2015 20:12:18 -0700 (PDT) Received: from mail-lb0-f172.google.com (mail-lb0-f172.google.com. [209.85.217.172]) by mx.google.com with ESMTPS id cz4si10704421lac.110.2015.06.19.20.12.18 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Jun 2015 20:12:18 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.172 as permitted sender) client-ip=209.85.217.172; Received: by lbbti3 with SMTP id ti3so81892772lbb.1 for ; Fri, 19 Jun 2015 20:12:18 -0700 (PDT) X-Received: by 10.152.6.69 with SMTP id y5mr20254556lay.72.1434769937978; Fri, 19 Jun 2015 20:12:17 -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 hn6csp975758lbb; Fri, 19 Jun 2015 20:12:16 -0700 (PDT) X-Received: by 10.66.161.102 with SMTP id xr6mr38042740pab.8.1434769936038; Fri, 19 Jun 2015 20:12:16 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ei4si19142491pbb.116.2015.06.19.20.12.14; Fri, 19 Jun 2015 20:12:16 -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 S1753372AbbFTDMN (ORCPT + 11 others); Fri, 19 Jun 2015 23:12:13 -0400 Received: from mail-pd0-f179.google.com ([209.85.192.179]:35392 "EHLO mail-pd0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753535AbbFTDMM (ORCPT ); Fri, 19 Jun 2015 23:12:12 -0400 Received: by pdbci14 with SMTP id ci14so43661672pdb.2 for ; Fri, 19 Jun 2015 20:12:11 -0700 (PDT) X-Received: by 10.66.165.67 with SMTP id yw3mr37696470pab.95.1434769931862; Fri, 19 Jun 2015 20:12:11 -0700 (PDT) Received: from localhost ([122.167.70.98]) by mx.google.com with ESMTPSA id b10sm12580805pdo.84.2015.06.19.20.12.10 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 19 Jun 2015 20:12:10 -0700 (PDT) Date: Sat, 20 Jun 2015 08:42:07 +0530 From: Viresh Kumar To: Preeti U Murthy Cc: Rafael Wysocki , ke.wang@spreadtrum.com, 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 Subject: Re: [PATCH V2 10/10] cpufreq: propagate errors returned from __cpufreq_governor() Message-ID: <20150620031207.GE1955@linux> References: <46b51eea20399c927fb1f16839773f618133ae09.1434713657.git.viresh.kumar@linaro.org> <55845930.7020503@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <55845930.7020503@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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.217.172 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: , On 19-06-15, 23:32, Preeti U Murthy wrote: > If INIT itself fails, we need to set policy->governor to NULL. I > included this in the testing of the patchset. Diff from earlier patch: diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index da672b910760..59fa0c1b7922 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2284,14 +2284,23 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, old_gov = policy->governor; /* end old governor */ if (old_gov) { - if(!(ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP))) { + ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP); + if (ret) { + /* This can happen due to race with other operations */ + pr_debug("%s: Failed to Stop Governor: %s (%d)\n", + __func__, old_gov->name, ret); + return ret; + } else { up_write(&policy->rwsem); ret = __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT); down_write(&policy->rwsem); - } - if (ret) - return ret; + if (ret) { + pr_err("%s: Failed to Exit Governor: %s (%d)\n", + __func__, old_gov->name, ret); + return ret; + } + } } /* start new governor */ @@ -2309,7 +2318,9 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, pr_debug("starting governor %s failed\n", policy->governor->name); if (old_gov) { policy->governor = old_gov; - if (!__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) + if (__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) + policy->governor = NULL; + else __cpufreq_governor(policy, CPUFREQ_GOV_START); } New-patch: --------------8<------------------- From: Viresh Kumar Date: Thu, 11 Sep 2014 10:50:48 +0530 Subject: [PATCH] cpufreq: propagate errors returned from __cpufreq_governor() Return codes aren't honored properly in cpufreq_set_policy(). This can lead to two problems: - wrong errors propagated to sysfs - we try to do next state-change even if the previous one failed cpufreq_governor_dbs() now returns proper errors on all invalid state-transition requests and this code should honor that. Reviewed-and-Tested-by: Preeti U Murthy Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index b612411655f9..59fa0c1b7922 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2284,16 +2284,29 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, old_gov = policy->governor; /* end old governor */ if (old_gov) { - __cpufreq_governor(policy, CPUFREQ_GOV_STOP); - up_write(&policy->rwsem); - __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT); - down_write(&policy->rwsem); + ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP); + if (ret) { + /* This can happen due to race with other operations */ + pr_debug("%s: Failed to Stop Governor: %s (%d)\n", + __func__, old_gov->name, ret); + return ret; + } else { + up_write(&policy->rwsem); + ret = __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT); + down_write(&policy->rwsem); + + if (ret) { + pr_err("%s: Failed to Exit Governor: %s (%d)\n", + __func__, old_gov->name, ret); + return ret; + } + } } /* start new governor */ policy->governor = new_policy->governor; - if (!__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) { - if (!__cpufreq_governor(policy, CPUFREQ_GOV_START)) + if (!(ret = __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT))) { + if (!(ret = __cpufreq_governor(policy, CPUFREQ_GOV_START))) goto out; up_write(&policy->rwsem); @@ -2305,11 +2318,13 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, pr_debug("starting governor %s failed\n", policy->governor->name); if (old_gov) { policy->governor = old_gov; - __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT); - __cpufreq_governor(policy, CPUFREQ_GOV_START); + if (__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) + policy->governor = NULL; + else + __cpufreq_governor(policy, CPUFREQ_GOV_START); } - return -EINVAL; + return ret; out: pr_debug("governor: change or update limits\n");