From patchwork Thu Dec 14 01:21:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Smythies X-Patchwork-Id: 10111301 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 69E72603ED for ; Thu, 14 Dec 2017 01:21:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5B8E6295DC for ; Thu, 14 Dec 2017 01:21:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4E24429926; Thu, 14 Dec 2017 01:21:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DAFEA295DC for ; Thu, 14 Dec 2017 01:21:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751644AbdLNBVg (ORCPT ); Wed, 13 Dec 2017 20:21:36 -0500 Received: from cmta20.telus.net ([209.171.16.93]:51246 "EHLO cmta20.telus.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751641AbdLNBVe (ORCPT ); Wed, 13 Dec 2017 20:21:34 -0500 Received: from dougxps ([173.180.45.4]) by cmsmtp with SMTP id PIDKeZjAp2lbdPIDLeHx3R; Wed, 13 Dec 2017 18:21:33 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telus.net; s=neo; t=1513214493; bh=W6jiEp6h5sTm79KmNGPDXstZAN6MWiB6icLBqBmoSog=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=6DvE0qkwzpFfddnlCKC09XxxxqxrP8z4zDoIZnaOJ/zbTGo1kbpsqVeX/Sx6skDAh 6xWSPjOkfCgGd5e/SJfZxyuL7mfbCzR+QM2WO1p8UPNOXXS8Z5F/vNHpGuLGuV3Zca w15Yh9Q+2VUjwv7BMVJJVHCl+z0LD18dMwx4IHCvduN6soaxEzWnlSaeu/AiwyFbDe pYJuiFwuPH4EMQVrdcFjSICPZsRmoR/C4xJiVJjtKmqpwd/7lVR6ryrKdaxvcojsp9 4JxZG9dygUYF8f6dlIDWuhnT2Y1V+TVz7rr0VHKmwpj+nBGZ3N46NuTmg1RgSY25+t 11S0bnLKl9dtQ== X-Authority-Analysis: v=2.2 cv=KOm1NBNo c=1 sm=1 tr=0 a=zJWegnE7BH9C0Gl4FFgQyA==:117 a=zJWegnE7BH9C0Gl4FFgQyA==:17 a=Pyq9K9CWowscuQLKlpiwfMBGOR0=:19 a=kj9zAlcOel0A:10 a=s58H9iQMo-e5nxfJ2bEA:9 a=CjuIK1q_8ugA:10 From: "Doug Smythies" To: "'Andy Tang'" , "'Viresh Kumar'" , "'Stratos Karafotis'" Cc: "'Rafael J. Wysocki'" , "'Rafael J. Wysocki'" , "'Linux PM'" , "Doug Smythies" References: <000801d37364$d48f6ed0$7dae4c70$@net> <000f01d373bf$deacca10$9c065e30$@net> <20171213061759.GT25177@vireshk-i7> P0QweRTHbuQ9TP0R1eouYx In-Reply-To: P0QweRTHbuQ9TP0R1eouYx Subject: RE: Ask for help on governor Date: Wed, 13 Dec 2017 17:21:29 -0800 Message-ID: <000701d37479$e0570320$a1050960$@net> MIME-Version: 1.0 X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-ca Thread-Index: AdNy8nQh9C0ITbnWQD+U0VqS+iYXSQAVcrXwACR4GYAAABq9AAAk23ug X-CMAE-Envelope: MS4wfO68crDIFRlb8/Cg65zaLb/4q9XpGuC/HLukEYNzONxP7RaBPbHsL5LoOdRmLD8kAglnHkhjxfw7gVuvXmnrHtv3mZ6qn2Yhy2m/9nMPYzGj3weoO0Rf gyyng6Eo14hczpNcHAFUwIh3PgALewjJaCq11z8tEMYevE0SeU4ztclaVQ+HkbybA+SFvoOFdTWbAJ5WUU3JlDRgbGwN3WhLLvm0o038vmOWOqheKrVHKWeI JXGiJ2K0MSFlDuttU9nq22H4JrZ/kSspT8sKQe3Nt12Hc+DapyoovWRHqzQct4TnNUBZG6jIR96mxL40OM+YmODrmDVXSsV3kyB6q+zmAKBAByg9N35nT+Ja y9+d4oBg Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Note: adding Stratos, the commit author. On 2017.12.12 22:22 Andy Tang wrote: > Anyway I found the root cause myself. > > It was caused by commit: 00bfe05889e91b5112893b001e4a47b0a0f8bdd7. Agreed. Then why did my kernel bisection come to a different conclusion? Because, in my case, the issue only manifested itself when the sampling rate (which should really be called sampling period), became low enough to bring the issue to the surface. The other important thing to note for my system is that I use (O.K., steal) the Ubuntu kernel configuration and so my tick rate is 250 Hz, not 1000 Hz. I think the math for the idle periods calculation breaks down here (from drivers/cpufreq/cpufreq_governor.c): if (time_elapsed > 2 * sampling_rate) { unsigned int periods = time_elapsed / sampling_rate; if (periods < idle_periods) idle_periods = periods; } if 2 * sampling_rate is less than one jiffy. I.E. isn't time_elapsed always exactly one jiffy for a fully loaded CPU? Important note: on my system a jiffy is just over 4 milliseconds. So, for my test which is 100% load on one CPU, basically, idle_periods is always 2, maybe more and the conservative code is always resetting the target CPU frequency to minimum. For whatever reason, on my system, a frequency step of 5% will not raise the pstate, even though it should (the math works out to 1790 MHz, or pstate 17, but I never see it. If I raise the frequency step to anything else, the math makes complete sense. Example: frequency step = 10% so 3800 * 0.1 + 1600 = 1980 which means I should see pstate 19 being asked for. I do. However, it does not continue to increase because of the idle_periods problem, driving it back as an intermediate calculation, so all I ever see is requested pstate of 19. O.K. so if all this is true, then a 1000 Hz kernel shouldn't have a problem. Sort of, it doesn't. Why "sort of"? Because the default sample period of 500 usec is right on the edge, and sometimes the requested pstate does drop as a result. I used this command to watch the requested pstate: watch -d -n 1 sudo rdmsr --bitfield 15:8 -d -a 0x199 (translation: watch the actual requested pstates for all CPUs, by reading the processor itself.) and while for CPU 7, it should clamp at 38 (for my system) it doesn't. O.K. so just increase the sampling period a little, to say 510 uSec, and then yes, it clamps at 38. O.K. so finally, I reverted the commit (but in a cheating way): And everything is fine. ... Doug diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 58d4f4e..3493ca7 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -222,6 +222,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy) max_load = load; } + idle_periods = 0; policy_dbs->idle_periods = idle_periods; return max_load;