From patchwork Fri Jan 11 19:03:35 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aravind Gopalakrishnan X-Patchwork-Id: 1967061 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 71277DF2A2 for ; Fri, 11 Jan 2013 19:04:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753841Ab3AKTEU (ORCPT ); Fri, 11 Jan 2013 14:04:20 -0500 Received: from am1ehsobe006.messaging.microsoft.com ([213.199.154.209]:33051 "EHLO am1outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753679Ab3AKTET (ORCPT ); Fri, 11 Jan 2013 14:04:19 -0500 Received: from mail48-am1-R.bigfish.com (10.3.201.239) by AM1EHSOBE011.bigfish.com (10.3.207.133) with Microsoft SMTP Server id 14.1.225.23; Fri, 11 Jan 2013 19:04:17 +0000 Received: from mail48-am1 (localhost [127.0.0.1]) by mail48-am1-R.bigfish.com (Postfix) with ESMTP id 6FBAD340263; Fri, 11 Jan 2013 19:04:17 +0000 (UTC) X-Forefront-Antispam-Report: CIP:163.181.249.108; KIP:(null); UIP:(null); IPV:NLI; H:ausb3twp01.amd.com; RD:none; EFVD:NLI X-SpamScore: -7 X-BigFish: VPS-7(zz98dI9371I15bfK542I1432Izz1ee6h1de0h1202h1e76h1d1ah1d2ahzz8275bh8275dhz2dh668h839h93fhd25hf0ah1288h12a5h12a9h12bdh137ah13b6h1441h1504h1537h153bh15d0h162dh1631h1758h1155h) Received: from mail48-am1 (localhost.localdomain [127.0.0.1]) by mail48-am1 (MessageSwitch) id 1357931019186519_20054; Fri, 11 Jan 2013 19:03:39 +0000 (UTC) Received: from AM1EHSMHS015.bigfish.com (unknown [10.3.201.234]) by mail48-am1.bigfish.com (Postfix) with ESMTP id 209C9140118; Fri, 11 Jan 2013 19:03:39 +0000 (UTC) Received: from ausb3twp01.amd.com (163.181.249.108) by AM1EHSMHS015.bigfish.com (10.3.207.153) with Microsoft SMTP Server id 14.1.225.23; Fri, 11 Jan 2013 19:03:38 +0000 X-WSS-ID: 0MGH6A0-01-1A1-02 X-M-MSG: Received: from sausexedgep01.amd.com (sausexedgep01-ext.amd.com [163.181.249.72]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by ausb3twp01.amd.com (Axway MailGate 3.8.1) with ESMTP id 261841028064; Fri, 11 Jan 2013 13:03:35 -0600 (CST) Received: from SAUSEXDAG03.amd.com (163.181.55.3) by sausexedgep01.amd.com (163.181.36.54) with Microsoft SMTP Server (TLS) id 8.3.192.1; Fri, 11 Jan 2013 13:03:56 -0600 Received: from SAUSEXDAG04.amd.com ([fe80::9143:6575:e649:e862]) by sausexdag03.amd.com ([fe80::85b5:3838:d8b4:20ba%19]) with mapi id 14.02.0318.004; Fri, 11 Jan 2013 13:03:35 -0600 From: "Gopalakrishnan, Aravind" To: Borislav Petkov , Andre Przywara , "rjw@sisk.pl" , "cpufreq@vger.kernel.org" , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" CC: Andreas Subject: RE: [PATCH] drivers/cpufreq: Warn user when powernow-k8 tries to fall back to acpi-cpufreq and it is unavailable. Thread-Topic: [PATCH] drivers/cpufreq: Warn user when powernow-k8 tries to fall back to acpi-cpufreq and it is unavailable. Thread-Index: AQHN7s8exX7w9Ky4iEWIRc9vlXABaphEnLsAgAAh3wD//8ByQA== Date: Fri, 11 Jan 2013 19:03:35 +0000 Message-ID: <4923C2DE085EEB4FAB1D375DD09D0BA6100CF170@sausexdag04.amd.com> References: <1357780161-30581-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <20130111144940.GB21882@liondog.tnic> <20130111165054.GD10751@liondog.tnic> In-Reply-To: <20130111165054.GD10751@liondog.tnic> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.236.49.67] MIME-Version: 1.0 X-OriginatorOrg: amd.com Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org So, I had tried out the case when acpi-cpufreq was compiled into the kernel and looked at the return values from request_module(); it returns a positive value (255) both when acpi-cpufreq was compiled-in and when not compiled-in. (Please do let me know if I am missing something here...) (This was the case Andreas had mentioned in the bug report too) It was due to this that I had decided to just check the CONFIG option and print out a warning to the user. -----Original Message----- From: Borislav Petkov [mailto:bp@alien8.de] Sent: Friday, January 11, 2013 10:51 AM To: Gopalakrishnan, Aravind; Andre Przywara; rjw@sisk.pl; cpufreq@vger.kernel.org; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org Cc: Andreas Subject: Re: [PATCH] drivers/cpufreq: Warn user when powernow-k8 tries to fall back to acpi-cpufreq and it is unavailable. Adding bugreporter from BZ to CC. On Fri, Jan 11, 2013 at 03:49:40PM +0100, Borislav Petkov wrote: > + Andre. > > On Wed, Jan 09, 2013 at 07:09:21PM -0600, Aravind Gopalakrishnan wrote: > > This patch is in reference to bug#:51741. > > (https://bugzilla.kernel.org/show_bug.cgi?id=51741) > > powernow-k8 falls back to acpi-cpufreq if CPU is not supported. > > However, it states that acpi-cpufreq has taken over even if > > acpi-cpufreq is not compiled in. This patch rewords the warning > > message to clarify that the CPU is unsupported and prints a warning message when there is no acpi-cpufreq present. > > > > Signed-off-by: Aravind Gopalakrishnan > > > > --- > > drivers/cpufreq/powernow-k8.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/powernow-k8.c > > b/drivers/cpufreq/powernow-k8.c index 056faf6..6fa58b4 100644 > > --- a/drivers/cpufreq/powernow-k8.c > > +++ b/drivers/cpufreq/powernow-k8.c > > @@ -1256,7 +1256,15 @@ static int __cpuinit powernowk8_init(void) > > int rv; > > > > if (static_cpu_has(X86_FEATURE_HW_PSTATE)) { > > - pr_warn(PFX "this CPU is not supported anymore, using acpi-cpufreq instead.\n"); > > + pr_warn(PFX > > + "this CPU is not supported anymore, use acpi-cpufreq instead" > > + "Look for message from acpi-cpufreq to ensure it is loaded." > > + ".\n"); > > +#ifndef CONFIG_X86_ACPI_CPUFREQ > > + pr_warn(PFX "acpi-cpufreq is disabled." > > + "Enable it in the config options to get frequency scaling.\n"); > > + return -ENODEV; > > +#endif > > request_module("acpi-cpufreq"); > > return -ENODEV; > > Ok, the suggestion in that BZ is valid and something needs to be done > for that case but I don't think that simply warning the user about it > is enough. > > First of all, CONFIG_X86_POWERNOW_K8 should depend on > CONFIG_X86_ACPI_CPUFREQ since powernow-k8.ko requests that module. > > Then, having that dependency out of the way, we can almost safely > request_module("acpi-cpufreq"). However, we can also check the return > value of that function to make sure loading went fine. > > And finally, we should check that acpi-cpufreq actually registered > properly and wasn't unloaded later for some other reason which wasn't > signalled through request_module retval. > > Andre, I'm not sure about the details of that last one but the first > two are easy. Any ideas, since you've been looking at acpi-cpufreq > code lately? Ok, this turned out to be not that hard, here's what I came up with. I haven't checked the case where acpi-cpufreq and powernow-k8 are built in and what happens then. This still doesn't deal with pathological situations when acpi-cpufreq f*cks up loading later and errors out. For that, we probably need to export some cpufreq internals like cpufreq_driver and look at its ->name member and decide whether it is properly set. I'm not sure that is really warranted though... In the meantime, here's a first stab at it, which should cover the major holes. -- From: Borislav Petkov Date: Fri, 11 Jan 2013 17:36:58 +0100 Subject: [PATCH] powernow-k8: Robustify loading of acpi-cpufreq Andreas says in https://bugzilla.kernel.org/show_bug.cgi?id=51741 that with his Gentoo config, acpi-cpufreq wasn't enabled and powernow-k8 couldn't handoff properly to acpi-cpufreq leading to running without P-state support (i.e., cores are constantly in P0). To alleaviate that, we need to make powernow-k8 depend on acpi-cpufreq so that acpi-cpufreq is always present as a module and is thus loadable. Also, request_module waits until acpi-cpufreq is loaded so we check its retval whether the loading succeeded or not. If not, we warn the user so that she can take precaution and fix the situation. Signed-off-by: Borislav Petkov --- drivers/cpufreq/Kconfig.x86 | 2 +- drivers/cpufreq/powernow-k8.c | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) -- 1.8.1.rc3 -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86 index 934854ae5eb4..7227cd734042 100644 --- a/drivers/cpufreq/Kconfig.x86 +++ b/drivers/cpufreq/Kconfig.x86 @@ -106,7 +106,7 @@ config X86_POWERNOW_K7_ACPI config X86_POWERNOW_K8 tristate "AMD Opteron/Athlon64 PowerNow!" select CPU_FREQ_TABLE - depends on ACPI && ACPI_PROCESSOR + depends on ACPI && ACPI_PROCESSOR && X86_ACPI_CPUFREQ help This adds the CPUFreq driver for K8/early Opteron/Athlon64 processors. Support for K10 and newer processors is now in acpi-cpufreq. diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c index 056faf6af1a9..1adbe86d0d3b 100644 --- a/drivers/cpufreq/powernow-k8.c +++ b/drivers/cpufreq/powernow-k8.c @@ -1256,8 +1256,12 @@ static int __cpuinit powernowk8_init(void) int rv; if (static_cpu_has(X86_FEATURE_HW_PSTATE)) { - pr_warn(PFX "this CPU is not supported anymore, using acpi-cpufreq instead.\n"); - request_module("acpi-cpufreq"); + pr_warn(PFX "This CPU is now supported by acpi-cpufreq, loading +it.\n"); + + if (request_module("acpi-cpufreq")) + pr_warn(PFX "Error loading acpi-cpufreq, make sure you " + "have it enabled, else you won't have any " + "Pstates support on this CPU!\n"); return -ENODEV; }