Messages in this thread | | | From | Doug Anderson <> | Date | Thu, 11 Jun 2020 10:48:40 -0700 | Subject | Re: [PATCH] cros_ec_spi: Even though we're RT priority, don't bump cpu freq |
| |
Hi,
On Thu, Jun 11, 2020 at 4:03 AM Quentin Perret <qperret@google.com> wrote: > > Hi Doug, > > On Wednesday 10 Jun 2020 at 15:18:43 (-0700), Douglas Anderson wrote: > > The cros_ec_spi driver is realtime priority so that it doesn't get > > preempted by other taks while it's talking to the EC but overall it > > really doesn't need lots of compute power. Unfortunately, by default, > > the kernel assumes that all realtime tasks should cause the cpufreq to > > jump to max and burn through power to get things done as quickly as > > possible. That's just not the correct behavior for cros_ec_spi. > > Is this specific to this driver, or something you would want applied > more globally to all RT tasks in ChromeOS (which is what we'd like to > have in Android for instance)?
Hrm. I guess my first instinct is to say that we still want this patch even if we have something that is applied more globally. Specifically it sounds as if the patch you point at is suggesting that we'd tweak the boost value to something other than max but we'd still have a boost value. In the case of cros_ec_spi I don't believe we need any boost value at all, so my patch would still be useful. The computational needs of cros_ec_spi are very modest and it can do its work at lower CPU frequencies just fine. It just can't be interrupted for large swaths of time.
> IOW, how do you feel about 20200511154053.7822-1-qais.yousef@arm.com ?
I'm not totally a fan, but I'm definitely not an expert in this area (I've also only read the patch description and not the patch or the whole thread). I really don't want yet another value that I need to tune from board to board. Even worse, this tuning value isn't board-specific but a combination of board and software specific. By this, I'd imagine a scenario where you're using a real-time task to get audio decoding done within a certain latency. I guess you'd tune this value to make sure that you can get all your audio decoding done in time but also not burn extra power. Now, imagine that the OS upgrades and the audio task suddenly has to decode more complex streams. You've got to go across all of your boards and re-tune every one? ...or, nobody thinks about it and older boards start getting stuttery audio? Perhaps the opposite happens and someone comes up with a newer lower-cpu-intensive codec and you could save power. Sounds like a bit of a nightmare.
I'd rather have a boolean value: boost all RT threads to max vs. don't boost all RT threads to max. Someone that just wanted RT stuff to run as fast as possible without any hassle on their system and didn't care about power efficiency could turn this on. Anyone who really cared about power could turn this off and then could find a more targeted way to boost things, hopefully in a way that doesn't require tuning. One option would be to still boost the CPU to max but only for certain tasks known to be really latency sensitive. Another might be to somehow measure whether or not the task is making its deadlines and boost the CPU frequency up if deadlines are not being met. I'm sure there are fancier ways.
...of course, I believe your patch allows me to do what I want: I can just set the default boost to 0. It just leaves in the temptation for others to require a default boost of something else and then I'm stuck in my tuning nightmare.
-Doug
-Doug
| |