lkml.org 
[lkml]   [2021]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH 3/4] cpufreq: Add Active Stats calls tracking frequency changes
From
Date


On 6/22/21 10:32 AM, Viresh Kumar wrote:
> Not commenting on the idea itself but just the code changes here.
>
> On 22-06-21, 08:59, Lukasz Luba wrote:
>> 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 <linux/active_stats.h>
>> #include <linux/cpu.h>
>> #include <linux/cpufreq.h>
>> #include <linux/cpu_cooling.h>
>> @@ -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);
>> +
>
> It would have been better if you would have modified
> cpufreq_stats_record_transition() instead, since that is there for
> similar kind of stats.

That cpufreq_stats_record_transition() is present only if
CONFIG_CPU_FREQ_STAT is set. I didn't wanted to be dependent on
this config.

>
> Plus don't you need to record this for all policy->cpus instead of
> just policy->cpu ?
>

It will be accounted for all cpus in that freq domain. The
active_stats_cpu_freq_fast_change() implementation uses
a shared structure (single for whole domain) 'shared_ast':
_active_stats_cpu_freq_change(ast->shared_ast, freq, ts)
(from patch 1/4)

\
 
 \ /
  Last update: 2021-06-22 13:08    [W:0.091 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site