All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukasz Luba <lukasz.luba@arm.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: rafael@kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH 5/5] powercap/drivers/dtpm: Scale the power with the load
Date: Tue, 9 Mar 2021 20:44:17 +0000	[thread overview]
Message-ID: <a9275548-1555-ce96-51c8-67942d757119@arm.com> (raw)
In-Reply-To: <e1dac038-2100-abdb-2ffe-d0d93952ca21@linaro.org>



On 3/9/21 7:03 PM, Daniel Lezcano wrote:
> 
> Hi Lukasz,
> 
> thanks for your comments, one question below.
> 
> On 09/03/2021 11:01, Lukasz Luba wrote:
> 
> [ ... ]
> 
>>>    +static u64 scale_pd_power_uw(struct cpumask *cpus, u64 power)
>>
>> renamed 'cpus' into 'pd_mask', see below
>>
>>> +{
>>> +    unsigned long max, util;
>>> +    int cpu, load = 0;
>>
>> IMHO 'int load' looks odd when used with 'util' and 'max'.
>> I would put in the line above to have them all the same type and
>> renamed to 'sum_util'.
>>
>>> +
>>> +    for_each_cpu(cpu, cpus) {
>>
>> I would avoid the temporary CPU mask in the get_pd_power_uw()
>> with this modified loop:
>>
>> for_each_cpu_and(cpu, pd_mask, cpu_online_mask) {
>>
>>
>>> +        max = arch_scale_cpu_capacity(cpu);
>>> +        util = sched_cpu_util(cpu, max);
>>> +        load += ((util * 100) / max);
>>
>> Below you can find 3 optimizations. Since we are not in the hot
>> path here, it's up to if you would like to use all/some of them
>> or just ignore.
>>
>> 1st optimization.
>> If we use 'load += (util << 10) / max' in the loop, then
>> we could avoid div by 100 and use a right shift:
>> (power * load) >> 10
>>
>> 2nd optimization.
>> Since we use EM CPU mask, which span all CPUs with the same
>> arch_scale_cpu_capacity(), you can avoid N divs inside the loop
>> and do it once, below the loop.
>>
>> 3rd optimization.
>> If we just simply add all 'util' into 'sum_util' (no mul or div in
>> the loop), then we might just have simple macro
>>
>> #define CALC_POWER_USAGE(power, sum_util, max) \
>>      (((power * (sum_util << 10)) / max) >> 10)
> 
> I don't understand the 'max' division, I was expecting here something
> like: ((sum_util << 10) / sum_max) >> 10)
> 
> no ?
> 

No, it should be single 'max', which is in range 0..1024.
We would like to calculate the power for the whole perf domain, e.g.
4 CPUs almost fully utilized would have util ~1000, then total power
should be around ~4 * EM_table[i].power. This '~4' is coming from
4 utils divided by one max util
4000 / 1024


The 'max' in the equation can be put before the bracket, as well as
'power'.

If we had floating point number, simple power for cpu1, cpu2, cpuN
would be just:
power_1 = power * util_1 / max
power_2 = power * util_2 / max
power_N = power * util_N / max
(since they have the same 'max' capacity and the same EM 'power')

The total domain power would be:
total_power = power_1 + power_2 + ... + power_N
which is:
total_power = (power * util_1 / max) + (power * util_2 / max) + ... +
               + (power * util_N / max)

put the 'power' and 'max' before the bracket:
total_power = power * (util_1 + util_2 + ... + util_N) * (1/max)

introduce the 'sum_util':
sum_util = util_1 + util_2 + ... + util_N
then:
total_power = power * sum_util / max

Unfortunately, we don't use floating point, so temporary fixed point
tricks, thus the '<< 10' and '>> 10' avoid some errors




  reply	other threads:[~2021-03-09 20:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01 21:21 [PATCH 1/5] powercap/drivers/dtpm: Encapsulate even more the code Daniel Lezcano
2021-03-01 21:21 ` [PATCH 2/5] powercap/drivers/dtpm: Create a registering system Daniel Lezcano
2021-03-09 14:46   ` Lukasz Luba
2021-03-01 21:21 ` [PATCH 3/5] powercap/drivers/dtpm: Simplify the dtpm table Daniel Lezcano
2021-03-09 15:02   ` Lukasz Luba
2021-03-01 21:21 ` [PATCH 4/5] powercap/drivers/dtpm: Use container_of instead of a private data field Daniel Lezcano
2021-03-09 15:17   ` Lukasz Luba
2021-03-01 21:21 ` [PATCH 5/5] powercap/drivers/dtpm: Scale the power with the load Daniel Lezcano
2021-03-09 10:01   ` Lukasz Luba
2021-03-09 19:03     ` Daniel Lezcano
2021-03-09 20:44       ` Lukasz Luba [this message]
2021-03-09 19:22     ` Daniel Lezcano
2021-03-08 19:31 ` [PATCH 1/5] powercap/drivers/dtpm: Encapsulate even more the code Daniel Lezcano
2021-03-08 19:55   ` Lukasz Luba
2021-03-08 21:20     ` Daniel Lezcano
2021-03-09 14:02 ` Lukasz Luba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9275548-1555-ce96-51c8-67942d757119@arm.com \
    --to=lukasz.luba@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --subject='Re: [PATCH 5/5] powercap/drivers/dtpm: Scale the power with the load' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.