From patchwork Fri Jun 26 06:19:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 50335 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f199.google.com (mail-wi0-f199.google.com [209.85.212.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 26B9C228FC for ; Fri, 26 Jun 2015 06:19:50 +0000 (UTC) Received: by wicnd19 with SMTP id nd19sf2613863wic.0 for ; Thu, 25 Jun 2015 23:19:49 -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=jb3QLNwur0w0a2o58lAINhUfXiO5uhw6qPO753wzjvs=; b=BhwVHgXpJyvo32xDZBies0g4TMYmCcIOHlAF2DYh49JLzStZtJt3pakd8JR+T2S8ZW QpgZW2kYSuHAtH2Ug2a1P9qhdiUB5DQeZ79P031BnyGaAWJcwLmPfa5QpjSHRs/ppSwN 0eIf7bpHz44Z7VfUJ8PY8Dh3WkYx39S9/K+4UcHl4Qut4lY67gXzW3/VtJyY27Acz6vv qXl4ZhlRlErugTCc2lk8vxW3o+jVp8dk3O8G57yGXxQhxbKgcOGICo/TxAzYqKrtg0SQ +yBlxqt0JTXKE1nG7EaGl5+8djnYrIbVty0zeo57MOdIZ+ndOYxXO5CIvpu89zaOpptU du9Q== X-Gm-Message-State: ALoCoQmJgOtRG699i8OizaeN8kWVVTMuEYTZGEuM7L7MrA0Sz8+cNrkC2b4d8zu5qhYPelljGOTc X-Received: by 10.181.12.104 with SMTP id ep8mr763665wid.7.1435299589274; Thu, 25 Jun 2015 23:19:49 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.8.111 with SMTP id q15ls388390laa.28.gmail; Thu, 25 Jun 2015 23:19:49 -0700 (PDT) X-Received: by 10.152.20.138 with SMTP id n10mr31179089lae.115.1435299588985; Thu, 25 Jun 2015 23:19:48 -0700 (PDT) Received: from mail-lb0-f177.google.com (mail-lb0-f177.google.com. [209.85.217.177]) by mx.google.com with ESMTPS id d9si26528307lad.19.2015.06.25.23.19.48 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Jun 2015 23:19:48 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.177 as permitted sender) client-ip=209.85.217.177; Received: by lbnk3 with SMTP id k3so58398236lbn.1 for ; Thu, 25 Jun 2015 23:19:48 -0700 (PDT) X-Received: by 10.152.36.161 with SMTP id r1mr15878laj.88.1435299588858; Thu, 25 Jun 2015 23:19:48 -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 hn6csp1074420lbb; Thu, 25 Jun 2015 23:19:47 -0700 (PDT) X-Received: by 10.70.54.7 with SMTP id f7mr97410043pdp.75.1435299584330; Thu, 25 Jun 2015 23:19:44 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id tl1si48837819pac.65.2015.06.25.23.19.43; Thu, 25 Jun 2015 23:19:44 -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 S1750729AbbFZGTm (ORCPT + 11 others); Fri, 26 Jun 2015 02:19:42 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:34300 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751557AbbFZGTl (ORCPT ); Fri, 26 Jun 2015 02:19:41 -0400 Received: by pabvl15 with SMTP id vl15so63351359pab.1 for ; Thu, 25 Jun 2015 23:19:41 -0700 (PDT) X-Received: by 10.68.69.98 with SMTP id d2mr7330397pbu.71.1435299581303; Thu, 25 Jun 2015 23:19:41 -0700 (PDT) Received: from localhost ([223.227.180.190]) by mx.google.com with ESMTPSA id c3sm32080725pdk.39.2015.06.25.23.19.38 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 25 Jun 2015 23:19:40 -0700 (PDT) Date: Fri, 26 Jun 2015 11:49:31 +0530 From: Viresh Kumar To: Preeti U Murthy Cc: Rafael Wysocki , linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org Subject: Re: [PATCH 03/10] cpufreq: conservative: remove 'enable' field Message-ID: <20150626061931.GD16275@linux> References: <558CE9DD.1050105@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <558CE9DD.1050105@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.177 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 26-06-15, 11:27, Preeti U Murthy wrote: > On 06/22/2015 01:32 PM, Viresh Kumar wrote: > > Conservative governor has its own 'enable' field to check two things: > > - If conservative governor is used for a CPU or not > > - If governor is currently enabled or not, as there can be a race around > > the notifier being called while it was getting unregistered from > > cpufreq_governor_dbs(). > > The race is between changing governors in cpufreq_set_policy() and the > notifier being called, isn't it ? The governor will get unregistered > when we remove the cpufreq module and here too we do not set > policy->governor to NULL nor set the enable bit to 0. So it does not > look like we were protecting these checks against un-registering the > governor. I was talking about the same race which I believed to exist in 2/10 as well.. But there is no such race it seems as we discussed yesterday. So, only the first point is what the enable field was required for. And because of that getting NAK'd, here is the new version: -------------------------8<----------------- Message-Id: <4ce8fc98a46af394118237ef90ed0b80a7652cfb.1435299551.git.viresh.kumar@linaro.org> From: Viresh Kumar Date: Fri, 19 Jun 2015 11:40:14 +0530 Subject: [PATCH] cpufreq: conservative: remove 'enable' field Conservative governor has its own 'enable' field to check if conservative governor is used for a CPU or not This can be checked by policy->governor with 'cpufreq_gov_conservative' and so this field can be dropped. Because its not guaranteed that dbs_info->cdbs.ccdbs will a valid pointer for all CPUs (will be NULL for CPUs that don't use ondemand/conservative governors), we can't use it anymore. Lets get policy with cpufreq_cpu_get() instead. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq_conservative.c | 34 +++++++++++++++++++++------------- drivers/cpufreq/cpufreq_governor.c | 12 +----------- drivers/cpufreq/cpufreq_governor.h | 1 - 3 files changed, 22 insertions(+), 25 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 0e4154e584bf..f53719e5bed9 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -23,6 +23,19 @@ static DEFINE_PER_CPU(struct cs_cpu_dbs_info_s, cs_cpu_dbs_info); +static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy, + unsigned int event); + +#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE +static +#endif +struct cpufreq_governor cpufreq_gov_conservative = { + .name = "conservative", + .governor = cs_cpufreq_governor_dbs, + .max_transition_latency = TRANSITION_LATENCY_LIMIT, + .owner = THIS_MODULE, +}; + static inline unsigned int get_freq_target(struct cs_dbs_tuners *cs_tuners, struct cpufreq_policy *policy) { @@ -119,12 +132,14 @@ static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val, struct cpufreq_freqs *freq = data; struct cs_cpu_dbs_info_s *dbs_info = &per_cpu(cs_cpu_dbs_info, freq->cpu); - struct cpufreq_policy *policy; + struct cpufreq_policy *policy = cpufreq_cpu_get(freq->cpu); - if (!dbs_info->enable) + if (!policy) return 0; - policy = dbs_info->cdbs.ccdbs->policy; + /* policy isn't governed by conservative governor */ + if (policy->governor != &cpufreq_gov_conservative) + goto policy_put; /* * we only care if our internally tracked freq moves outside the 'valid' @@ -134,6 +149,9 @@ static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val, || dbs_info->requested_freq < policy->min) dbs_info->requested_freq = freq->new; +policy_put: + cpufreq_cpu_put(policy); + return 0; } @@ -367,16 +385,6 @@ static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy, return cpufreq_governor_dbs(policy, &cs_dbs_cdata, event); } -#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE -static -#endif -struct cpufreq_governor cpufreq_gov_conservative = { - .name = "conservative", - .governor = cs_cpufreq_governor_dbs, - .max_transition_latency = TRANSITION_LATENCY_LIMIT, - .owner = THIS_MODULE, -}; - static int __init cpufreq_gov_dbs_init(void) { return cpufreq_register_governor(&cpufreq_gov_conservative); diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index af63402a94a9..836aefd03c1b 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -463,7 +463,6 @@ static int cpufreq_governor_start(struct cpufreq_policy *policy, cdata->get_cpu_dbs_info_s(cpu); cs_dbs_info->down_skip = 0; - cs_dbs_info->enable = 1; cs_dbs_info->requested_freq = policy->cur; } else { struct od_ops *od_ops = cdata->gov_ops; @@ -482,9 +481,7 @@ static int cpufreq_governor_start(struct cpufreq_policy *policy, 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_dbs_info *cdbs = dbs_data->cdata->get_cpu_cdbs(policy->cpu); struct cpu_common_dbs_info *ccdbs = cdbs->ccdbs; /* State should be equivalent to START */ @@ -493,13 +490,6 @@ static int cpufreq_governor_stop(struct cpufreq_policy *policy, gov_cancel_work(dbs_data, policy); - if (cdata->governor == GOV_CONSERVATIVE) { - struct cs_cpu_dbs_info_s *cs_dbs_info = - cdata->get_cpu_dbs_info_s(cpu); - - cs_dbs_info->enable = 0; - } - ccdbs->policy = NULL; mutex_destroy(&ccdbs->timer_mutex); return 0; diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h index 2125c299c602..a0d24149f18c 100644 --- a/drivers/cpufreq/cpufreq_governor.h +++ b/drivers/cpufreq/cpufreq_governor.h @@ -170,7 +170,6 @@ struct cs_cpu_dbs_info_s { struct cpu_dbs_info cdbs; unsigned int down_skip; unsigned int requested_freq; - unsigned int enable:1; }; /* Per policy Governors sysfs tunables */