Re: [RFC PATCH v4 03/12] PM: Introduce an Energy Model management framework

[Dietmar Eggemann <dietmar.eggemann@arm.com>]

On 07/16/2018 12:29 PM, Quentin Perret wrote:
> On Tuesday 10 Jul 2018 at 09:32:02 (+0100), Quentin Perret wrote:

[...]

> Another thing to take into consideration here is basically that the
> thermal subsystem (IPA) will be impacted by the RCU protection on the
> cs_table. However, since the 'frequency' and 'power' fields do not change
> at run-time, and since IPA doesn't need the 'capacity' value, there is no
> good reason to have IPA do rcu_read_lock() all over the place, so
> arguably something needs to be fixed here.
> 
> One possibility is to remove entirely the following struct:
> 	struct em_cap_state {
> 		unsigned long capacity;
> 		unsigned long frequency; /* Kilo-hertz */
> 		unsigned long power; /* Milli-watts */
> 	};
> 
> and to have three independent vectors (of the same size) for frequency,
> power and capacity. That way only the 'capacity' vector would be RCU
> protected, and IPA could use 'frequency' and 'power' directly, without
> further protections.
> 
> A second possibility is to remove the capacity values from the EM
> altogether (as I suggested in my previous message) and to get rid of the
> need for RCU protection at the same occasion.

I see an impact of 'calculating capacity on the fly' in 
compute_energy()->em_fd_energy(). Running the first energy test case (# 
task equal 10) on the Juno r0 board with function profiling gives me:

v4:

   Function              Hit    Time            Avg             s^2
A53 - cpu [0,3-5]
   compute_energy      14620    30790.86 us     2.106 us        8.421 us
   compute_energy        682    1512.960 us     2.218 us        0.154 us
   compute_energy       1207    2627.820 us     2.177 us        0.172 us
   compute_energy         93    206.720 us      2.222 us        0.151 us
A57 - cpu [1-2]
   compute_energy        153    160.100 us      1.046 us        0.190 us
   compute_energy        136    130.760 us      0.961 us        0.077 us


v4 + 'calculating capacity on the fly':

   Function              Hit    Time            Avg             s^2
A53 - cpu [0,3-5]
   compute_energy      11623    26941.12 us     2.317 us        12.203 us
   compute_energy       5062    11771.48 us     2.325 us        0.819 us
   compute_energy       4391    10396.78 us     2.367 us        1.753 us
   compute_energy       2234    5265.640 us     2.357 us        0.955 us
A57 - cpu [1-2]
   compute_energy         59    66.020 us       1.118 us        0.112 us
   compute_energy        229    234.880 us      1.025 us        0.135 us

The code is not optimized, I just replaced cs->capacity with 
arch_scale_cpu_capacity(NULL, cpu) (max_cap) and 'max_cap * 
cs->frequency / max_freq' respectively.
There are 3 compute_energy() calls per wake-up on a system with 2 
frequency domains.

> The second option simplifies the code of the EM framework significantly
> (no more em_rescale_cpu_capacity()) and shouldn't introduce massive
> overheads on the scheduler side (the energy calculation already
> requires one multiplication and one division, so nothing new on that
> side). At the same time, that would make it a whole lot easier to
> interface the EM framework with IPA without having to deal with RCU all
> over the place.

IMO, em_rescale_cpu_capacity() is just the capacity related example what 
the EM needs if its values can be changed at runtime. There might be 
other use cases in the future like changing power values depending on 
temperature.
So maybe it's a good idea to not have this 'EM values can change at 
runtime' feature in the first version of the EM and emphasize on 
simplicity of the code instead (if we can eliminate the extra runtime 
overhead).

[...]