From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B925DC2B9F4 for ; Tue, 22 Jun 2021 13:42:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9D01061374 for ; Tue, 22 Jun 2021 13:42:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230103AbhFVNoz (ORCPT ); Tue, 22 Jun 2021 09:44:55 -0400 Received: from foss.arm.com ([217.140.110.172]:49466 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229988AbhFVNoz (ORCPT ); Tue, 22 Jun 2021 09:44:55 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2B6D2ED1; Tue, 22 Jun 2021 06:42:39 -0700 (PDT) Received: from [10.57.7.129] (unknown [10.57.7.129]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 16BB43F694; Tue, 22 Jun 2021 06:42:36 -0700 (PDT) Subject: Re: [RFC PATCH 3/4] cpufreq: Add Active Stats calls tracking frequency changes To: "Rafael J. Wysocki" Cc: Linux Kernel Mailing List , Daniel Lezcano , Linux PM , Amit Kucheria , "Zhang, Rui" , Dietmar Eggemann , Chris Redpath , Beata.Michalska@arm.com, Viresh Kumar , "Rafael J. Wysocki" , Amit Kachhap References: <20210622075925.16189-1-lukasz.luba@arm.com> <20210622075925.16189-4-lukasz.luba@arm.com> From: Lukasz Luba Message-ID: <4e5476a6-fa9f-a9ef-ff26-8fa1b4bb90c0@arm.com> Date: Tue, 22 Jun 2021 14:42:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 6/22/21 1:28 PM, Rafael J. Wysocki wrote: > On Tue, Jun 22, 2021 at 9:59 AM Lukasz Luba wrote: >> >> The Active Stats framework tracks and accounts the activity of the CPU >> for each performance level. It accounts the real residency, when the CPU >> was not idle, at a given performance level. This patch adds needed calls >> which provide the CPU frequency transition events to the Active Stats >> framework. >> >> Signed-off-by: Lukasz Luba >> --- >> drivers/cpufreq/cpufreq.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 802abc925b2a..d79cb9310572 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -14,6 +14,7 @@ >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> >> +#include >> #include >> #include >> #include >> @@ -387,6 +388,8 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy, >> >> cpufreq_stats_record_transition(policy, freqs->new); >> policy->cur = freqs->new; >> + >> + active_stats_cpu_freq_change(policy->cpu, freqs->new); >> } >> } >> >> @@ -2085,6 +2088,8 @@ unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy, >> policy->cpuinfo.max_freq); >> cpufreq_stats_record_transition(policy, freq); >> >> + active_stats_cpu_freq_fast_change(policy->cpu, freq); >> + > > This is quite a bit of overhead and so why is it needed in addition to > the code below? The code below is tracing, which is good for post-processing. We use in our tool LISA, when we analyze the EAS decision, based on captured trace data. This new code is present at run time, so subsystems like our thermal governor IPA can use it and get better estimation about CPU used power for any arbitrary period, e.g. 50ms, 100ms, 300ms, ... > > And pretty much the same goes for the idle loop change. There is > quite a bit of instrumentation in that code already and it avoids > adding new locking for a reason. Why is it a good idea to add more > locking to that code? This active_stats_cpu_freq_fast_change() doesn't use the locking, it relies on schedutil lock in [1]. > >> if (trace_cpu_frequency_enabled()) { >> for_each_cpu(cpu, policy->cpus) >> trace_cpu_frequency(freq, cpu); >> -- [1] https://elixir.bootlin.com/linux/latest/source/kernel/sched/cpufreq_schedutil.c#L447