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).
[...]
