From patchwork Tue Sep 10 16:22:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guennadi Liakhovetski X-Patchwork-Id: 2866921 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 16F2E9F485 for ; Tue, 10 Sep 2013 16:23:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0A686202E8 for ; Tue, 10 Sep 2013 16:23:29 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 446C2202D9 for ; Tue, 10 Sep 2013 16:23:27 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VJQip-0008VK-My; Tue, 10 Sep 2013 16:23:23 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VJQin-0001qQ-HW; Tue, 10 Sep 2013 16:23:21 +0000 Received: from moutng.kundenserver.de ([212.227.126.187]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VJQik-0001pD-80 for linux-arm-kernel@lists.infradead.org; Tue, 10 Sep 2013 16:23:19 +0000 Received: from axis700.grange (dslb-088-076-070-015.pools.arcor-ip.net [88.76.70.15]) by mrelayeu.kundenserver.de (node=mrbap1) with ESMTP (Nemesis) id 0M7m2S-1WEiE43vOq-00vw3G; Tue, 10 Sep 2013 18:22:49 +0200 Received: by axis700.grange (Postfix, from userid 1000) id 89D9540BB4; Tue, 10 Sep 2013 18:22:48 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by axis700.grange (Postfix) with ESMTP id 866CD40BB3; Tue, 10 Sep 2013 18:22:48 +0200 (CEST) Date: Tue, 10 Sep 2013 18:22:48 +0200 (CEST) From: Guennadi Liakhovetski X-X-Sender: lyakh@axis700.grange To: Viresh Kumar Subject: Re: "cpufreq: fix serialization issues with freq change notifiers" breaks cpufreq too In-Reply-To: Message-ID: References: MIME-Version: 1.0 X-Provags-ID: V02:K0:4WD/d6JczBAK6wibxkkWbmhD5WCf49EytDZ5weOK48V uVxkgrBd2r4bRVYfvBv6p7vilTMcN9KQM7YO5bz2QExOelSWsB AyOaetdJz2pQVqWfQs3zXydiVGZE5k+joity4XmybWnOkmMKPM sQKdSdo9dXhb7A5zNKkXBQLWA95lMrDwh7D2s8shsO3+YJpFP0 wB52JzX2IrnmPM8GctNuBpc+o39q6K48Hq78c9aWRBazDXMM4g l6H3yIm/SvosXc/DElt44L4Lw5u2ooVUU8Brb8x4zIYjpqhTEF Xw7tbKheP6WwLj3bi55EsMZuFjzOVjHKZy0WAMF8mJ+rV7KbJj SIrBUHdZNRR5TAplTRhFkxkVm98jI67yKRVZyCrGIkFYhuJP46 GY3qrs08ML2+w== X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130910_122318_502174_884509FD X-CRM114-Status: GOOD ( 22.94 ) X-Spam-Score: -2.6 (--) Cc: "linux-pm@vger.kernel.org" , SH-Linux , Greg KH , "Rafael J. Wysocki" , Magnus Damm , Linux Kernel Mailing List , "cpufreq@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 10 Sep 2013, Viresh Kumar wrote: > On 10 September 2013 20:42, Guennadi Liakhovetski wrote: > > 4. reverted that commit, resolving a trivial conflict. Added a debug > > output in __cpufreq_driver_target() of > > > > > > if (cpufreq_disabled()) > > return -ENODEV; > > + pr_info("%s() %d\n", __func__, policy->transition_ongoing); > > if (policy->transition_ongoing) > > return -EBUSY; > > Are you sure this diff is on linux-next and not after the revert that > you mentioned later in the mail? There is some locking introduced > by my patch which I can't see in you diff.. Of course, isn't that what I've written above? reverted a commit and added debug - in that order. > > Built and booted, got > > > > cpufreq: __cpufreq_driver_target(): 1 > > > > printed out 4 times from the beginning. > > > > 5. tried > > > > echo powersave > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor > > > > the above output appeared 2 more times - no frequency change resulted. > > > > 6. reverted commit dceff5ce18801dddc220d6238628619c93bc3cb6, built booted > > - cpufreq works again. > > > >> I am afraid you need to give us some more information on how it broke > >> your stuff.. :) > > > > Hope the above is enough. > > A bit more would be helpful.. Can you add these debug prints to all the places > where transition_ongoing is getting modified? with %s, __func__ to distinguish > them better? That will make it a bit easy for me... Sure, I can... So, with the performance governor I get [ 1.290000] cpufreq-cpu0 cpufreq-cpu0: Looking up cpu0-supply from device tree [ 1.290000] cpufreq: trying to register driver generic_cpu0 [ 1.290000] cpufreq: adding CPU 0 [ 1.290000] cpufreq: Adding link for CPU: 1 [ 1.290000] cpufreq: setting new policy for CPU 0: 398667 - 1196000 kHz [ 1.290000] cpufreq: new min and max freqs are 398667 - 1196000 kHz [ 1.290000] cpufreq: governor switch [ 1.290000] cpufreq: __cpufreq_governor for CPU 0, event 4 [ 1.290000] cpufreq: __cpufreq_governor for CPU 0, event 1 [ 1.290000] cpufreq_performance: setting to 1196000 kHz because of event 1 [ 1.290000] cpufreq: __cpufreq_driver_target().1665 1 This is my debug - .transition_ongoing is incremented ^^^^^^^^ [ 1.300000] cpufreq: target for CPU 0: 1196000 kHz, relation 1, requested 1196000 kHz [ 1.300000] cpufreq: governor: change or update limits [ 1.300000] cpufreq: __cpufreq_governor for CPU 0, event 3 [ 1.300000] cpufreq_performance: setting to 1196000 kHz because of event 3 [ 1.300000] cpufreq: initialization complete [ 1.300000] cpufreq: adding CPU 1 [ 1.300000] cpufreq: driver generic_cpu0 up and running That's it. It never gets decremented again. > Also, what's the configuration of your system? How many CPUs? 2 CPU cores. > And are all of them sharing clock? (I believe yes, as you are using cpufreq-cpu0).. Correct. Debug diff is below. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index ecc55d1..374e030 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -15,6 +15,8 @@ * published by the Free Software Foundation. */ +#define DEBUG + #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include @@ -292,6 +294,7 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy, policy->transition_ongoing++; write_unlock_irqrestore(&cpufreq_driver_lock, flags); + pr_info("%s().%d %d\n", __func__, __LINE__, policy->transition_ongoing); /* detect if the driver reported a value as "old frequency" * which is not equal to what the cpufreq core thinks is @@ -321,6 +324,7 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy, policy->transition_ongoing--; write_unlock_irqrestore(&cpufreq_driver_lock, flags); + pr_info("%s().%d %d\n", __func__, __LINE__, policy->transition_ongoing); adjust_jiffies(CPUFREQ_POSTCHANGE, freqs); pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new, @@ -359,6 +363,7 @@ void cpufreq_notify_transition(struct cpufreq_policy *policy, write_lock_irqsave(&cpufreq_driver_lock, flags); policy->transition_ongoing--; write_unlock_irqrestore(&cpufreq_driver_lock, flags); + pr_info("%s().%d %d\n", __func__, __LINE__, policy->transition_ongoing); } } EXPORT_SYMBOL_GPL(cpufreq_notify_transition); @@ -1356,6 +1361,7 @@ static void cpufreq_out_of_sync(unsigned int cpu, unsigned int old_freq, } policy->transition_ongoing++; write_unlock_irqrestore(&cpufreq_driver_lock, flags); + pr_info("%s().%d %d\n", __func__, __LINE__, policy->transition_ongoing); cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE); cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE); @@ -1656,6 +1662,7 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy, } policy->transition_ongoing++; write_unlock_irqrestore(&cpufreq_driver_lock, flags); + pr_info("%s().%d %d\n", __func__, __LINE__, policy->transition_ongoing); /* Make sure that target_freq is within supported range */ if (target_freq > policy->max) diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c index cf117de..5575b08 100644 --- a/drivers/cpufreq/cpufreq_performance.c +++ b/drivers/cpufreq/cpufreq_performance.c @@ -10,6 +10,8 @@ * */ +#define DEBUG + #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include