From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965327AbcALLHG (ORCPT ); Tue, 12 Jan 2016 06:07:06 -0500 Received: from mail-pa0-f42.google.com ([209.85.220.42]:36640 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1762192AbcALLHD (ORCPT ); Tue, 12 Jan 2016 06:07:03 -0500 Date: Tue, 12 Jan 2016 16:36:58 +0530 From: Viresh Kumar To: Juri Lelli Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, peterz@infradead.org, rjw@rjwysocki.net, mturquette@baylibre.com, steve.muckle@linaro.org, vincent.guittot@linaro.org, morten.rasmussen@arm.com, dietmar.eggemann@arm.com Subject: Re: [RFC PATCH 15/19] cpufreq: remove useless usage of cpufreq_governor_mutex in __cpufreq_governor Message-ID: <20160112110658.GG1084@ubuntu> References: <1452533760-13787-1-git-send-email-juri.lelli@arm.com> <1452533760-13787-16-git-send-email-juri.lelli@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452533760-13787-16-git-send-email-juri.lelli@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11-01-16, 17:35, Juri Lelli wrote: > Commit 6f1e4efd882e ("cpufreq: Fix timer/workqueue corruption by > protecting reading governor_enabled") made policy->governor_enabled > guarded by cpufreq_governor_mutex in __cpufreq_governor. Now that > holding of policy->rwsem is asserted in __cpufreq_governor, > cpufreq_governor_mutex is overkilling. I am sure that is going to break it. Try that x86, somehow I don't get it on my exynos boards. > - mutex_lock(&cpufreq_governor_mutex); > if ((policy->governor_enabled && event == CPUFREQ_GOV_START) > || (!policy->governor_enabled > && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) { > - mutex_unlock(&cpufreq_governor_mutex); > return -EBUSY; > } Actually the above checks should also be removed as the governors are responsible for maintaining their state machines. But userspace/powersave/performance don't have that support yet and so these checks save them from going into undefined states. Over that, above and below checks are incomplete.. > @@ -2006,8 +2004,6 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, > else if (event == CPUFREQ_GOV_START) > policy->governor_enabled = true; > > - mutex_unlock(&cpufreq_governor_mutex); > - > ret = policy->governor->governor(policy, event); > > if (!ret) { > @@ -2017,12 +2013,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, > policy->governor->initialized--; > } else { > /* Restore original values */ > - mutex_lock(&cpufreq_governor_mutex); > if (event == CPUFREQ_GOV_STOP) > policy->governor_enabled = true; > else if (event == CPUFREQ_GOV_START) > policy->governor_enabled = false; > - mutex_unlock(&cpufreq_governor_mutex); > } > > if (((event == CPUFREQ_GOV_POLICY_INIT) && ret) || > -- > 2.2.2 -- viresh