Messages in this thread |  | | Subject | Re: [PATCH v3 6/7] thermal/drivers/cpu_cooling: Introduce the cpu idle cooling driver | From | Martin Kepplinger <> | Date | Mon, 5 Aug 2019 08:53:39 +0200 |
| |
On 05.08.19 07:11, Martin Kepplinger wrote: > --- > > On 05-04-18, 18:16, Daniel Lezcano wrote: >> The cpu idle cooling driver performs synchronized idle injection across all >> cpus belonging to the same cluster and offers a new method to cool down a SoC. >> >> Each cluster has its own idle cooling device, each core has its own idle >> injection thread, each idle injection thread uses play_idle to enter idle. In >> order to reach the deepest idle state, each cooling device has the idle >> injection threads synchronized together. >> >> It has some similarity with the intel power clamp driver but it is actually >> designed to work on the ARM architecture via the DT with a mathematical proof >> with the power model which comes with the Documentation. >> >> The idle injection cycle is fixed while the running cycle is variable. That >> allows to have control on the device reactivity for the user experience. At >> the mitigation point the idle threads are unparked, they play idle the >> specified amount of time and they schedule themselves. The last thread sets >> the next idle injection deadline and when the timer expires it wakes up all >> the threads which in turn play idle again. Meanwhile the running cycle is >> changed by set_cur_state. When the mitigation ends, the threads are parked. >> The algorithm is self adaptive, so there is no need to handle hotplugging. >> >> If we take an example of the balanced point, we can use the DT for the hi6220. >> >> The sustainable power for the SoC is 3326mW to mitigate at 75°C. Eight cores >> running at full blast at the maximum OPP consumes 5280mW. The first value is >> given in the DT, the second is calculated from the OPP with the formula: >> >> Pdyn = Cdyn x Voltage^2 x Frequency >> >> As the SoC vendors don't want to share the static leakage values, we assume >> it is zero, so the Prun = Pdyn + Pstatic = Pdyn + 0 = Pdyn. >> >> In order to reduce the power to 3326mW, we have to apply a ratio to the >> running time. >> >> ratio = (Prun - Ptarget) / Ptarget = (5280 - 3326) / 3326 = 0,5874 >> >> We know the idle cycle which is fixed, let's assume 10ms. However from this >> duration we have to substract the wake up latency for the cluster idle state. >> In our case, it is 1.5ms. So for a 10ms latency for idle, we are really idle >> 8.5ms. >> >> As we know the idle duration and the ratio, we can compute the running cycle. >> >> running_cycle = 8.5 / 0.5874 = 14.47ms >> >> So for 8.5ms of idle, we have 14.47ms of running cycle, and that brings the >> SoC to the balanced trip point of 75°C. >> >> The driver has been tested on the hi6220 and it appears the temperature >> stabilizes at 75°C with an idle injection time of 10ms (8.5ms real) and >> running cycle of 14ms as expected by the theory above. >> >> Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/thermal/Kconfig | 10 + >> drivers/thermal/cpu_cooling.c | 479 ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/cpu_cooling.h | 6 + >> 3 files changed, 495 insertions(+) >> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >> index 5aaae1b..6c34117 100644 >> --- a/drivers/thermal/Kconfig >> +++ b/drivers/thermal/Kconfig >> @@ -166,6 +166,16 @@ config CPU_FREQ_THERMAL >> This will be useful for platforms using the generic thermal interface >> and not the ACPI interface. >> >> +config CPU_IDLE_THERMAL >> + bool "CPU idle cooling strategy" >> + depends on CPU_IDLE >> + help >> + This implements the generic CPU cooling mechanism through >> + idle injection. This will throttle the CPU by injecting >> + fixed idle cycle. All CPUs belonging to the same cluster >> + will enter idle synchronously to reach the deepest idle >> + state. >> + >> endchoice >> >> config CLOCK_THERMAL >> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c >> index 5c219dc..1eec8d6 100644 >> --- a/drivers/thermal/cpu_cooling.c >> +++ b/drivers/thermal/cpu_cooling.c >> @@ -10,18 +10,33 @@ >> * Viresh Kumar <viresh.kumar@linaro.org> >> * >> */ >> +#define pr_fmt(fmt) "CPU cooling: " fmt >> + >> #include <linux/module.h> >> #include <linux/thermal.h> >> #include <linux/cpufreq.h> >> +#include <linux/cpuidle.h> >> #include <linux/err.h> >> +#include <linux/freezer.h> >> #include <linux/idr.h> >> +#include <linux/kthread.h> >> #include <linux/pm_opp.h> >> #include <linux/slab.h> >> +#include <linux/sched/prio.h> >> +#include <linux/sched/rt.h> >> +#include <linux/smpboot.h> >> #include <linux/cpu.h> >> #include <linux/cpu_cooling.h> >> >> +#include <linux/ratelimit.h> >> + >> +#include <linux/platform_device.h> >> +#include <linux/of_platform.h> >> + >> #include <trace/events/thermal.h> >> >> +#include <uapi/linux/sched/types.h> >> + >> #ifdef CONFIG_CPU_FREQ_THERMAL >> /* >> * Cooling state <-> CPUFreq frequency >> @@ -928,3 +943,467 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) >> } >> EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister); >> #endif /* CONFIG_CPU_FREQ_THERMAL */ >> + >> +#ifdef CONFIG_CPU_IDLE_THERMAL >> +/** >> + * struct cpuidle_cooling_device - data for the idle cooling device >> + * @cdev: a pointer to a struct thermal_cooling_device >> + * @cpumask: a cpumask containing the CPU managed by the cooling device >> + * @timer: a hrtimer giving the tempo for the idle injection cycles >> + * @kref: a kernel refcount on this structure >> + * @count: an atomic to keep track of the last task exiting the idle cycle >> + * @idle_cycle: an integer defining the duration of the idle injection >> + * @state: an normalized integer giving the state of the cooling device >> + */ >> +struct cpuidle_cooling_device { >> + struct thermal_cooling_device *cdev; >> + struct cpumask *cpumask; >> + struct hrtimer timer; >> + struct kref kref; >> + atomic_t count; >> + unsigned int idle_cycle; >> + unsigned long state; >> +}; >> + >> +struct cpuidle_cooling_thread { >> + struct task_struct *tsk; >> + int should_run; >> +}; >> + >> +static DEFINE_PER_CPU(struct cpuidle_cooling_thread, cpuidle_cooling_thread); >> +static DEFINE_PER_CPU(struct cpuidle_cooling_device *, cpuidle_cooling_device); >> + >> +/** >> + * cpuidle_cooling_wakeup - Wake up all idle injection threads >> + * @idle_cdev: the idle cooling device >> + * >> + * Every idle injection task belonging to the idle cooling device and >> + * running on an online cpu will be wake up by this call. >> + */ >> +static void cpuidle_cooling_wakeup(struct cpuidle_cooling_device *idle_cdev) >> +{ >> + struct cpuidle_cooling_thread *cct; >> + int cpu; >> + >> + for_each_cpu_and(cpu, idle_cdev->cpumask, cpu_online_mask) { >> + cct = per_cpu_ptr(&cpuidle_cooling_thread, cpu); >> + cct->should_run = 1; >> + wake_up_process(cct->tsk); >> + } >> +} >> + >> +/** >> + * cpuidle_cooling_wakeup_fn - Running cycle timer callback >> + * @timer: a hrtimer structure >> + * >> + * When the mitigation is acting, the CPU is allowed to run an amount >> + * of time, then the idle injection happens for the specified delay >> + * and the idle task injection schedules itself until the timer event >> + * wakes the idle injection tasks again for a new idle injection >> + * cycle. The time between the end of the idle injection and the timer >> + * expiration is the allocated running time for the CPU. >> + * >> + * Always returns HRTIMER_NORESTART >> + */ >> +static enum hrtimer_restart cpuidle_cooling_wakeup_fn(struct hrtimer *timer) >> +{ >> + struct cpuidle_cooling_device *idle_cdev = >> + container_of(timer, struct cpuidle_cooling_device, timer); >> + >> + cpuidle_cooling_wakeup(idle_cdev); >> + >> + return HRTIMER_NORESTART; >> +} >> + >> +/** >> + * cpuidle_cooling_runtime - Running time computation >> + * @idle_cdev: the idle cooling device >> + * >> + * The running duration is computed from the idle injection duration >> + * which is fixed. If we reach 100% of idle injection ratio, that >> + * means the running duration is zero. If we have a 50% ratio >> + * injection, that means we have equal duration for idle and for >> + * running duration. >> + * >> + * The formula is deduced as the following: >> + * >> + * running = idle x ((100 / ratio) - 1) >> + * >> + * For precision purpose for integer math, we use the following: >> + * >> + * running = (idle x 100) / ratio - idle >> + * >> + * For example, if we have an injected duration of 50%, then we end up >> + * with 10ms of idle injection and 10ms of running duration. >> + * >> + * Returns a s64 nanosecond based >> + */ >> +static s64 cpuidle_cooling_runtime(struct cpuidle_cooling_device *idle_cdev) >> +{ >> + s64 next_wakeup; >> + unsigned long state = idle_cdev->state; >> + >> + /* >> + * The function should not be called when there is no >> + * mitigation because: >> + * - that does not make sense >> + * - we end up with a division by zero >> + */ >> + if (!state) >> + return 0; >> + >> + next_wakeup = (s64)((idle_cdev->idle_cycle * 100) / state) - >> + idle_cdev->idle_cycle; >> + >> + return next_wakeup * NSEC_PER_USEC; >> +} >> + > > There is a bug in your calculation formula here when "state" becomes 100. > You return 0 for the injection rate, which is the same as "rate" being 0, > which is dangerous. You stop cooling when it's most necessary :) > > I'm not sure how much sense really being 100% idle makes, so I, when testing > this, just say if (state == 100) { state = 99 }. Anyways, just don't return 0. >
oh and also, this breaks S3 suspend:
Aug 5 06:09:20 pureos kernel: [ 807.487887] PM: suspend entry (deep) Aug 5 06:09:40 pureos kernel: [ 807.501148] Filesystems sync: 0.013 seconds Aug 5 06:09:40 pureos kernel: [ 807.501591] Freezing user space processes ... (elapsed 0.003 seconds) done. Aug 5 06:09:40 pureos kernel: [ 807.504741] OOM killer disabled. Aug 5 06:09:40 pureos kernel: [ 807.504744] Freezing remaining freezable tasks ... Aug 5 06:09:40 pureos kernel: [ 827.517712] Freezing of tasks failed after 20.002 seconds (4 tasks refusing to freeze, wq_busy=0): Aug 5 06:09:40 pureos kernel: [ 827.527122] thermal-idle/0 S 0 161 2 0x00000028 Aug 5 06:09:40 pureos kernel: [ 827.527131] Call trace: Aug 5 06:09:40 pureos kernel: [ 827.527148] __switch_to+0xb4/0x200 Aug 5 06:09:40 pureos kernel: [ 827.527156] __schedule+0x1e0/0x488 Aug 5 06:09:40 pureos kernel: [ 827.527162] schedule+0x38/0xc8 Aug 5 06:09:40 pureos kernel: [ 827.527169] smpboot_thread_fn+0x250/0x2a8 Aug 5 06:09:40 pureos kernel: [ 827.527176] kthread+0xf4/0x120 Aug 5 06:09:40 pureos kernel: [ 827.527182] ret_from_fork+0x10/0x18 Aug 5 06:09:40 pureos kernel: [ 827.527186] thermal-idle/1 S 0 162 2 0x00000028 Aug 5 06:09:40 pureos kernel: [ 827.527192] Call trace: Aug 5 06:09:40 pureos kernel: [ 827.527197] __switch_to+0x188/0x200 Aug 5 06:09:40 pureos kernel: [ 827.527203] __schedule+0x1e0/0x488 Aug 5 06:09:40 pureos kernel: [ 827.527208] schedule+0x38/0xc8 Aug 5 06:09:40 pureos kernel: [ 827.527213] smpboot_thread_fn+0x250/0x2a8 Aug 5 06:09:40 pureos kernel: [ 827.527218] kthread+0xf4/0x120 Aug 5 06:09:40 pureos kernel: [ 827.527222] ret_from_fork+0x10/0x18 Aug 5 06:09:40 pureos kernel: [ 827.527226] thermal-idle/2 S 0 163 2 0x00000028 Aug 5 06:09:40 pureos kernel: [ 827.527231] Call trace: Aug 5 06:09:40 pureos kernel: [ 827.527237] __switch_to+0xb4/0x200 Aug 5 06:09:40 pureos kernel: [ 827.527242] __schedule+0x1e0/0x488 Aug 5 06:09:40 pureos kernel: [ 827.527247] schedule+0x38/0xc8 Aug 5 06:09:40 pureos kernel: [ 827.527259] smpboot_thread_fn+0x250/0x2a8 Aug 5 06:09:40 pureos kernel: [ 827.527264] kthread+0xf4/0x120 Aug 5 06:09:40 pureos kernel: [ 827.527268] ret_from_fork+0x10/0x18 Aug 5 06:09:40 pureos kernel: [ 827.527272] thermal-idle/3 S 0 164 2 0x00000028 Aug 5 06:09:40 pureos kernel: [ 827.527278] Call trace: Aug 5 06:09:40 pureos kernel: [ 827.527283] __switch_to+0xb4/0x200 Aug 5 06:09:40 pureos kernel: [ 827.527288] __schedule+0x1e0/0x488 Aug 5 06:09:40 pureos kernel: [ 827.527293] schedule+0x38/0xc8 Aug 5 06:09:40 pureos kernel: [ 827.527298] smpboot_thread_fn+0x250/0x2a8 Aug 5 06:09:40 pureos kernel: [ 827.527303] kthread+0xf4/0x120 Aug 5 06:09:40 pureos kernel: [ 827.527308] ret_from_fork+0x10/0x18 Aug 5 06:09:40 pureos kernel: [ 827.527375] Restarting kernel threads ... done. Aug 5 06:09:40 pureos kernel: [ 827.527771] OOM killer enabled. Aug 5 06:09:40 pureos kernel: [ 827.527772] Restarting tasks ... done. Aug 5 06:09:40 pureos kernel: [ 827.528926] PM: suspend exit
do you know where things might go wrong here?
thanks,
martin
|  |