Messages in this thread | | | Date | Wed, 21 Dec 2022 21:43:24 +0100 | Subject | Re: [RFC][PATCH] thermal/idle_inject: Support 100% idle injection | From | Daniel Lezcano <> |
| |
On 21/12/2022 21:36, srinivas pandruvada wrote: > On Wed, 2022-12-21 at 14:43 +0100, Daniel Lezcano wrote: >> >> Hi Srinivas, >> >> >> On 09/12/2022 02:36, Srinivas Pandruvada wrote: >>> The users of idle injection framework allow 100% idle injection. >>> For >>> example: thermal/cpuidle_cooling.c driver. When the ratio set to >>> 100%, >>> the runtime_duration becomes zero. >>> >>> In the function idle_inject_set_duration() in idle injection >>> framework >>> run_duration_us == 0 is silently ignored, without any error (it is >>> a >>> void function). So, the caller will assume that everything is fine >>> and >>> 100% idle is effective. But in reality the idle inject will be >>> whatever >>> set before. >> >> Good catch >> >>> There are two options: >>> - The caller change their max state to 99% instead of 100% and >>> document that 100% is not supported by idle inject framework >>> - Support 100% idle support in idle inject framework >> >> Yes, from my POV a CPU being impossible to cool down for any reason >> should end up by staying off. >> >>> Since there are other protections via RT throttling, this framework >>> can >>> allow 100% idle. The RT throttling will be activated at 95% idle by >>> default. The caller disabling RT throttling and injecting 100% >>> idle, >>> should be aware that CPU can't be used at all. >> >> Would it make sense to write a trace in this case ? > > There is one printk already: > printk_deferred_once("sched: RT throttling activated\n") > You mean we should add > > trace_sched_* for this?
I meant the CPU is going 100% idle
>>> The idle inject timer is started for (run_duration_us + >>> idle_duration_us) >>> duration. Hence replace (run_duration_us && idle_duration_us) with >>> (run_duration_us + idle_duration_us) in the function >>> idle_inject_set_duration(). >> >> Sounds good to me >> > I will submit a patch for this. > > Thanks, > Srinivas > >>> Signed-off-by: Srinivas Pandruvada >>> <srinivas.pandruvada@linux.intel.com> >>> --- >>> drivers/powercap/idle_inject.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/powercap/idle_inject.c >>> b/drivers/powercap/idle_inject.c >>> index f48e71501429..4a4fe60d2563 100644 >>> --- a/drivers/powercap/idle_inject.c >>> +++ b/drivers/powercap/idle_inject.c >>> @@ -184,7 +184,7 @@ void idle_inject_set_duration(struct >>> idle_inject_device *ii_dev, >>> unsigned int run_duration_us, >>> unsigned int idle_duration_us) >>> { >>> - if (run_duration_us && idle_duration_us) { >>> + if (run_duration_us + idle_duration_us) { >>> WRITE_ONCE(ii_dev->run_duration_us, >>> run_duration_us); >>> WRITE_ONCE(ii_dev->idle_duration_us, >>> idle_duration_us); >>> } >> >
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |